Skip to content

Name suggestions don't contain suggestions starting with a digit.#41224

Merged
dpoeschl merged 7 commits intodotnet:masterfrom
chris-schuetz:Issue39519
Jan 31, 2020
Merged

Name suggestions don't contain suggestions starting with a digit.#41224
dpoeschl merged 7 commits intodotnet:masterfrom
chris-schuetz:Issue39519

Conversation

@chris-schuetz
Copy link
Contributor

@chris-schuetz chris-schuetz commented Jan 26, 2020

Fixes #39519

@chris-schuetz chris-schuetz requested a review from a team as a code owner January 26, 2020 09:21
@CyrusNajmabadi
Update src/Features/CSharp/Portable/Completion/CompletionProviders/De…
0b37396 
…clarationNameCompletionProvider.NameGenerator.cs

Co-Authored-By: CyrusNajmabadi <[email protected]>
@jinujoseph jinujoseph added Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. labels Jan 28, 2020
chris-schuetz and others added 2 commits January 29, 2020 21:47
@CyrusNajmabadi
Update src/Features/CSharp/Portable/Completion/CompletionProviders/De…
a9439ce 
…clarationNameCompletionProvider.NameGenerator.cs

Co-Authored-By: CyrusNajmabadi <[email protected]>
Using CSharpSyntaxFactsService.Instance instead of accessing it via D…
186b940 
…ocument.GetLanguageService<ISyntaxFactsService>().
dpoeschl
dpoeschl suggested changes Jan 29, 2020
View reviewed changes
Copy link
Contributor

@dpoeschl dpoeschl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one test-related issue left for me.

Thank you for your contribution here!

@chris-schuetz
Copy link
Contributor Author

chris-schuetz commented Jan 29, 2020

I had to move the IsValidIdentifier check to a different position in the DeclarationNameCompletionProvider.

The DeclarationNameCompletionProvider will try to escape keywords shortly before adding them to the suggestions. This allows an invalid identifier like string to become a valid identifier @string. I moved the check after the escape operation was performed, so name suggestions aren't removed if they can be made valid by escaping.

dpoeschl
dpoeschl approved these changes Jan 30, 2020
View reviewed changes
Copy link
Contributor

@dpoeschl dpoeschl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Thanks @chris-schuetz!

@dpoeschl dpoeschl merged commit 766d69e into dotnet:master Jan 31, 2020
@chris-schuetz chris-schuetz deleted the Issue39519 branch February 1, 2020 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@CyrusNajmabadi CyrusNajmabadi CyrusNajmabadi left review comments

@dpoeschl dpoeschl dpoeschl approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@dpoeschl dpoeschl

Labels

Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Intellisense suggests numeric identifier name.

4 participants

@chris-schuetz @dpoeschl @CyrusNajmabadi @jinujoseph