Expose Go To Impl and Spell Check to Razor #74978
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
| @@ -33,26 +34,32 @@ public FindImplementationsHandler(IGlobalOptionService globalOptions) | |||
|
|
|||
| public LSP.TextDocumentIdentifier GetTextDocumentIdentifier(LSP.TextDocumentPositionParams request) => request.TextDocument; | |||
|
|
|||
| public async Task<LSP.Location[]> HandleRequestAsync(LSP.TextDocumentPositionParams request, RequestContext context, CancellationToken cancellationToken) | |||
| public Task<LSP.Location[]> HandleRequestAsync(LSP.TextDocumentPositionParams request, RequestContext context, CancellationToken cancellationToken) | |||
There was a problem hiding this comment.
assuming you intentionally dropped async here
There was a problem hiding this comment.
Indeed it would be a compiler warning if I left it in place.
If an async method only has a single await at the end, it is usually better to remove it, and the async modifier, and just return the Task directly. This avoids the creation of a state machine and so gives a slight perf bump. It does however change the call stack for exceptions that might be thrown so I wouldn't say its a hard and fast rule.
| { | ||
| public readonly record struct SpellCheckSpan(int StartIndex, int Length, VSInternalSpellCheckableRangeKind Kind); | ||
|
|
||
| public static async Task<ImmutableArray<SpellCheckSpan>> GetSpellCheckSpansAsync(Document document, CancellationToken cancellationToken) |
There was a problem hiding this comment.
would it be helpful to include try catch block in the GetSpellCheckSpansAsync method here?
There was a problem hiding this comment.
I think for our purposes we're better off letting the exception bubble out of the ExternalAccess layer here, and if necessary then we can handle it in Razor. If we handled the exceptions here it would go through Roslyn's exception reporting and telemetry, which might make it harder to get in front of the right team.
|
@dibarbet @JoeRobich strictly speaking with Andrew's approval I could merge this, but that feels a little hollow. Would love a second sign-off :) |
Needs dotnet/roslyn#74978 Fixes #10695 Part of #9519 Pretty straightforward. A tiny bit of code moved to be shared with Go To Def, but that's it.
Needs dotnet/roslyn#74978 Fixes #10746 Part of #9519
Part of dotnet/razor#10746 and dotnet/razor#10695