Skip to content

Fixing async keyword highlighting on local functions#25037

Merged
DustinCampbell merged 7 commits intodotnet:masterfrom
Neme12:asyncHighlighter
Apr 5, 2018
Merged

Fixing async keyword highlighting on local functions#25037
DustinCampbell merged 7 commits intodotnet:masterfrom
Neme12:asyncHighlighter

Conversation

@Neme12
Copy link
Copy Markdown
Contributor

@Neme12 Neme12 commented Feb 24, 2018 •
edited
Loading

fixes #25036

First 4 commits are just refactoring/cleanup. The last commit actually adds local functions.

The cleanup involved (it's not clear from the final diff, which shows some false renames):

  • Replacing AsyncSimpleLambdaHighlighter, AsyncParenthesizedLambdaHighlighter & AsyncAnonymousMethodHighlighter with one class: AsyncAnonymousFunctionHighlighter (their functionality was exactly the same)
  • Merging their equivalent test classes: AsyncSimpleLambdaHighlighterTests, AsyncParenthesizedLambdaHighlighterTests & AsyncAnonymousMethodHighlighterTests into AsyncAnonymousFunctionHighlighterTests
  • In AbstractAsyncHighlighter<TNode>.HighlightRelatedKeywords, replacing cases for SimpleLambdaExpressionSyntax, ParenthesizedLambdaExpressionSyntax & AnonymousMethodExpressionSyntax with one for AnonymousFunctionExpressionSyntax (the code was exactly the same)

@Neme12
Refactoring into AsyncAnonymousFunctionHighlighter (replacing AsyncSi…
f3e3e27 
…mpleLambdaHighlighter, AsyncParenthesizedLambdaHighlighter & AsyncAnonymousMethodHighlighter)
@Neme12
Common case for AnonymousFunctionExpressionSyntax
96c945a
@Neme12
Renaming tests in AsyncSimpleLambdaHighlighterTests, AsyncParenthesiz…
486dd72 
…edLambdaHighlighterTests & AsyncAnonymousMethodHighlighterTests (so they can be merged into one class)
@Neme12
Moving tests into AsyncAnonymousFunctionHighlighterTests (replacing A…
8f42974 
…syncSimpleLambdaHighlighterTests, AsyncParenthesizedLambdaHighlighterTests & AsyncAnonymousMethodHighlighterTests) for parity with AsyncAnonymousFunctionHighlighter
@Neme12
Adding AsyncLocalFunctionHighlighter + test
d8b038e
@Neme12 Neme12 requested a review from a team as a code owner February 24, 2018 17:28
@jcouv jcouv mentioned this pull request Feb 25, 2018
Closed
@Neme12
Copy link
Copy Markdown
Contributor Author

Neme12 commented Mar 3, 2018

@dotnet-bot retest ubuntu_16_mono_debug_prtest please

@jcouv jcouv added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Mar 11, 2018
@Neme12
Copy link
Copy Markdown
Contributor Author

Neme12 commented Mar 29, 2018

retest windows_debug_vs-integration_prtest please

@Neme12
Copy link
Copy Markdown
Contributor Author

Neme12 commented Mar 29, 2018

retest windows_release_vs-integration_prtest please

@jcouv jcouv added the Area-IDE label Mar 30, 2018
@jcouv jcouv added this to the 15.8 milestone Mar 30, 2018
DustinCampbell
DustinCampbell approved these changes Apr 5, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@DustinCampbell DustinCampbell left a comment

Choose a reason for hiding this comment

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

Looks great @Neme12!

@DustinCampbell DustinCampbell requested a review from a team April 5, 2018 12:09
@DustinCampbell
Copy link
Copy Markdown
Member

DustinCampbell commented Apr 5, 2018

@dotnet-bot restest this please

@Neme12
Copy link
Copy Markdown
Contributor Author

Neme12 commented Apr 5, 2018

@dotnet-bot retest this please

@Neme12
Copy link
Copy Markdown
Contributor Author

Neme12 commented Apr 5, 2018

failure is #25931

@Neme12
Copy link
Copy Markdown
Contributor Author

Neme12 commented Apr 5, 2018

@dotnet-bot retest windows_debug_unit64_prtest please

@Neme12
Copy link
Copy Markdown
Contributor Author

Neme12 commented Apr 5, 2018

@dotnet-bot retest windows_debug_unit32_prtest please

@Neme12
Copy link
Copy Markdown
Contributor Author

Neme12 commented Apr 5, 2018

retest windows_release_unit32_prtest please

@Neme12
Copy link
Copy Markdown
Contributor Author

Neme12 commented Apr 5, 2018

retest windows_debug_unit32_prtest please

@Neme12
Copy link
Copy Markdown
Contributor Author

Neme12 commented Apr 5, 2018

retest windows_debug_spanish_unit32_prtest please

@Neme12
Copy link
Copy Markdown
Contributor Author

Neme12 commented Apr 5, 2018 •
edited
Loading

@DustinCampbell I finally won fighting the tests here... would you give this small-ish PR also a chance? 😄

@DustinCampbell
Copy link
Copy Markdown
Member

DustinCampbell commented Apr 5, 2018

Absolutely, and thanks for your contribution. As the one who originally wrote that keyword highlighting code near the beginning of Roslyn, I'm happy to see it updated. Thanks!

@DustinCampbell DustinCampbell self-assigned this Apr 5, 2018
@DustinCampbell DustinCampbell merged commit 2715ecb into dotnet:master Apr 5, 2018
@Neme12
Copy link
Copy Markdown
Contributor Author

Neme12 commented Apr 5, 2018

Thanks!

@Neme12 Neme12 deleted the asyncHighlighter branch April 5, 2018 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DustinCampbell DustinCampbell DustinCampbell approved these changes

+1 more reviewer

@CyrusNajmabadi CyrusNajmabadi CyrusNajmabadi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@DustinCampbell DustinCampbell

Labels

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

Projects

None yet

Milestone

15.8

Development

Successfully merging this pull request may close these issues.

Async keyword not highlighted on local functions

4 participants

@Neme12 @DustinCampbell @CyrusNajmabadi @jcouv