-
Notifications
You must be signed in to change notification settings - Fork 226
Ensure RazorVSInternalCompletionParams is used for serialization of completion requests #12271
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ensure RazorVSInternalCompletionParams is used for serialization of completion requests #12271
Conversation
…ompletion requests Without this type being used, serialization loses the VSInternalCompletionContext.InvokeKind, which is critical for webtools to determine the correct range to replace. Without this, webtools indicates a range to be replaced based on the assumption that this is a forced completion session, which in turns affects how vseditor determines filtering of items in the completion list. Fixes https://dev.azure.com/devdiv/DevDiv/_workitems/edit/2581568
...rc/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Cohost/HtmlRequestInvoker.cs
Show resolved
Hide resolved
| if (documentPositionInfo.LanguageKind == RazorLanguageKind.Html) | ||
| { | ||
| htmlCompletionList = await GetHtmlCompletionListAsync(request, razorDocument, razorCompletionOptions, correlationId, cancellationToken).ConfigureAwait(false); | ||
| var completionParams = new RazorVSInternalCompletionParams() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can/should this endpoint just use RazorVSCompletionParams in its definition directly? They look like they'd be serialization-compatible?
Could be a follow up though, if this needs to get into P2 under M2. We also need to make sure this doesn't do anything weird in VS Code, but that can also wait since we control insertions there more directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can/should this endpoint just use RazorVSCompletionParams in its definition directly? They look like they'd be serialization-compatible?
I'll take a look
if this needs to get into P2 under M2
I'm not too concerned about getting this into P2
We also need to make sure this doesn't do anything weird in VS Code
At some point, I'm going to have to get comfortable doing this, but for now, I think I'd still like to lean on you for that verification
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll take a look
Seems to work, updated in commit 2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At some point, I'm going to have to get comfortable doing this, but for now, I think I'd still like to lean on you for that verification
Sure thing, but I'm sure I'll forget by the week after next when I get back to do it 😛
It's pretty easy though, just create a .vscode folder under your project root, and a settings.json file in there, and put in:
{
"dotnet.server.componentPaths": {
"razorExtension": "C:\\[Path to your Razor working folder]\artifacts\\bin\\Microsoft.VisualStudioCode.RazorExtension\\Debug\\net9.0\\"
}
}
2) Propagate RazorVSInternalCompletionParams usage a bit further out per David's suggestion
Without this type being used, serialization loses the VSInternalCompletionContext.InvokeKind, which is critical for webtools to determine the correct range to replace.
Without this, webtools indicates a range to be replaced based on the assumption that this is a forced completion session, which in turns affects how vseditor determines filtering of items in the completion list.
Fixes https://dev.azure.com/devdiv/DevDiv/_workitems/edit/2581568