Allow LSP and cohosting to provide specialized methods to get a syntax tree#10765
Merged
davidwengier merged 8 commits intodotnet:mainfrom Aug 21, 2024
Merged
Allow LSP and cohosting to provide specialized methods to get a syntax tree#10765davidwengier merged 8 commits intodotnet:mainfrom
davidwengier merged 8 commits intodotnet:mainfrom
Conversation
…x tree for a C# document
This was a subtle future bug, that we would have hit when we removed the C# syntax tree bits in future. By checking for Razor GTD first we weren't deferring to Roslyn for plain attributes (ie, the case without `@bind-` in the tests) which means at best we do some extra unnecessary work, and at worst we could lose features for things Roslyn supports but we don't. I did not intend to find this bug when I started. Sorry for missing it in the PR Dustin. The key is that `ignoreAttributes` should have been `true` when porting over the code, and changing that would have made the test fail in your branch.
See comment in the file, but essentially this is the only way for the code that returns the generated documents syntax tree to be hit in cohosting.
In light of "should we get rid of document context" convo this morning, figured this made sense to do (and I needed _something_ on IDocumentSnapshot in order to actually make this whole idea work)
...soft.CodeAnalysis.Razor.Workspaces/GoToDefinition/AbstractRazorComponentDefinitionService.cs
Outdated
Show resolved
Hide resolved
...rc/Microsoft.CodeAnalysis.Razor.Workspaces/GoToDefinition/RazorComponentDefinitionHelpers.cs
Show resolved
Hide resolved
...or/src/Microsoft.CodeAnalysis.Remote.Razor/GoToDefinition/RazorComponentDefinitionService.cs
Outdated
Show resolved
Hide resolved
Member
Author
|
Thanks for the feedback @DustinCampbell, I ended up adding a method to get the syntax tree from a document snapshot, as you suggested, and it's much neater now. |
This was referenced Aug 29, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is a classic self-nerd-snipe, and resolves this comment: #10750 (comment)
Also inadvertently found a slight "bug" with the existing go to def code in cohosting. Bug is in quotes because the actual user behaviour probably was identical in 99% of cases.