Fix OmniSharp formatting issue #8669
Conversation
| public const string RazorDocumentFormattingEndpoint = "textDocument/formatting"; | ||
|
|
||
| public const string RazorDocumentOnTypeFormattingEndpoint = "textDocument/onTypeFormatting"; |
There was a problem hiding this comment.
These were using the per-spec LSP methods, but for internal comms between our language servers. Bad mojo. This is where I started because I assumed this would be the issue, but I was totally wrong, but figured it was still good to fix this. Most of the changes are due to this clean up :)
| @@ -5,32 +5,28 @@ namespace Microsoft.AspNetCore.Razor.LanguageServer.Common; | |||
|
|
|||
| internal static class RazorLanguageServerCustomMessageTargets | |||
| { | |||
There was a problem hiding this comment.
Decided to re-org this file, so we can track how we're going on the path to single server everywhere. Further along than I thought!
src/Razor/src/Microsoft.AspNetCore.Razor.VSCode/src/Formatting/FormattingHandler.ts
Show resolved
Hide resolved
| // Our OnTypeFormatter doesn't do anything at the moment, but it's needed so | ||
| // VS Code doesn't throw an exception when it tries to send us an | ||
| // OnTypeFormatting request. |
There was a problem hiding this comment.
Our on type formatter now does do something, but also I wasn't getting any actual hits on breakpoints in it, so not sure what is going on here. Still investigating this.
There was a problem hiding this comment.
Okay, this was just the big "do on type formatting" setting in VS Code. With that on, this works, though VS Code didn't want to send back any edits, but I think its probably better to just leave this in place, should they start. The server should handle the edits fine.
| @@ -254,63 +254,6 @@ public override async Task<RazorDocumentRangeFormattingResponse> HtmlOnTypeForma | |||
| return response; | |||
| } | |||
|
|
|||
| public override async Task<RazorDocumentRangeFormattingResponse> RazorRangeFormattingAsync(RazorDocumentRangeFormattingParams request, CancellationToken cancellationToken) | |||
There was a problem hiding this comment.
We moved away from range formatting ages ago, but this was never cleaned up. Nothing ever called this
|
Just had a thought. @allisonchou is it better to do this sort of thing as two PRs, one for the server, one for the extension, so I can also bump version numbers or something? Or is the version number stuff all in the O# repo? |
There was a problem hiding this comment.
🎉 !!!
Just had a thought. @allisonchou is it better to do this sort of thing as two PRs, one for the server, one for the extension, so I can also bump version numbers or something? Or is the version number stuff all in the O# repo?
The version numbers are in the O# repo so we should be good here
src/Razor/src/Microsoft.AspNetCore.Razor.VSCode/src/Formatting/FormattingHandler.ts
Show resolved
Hide resolved
src/Razor/src/Microsoft.AspNetCore.Razor.VSCode/src/Formatting/FormattingHandler.ts
Outdated
Show resolved
Hide resolved
| @@ -38,6 +49,12 @@ export class FormattingHandler { | |||
| return this.emptyFormattingResponse; | |||
| } | |||
|
|
|||
| const textDocument = await vscode.workspace.openTextDocument(razorDocumentUri); | |||
There was a problem hiding this comment.
Also, is this line needed? I think documentManager.getDocument already opens the document for us
There was a problem hiding this comment.
I'm not actually needing the document opening part of this, but this was the only API I could find that gave me a TextDocument I needed to call the document synchronizer. If there is another/better way to do this, I'm all ears. I copied this from another PR I had, which I'm unable to link to, so could be a few spots to update if there is a better one.
|
|
||
| const serializableTextEdit = convertTextEditToSerializable(textEdit); | ||
| serializableTextEdits.push(serializableTextEdit); | ||
| const textDocument = await vscode.workspace.openTextDocument(razorDocumentUri); |
There was a problem hiding this comment.
(same question here as above)
src/Razor/src/Microsoft.AspNetCore.Razor.VSCode/src/Formatting/FormattingHandler.ts
Show resolved
Hide resolved
This is the Blue/Green version of dotnet/razor#8669. I literally just copied the code over from one repo to the other. Also testsed in Blue because I couldn't test in O# locally for some reason. Fixes dotnet#5561 for Blue/Green
Fixes dotnet/vscode-csharp#5561
Took the opportunity to clean up some of this, because it was a bit odd. Have left comments in the PR to explain what the changes are.