Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix OmniSharp formatting issue #8669

Merged
merged 6 commits into from
May 8, 2023
Merged

Fix OmniSharp formatting issue #8669

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7cd3723
Don't use a real LSP method for our internal LSP method
davidwengier May 4, 2023
310c835
Remove unused code
davidwengier May 4, 2023
bc66573
Remove unused types
davidwengier May 4, 2023
c9f5677
Update OmniSharp handlers to listen for the new LSP methods
davidwengier May 4, 2023
af17392
Fix tests
davidwengier May 4, 2023
2c25a1b
PR feedback
davidwengier May 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer.Common/LanguageServerConstants.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,6 @@ public static class LanguageServerConstants

public const string RazorCodeActionRunnerCommand = "razor/runCodeAction";

public const string RazorDocumentFormattingEndpoint = "textDocument/formatting";

public const string RazorDocumentOnTypeFormattingEndpoint = "textDocument/onTypeFormatting";
Comment on lines -32 to -34
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 :)


public const string RazorCompletionEndpointName = "razor/completion";

public const string RazorCompletionResolveEndpointName = "razor/completionItem/resolve";
Expand Down
40 changes: 15 additions & 25 deletions ...crosoft.AspNetCore.Razor.LanguageServer.Common/RazorLanguageServerCustomMessageTargets.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +5,28 @@ namespace Microsoft.AspNetCore.Razor.LanguageServer.Common;

internal static class RazorLanguageServerCustomMessageTargets
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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!

public const string RazorUpdateCSharpBufferEndpoint = "razor/updateCSharpBuffer";
// VS Internal
public const string RazorInlineCompletionEndpoint = "razor/inlineCompletion";
public const string RazorValidateBreakpointRangeName = "razor/validateBreakpointRange";
public const string RazorOnAutoInsertEndpointName = "razor/onAutoInsert";
public const string RazorSemanticTokensRefreshEndpoint = "razor/semanticTokensRefresh";
public const string RazorTextPresentationEndpoint = "razor/textPresentation";
public const string RazorUriPresentationEndpoint = "razor/uriPresentation";

// Cross platform
public const string RazorUpdateCSharpBufferEndpoint = "razor/updateCSharpBuffer";
public const string RazorUpdateHtmlBufferEndpoint = "razor/updateHtmlBuffer";

public const string RazorRangeFormattingEndpoint = "razor/rangeFormatting";

public const string RazorProvideCodeActionsEndpoint = "razor/provideCodeActions";

public const string RazorResolveCodeActionsEndpoint = "razor/resolveCodeActions";

public const string RazorProvideSemanticTokensRangeEndpoint = "razor/provideSemanticTokensRange";

public const string RazorProvideHtmlDocumentColorEndpoint = "razor/provideHtmlDocumentColor";

public const string RazorProvideHtmlColorPresentationEndpoint = "razor/provideHtmlColorPresentation";

public const string RazorInlineCompletionEndpoint = "razor/inlineCompletion";

public const string RazorProvideHtmlDocumentColorEndpoint = "razor/provideHtmlDocumentColor";
public const string RazorPullDiagnosticEndpointName = "razor/pullDiagnostics";
public const string RazorProvideSemanticTokensRangeEndpoint = "razor/provideSemanticTokensRange";
public const string RazorFoldingRangeEndpoint = "razor/foldingRange";
public const string RazorHtmlFormattingEndpoint = "razor/htmlFormatting";
public const string RazorHtmlOnTypeFormattingEndpoint = "razor/htmlOnTypeFormatting";

public const string RazorSemanticTokensRefreshEndpoint = "razor/semanticTokensRefresh";

public const string RazorTextPresentationEndpoint = "razor/textPresentation";

public const string RazorUriPresentationEndpoint = "razor/uriPresentation";

// Still to migrate
public const string RazorRenameEndpointName = "razor/rename";

public const string RazorHoverEndpointName = "razor/hover";
Expand All @@ -43,11 +39,5 @@ internal static class RazorLanguageServerCustomMessageTargets

public const string RazorImplementationEndpointName = "razor/implementation";

public const string RazorOnAutoInsertEndpointName = "razor/onAutoInsert";

public const string RazorValidateBreakpointRangeName = "razor/validateBreakpointRange";

public const string RazorPullDiagnosticEndpointName = "razor/pullDiagnostics";

public const string RazorReferencesEndpointName = "razor/references";
}
6 changes: 3 additions & 3 deletions src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/HtmlFormatter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public async Task<TextEdit[]> FormatAsync(
return Array.Empty<TextEdit>();
}

var @params = new VersionedDocumentFormattingParams()
var @params = new RazorDocumentFormattingParams()
{
TextDocument = new TextDocumentIdentifier
{
Expand All @@ -52,7 +52,7 @@ public async Task<TextEdit[]> FormatAsync(
};

var result = await _server.SendRequestAsync<DocumentFormattingParams, RazorDocumentFormattingResponse?>(
LanguageServerConstants.RazorDocumentFormattingEndpoint,
RazorLanguageServerCustomMessageTargets.RazorHtmlFormattingEndpoint,
@params,
cancellationToken);

Expand Down Expand Up @@ -80,7 +80,7 @@ public async Task<TextEdit[]> FormatOnTypeAsync(
};

var result = await _server.SendRequestAsync<RazorDocumentOnTypeFormattingParams, RazorDocumentFormattingResponse?>(
LanguageServerConstants.RazorDocumentOnTypeFormattingEndpoint,
RazorLanguageServerCustomMessageTargets.RazorHtmlOnTypeFormattingEndpoint,
@params,
cancellationToken);

Expand Down
4 changes: 2 additions & 2 deletions ...ting/VersionedDocumentFormattingParams.cs → ...rmatting/RazorDocumentFormattingParams.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
namespace Microsoft.AspNetCore.Razor.LanguageServer.Formatting;

[DataContract]
internal class VersionedDocumentFormattingParams : DocumentFormattingParams
internal class RazorDocumentFormattingParams : DocumentFormattingParams
{
[DataMember(Name = "_vs_hostDocumentVersion")]
[DataMember(Name = "hostDocumentVersion")]
public int HostDocumentVersion { get; set; }
}
18 changes: 0 additions & 18 deletions ...icrosoft.AspNetCore.Razor.LanguageServer/Formatting/RazorDocumentRangeFormattingParams.cs
View file
Open in desktop

This file was deleted.

107 changes: 89 additions & 18 deletions src/Razor/src/Microsoft.AspNetCore.Razor.VSCode/src/Formatting/FormattingHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,36 @@

import * as vscode from 'vscode';
import { RequestType } from 'vscode-languageclient';
import { IRazorDocument } from '../Document/IRazorDocument';
import { RazorDocumentManager } from '../Document/RazorDocumentManager';
import { RazorDocumentSynchronizer } from '../Document/RazorDocumentSynchronizer';
import { RazorLanguageServerClient } from '../RazorLanguageServerClient';
import { RazorLogger } from '../RazorLogger';
import { convertTextEditToSerializable, SerializableTextEdit } from '../RPC/SerializableTextEdit';
import { SerializableFormattingParams } from './SerializableFormattingParams';
import { SerializableFormattingResponse } from './SerializableFormattingResponse';
import { SerializableOnTypeFormattingParams } from './SerializableOnTypeFormattingParams';

export class FormattingHandler {
private static readonly provideFormattingEndpoint = 'textDocument/formatting';
private static readonly provideFormattingEndpoint = 'razor/htmlFormatting';
private static readonly provideOnTypeFormattingEndpoint = 'razor/htmlOnTypeFormatting';
private formattingRequestType: RequestType<SerializableFormattingParams, SerializableFormattingResponse, any> = new RequestType(FormattingHandler.provideFormattingEndpoint);
private onTypeFormattingRequestType: RequestType<SerializableOnTypeFormattingParams, SerializableFormattingResponse, any> = new RequestType(FormattingHandler.provideOnTypeFormattingEndpoint);
private emptyFormattingResponse = new SerializableFormattingResponse();

constructor(
private readonly documentManager: RazorDocumentManager,
private readonly documentSynchronizer: RazorDocumentSynchronizer,
private readonly serverClient: RazorLanguageServerClient,
private readonly logger: RazorLogger) { }

public register() {
return this.serverClient.onRequestWithParams<SerializableFormattingParams, SerializableFormattingResponse, any>(
public async register() {
await this.serverClient.onRequestWithParams<SerializableFormattingParams, SerializableFormattingResponse, any>(
this.formattingRequestType,
async (request, token) => this.provideFormatting(request, token));
await this.serverClient.onRequestWithParams<SerializableOnTypeFormattingParams, SerializableFormattingResponse, any>(
this.onTypeFormattingRequestType,
async (request, token) => this.provideOnTypeFormatting(request, token));
}

private async provideFormatting(
Expand All @@ -38,6 +47,12 @@ export class FormattingHandler {
return this.emptyFormattingResponse;
}

const textDocument = await vscode.workspace.openTextDocument(razorDocumentUri);
Copy link
Contributor

@allisonchou allisonchou May 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Also, is this line needed? I think documentManager.getDocument already opens the document for us

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 synchronized = await this.documentSynchronizer.trySynchronizeProjectedDocument(textDocument, razorDocument.csharpDocument, formattingParams.hostDocumentVersion, cancellationToken);
if (!synchronized) {
return this.emptyFormattingResponse;
}

const virtualHtmlUri = razorDocument.htmlDocument.uri;

const textEdits = await vscode.commands.executeCommand<vscode.TextEdit[]>(
Expand All @@ -49,24 +64,47 @@ export class FormattingHandler {
return this.emptyFormattingResponse;
}

const htmlDocText = razorDocument.htmlDocument.getContent();
const zeroBasedLineCount = this.countLines(htmlDocText);
const serializableTextEdits = Array<SerializableTextEdit>();
for (let textEdit of textEdits) {
// The below workaround is needed due to a bug on the HTML side where
// they'll sometimes send us an end position that exceeds the length
// of the document. Tracked by https://github.com/microsoft/vscode/issues/175298.
if (textEdit.range.end.line > zeroBasedLineCount) {
const lastLineLength = this.getLastLineLength(htmlDocText);
const updatedEndPosition = new vscode.Position(zeroBasedLineCount, lastLineLength);
const updatedRange = new vscode.Range(textEdit.range.start, updatedEndPosition);
textEdit = new vscode.TextEdit(updatedRange, textEdit.newText);
}
const serializableTextEdits = this.sanitizeTextEdits(razorDocument, textEdits);

return new SerializableFormattingResponse(serializableTextEdits);
} catch (error) {
this.logger.logWarning(`${FormattingHandler.provideFormattingEndpoint} failed with ${error}`);
}

return this.emptyFormattingResponse;
}

const serializableTextEdit = convertTextEditToSerializable(textEdit);
serializableTextEdits.push(serializableTextEdit);
private async provideOnTypeFormatting(
formattingParams: SerializableOnTypeFormattingParams,
cancellationToken: vscode.CancellationToken) {
try {
const razorDocumentUri = vscode.Uri.parse(formattingParams.textDocument.uri);
const razorDocument = await this.documentManager.getDocument(razorDocumentUri);
if (razorDocument === undefined) {
return this.emptyFormattingResponse;
}

const textDocument = await vscode.workspace.openTextDocument(razorDocumentUri);
Copy link
Contributor

Choose a reason for hiding this comment

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

(same question here as above)

const synchronized = await this.documentSynchronizer.trySynchronizeProjectedDocument(textDocument, razorDocument.csharpDocument, formattingParams.hostDocumentVersion, cancellationToken);
if (!synchronized) {
return this.emptyFormattingResponse;
}

const virtualHtmlUri = razorDocument.htmlDocument.uri;

const textEdits = await vscode.commands.executeCommand<vscode.TextEdit[]>(
'vscode.executeFormatOnTypeProvider',
virtualHtmlUri,
formattingParams.position,
formattingParams.ch,
formattingParams.options);

if (textEdits === undefined) {
return this.emptyFormattingResponse;
}

const serializableTextEdits = this.sanitizeTextEdits(razorDocument, textEdits);

return new SerializableFormattingResponse(serializableTextEdits);
} catch (error) {
this.logger.logWarning(`${FormattingHandler.provideFormattingEndpoint} failed with ${error}`);
Expand All @@ -75,6 +113,39 @@ export class FormattingHandler {
return this.emptyFormattingResponse;
}

private sanitizeTextEdits(razorDocument: IRazorDocument, textEdits: vscode.TextEdit[]) {
const htmlDocText = razorDocument.htmlDocument.getContent();
const zeroBasedLineCount = this.countLines(htmlDocText);
const serializableTextEdits = Array<SerializableTextEdit>();
for (let textEdit of textEdits) {
// The below workaround is needed due to a bug on the HTML side where
// they'll sometimes send us an end position that exceeds the length
// of the document. Tracked by https://github.com/microsoft/vscode/issues/175298.
if (textEdit.range.end.line > zeroBasedLineCount ||
textEdit.range.start.line > zeroBasedLineCount) {
const lastLineLength = this.getLastLineLength(htmlDocText);
const updatedPosition = new vscode.Position(zeroBasedLineCount, lastLineLength);

let start = textEdit.range.start;
let end = textEdit.range.end;
if (textEdit.range.start.line > zeroBasedLineCount) {
start = updatedPosition;
davidwengier marked this conversation as resolved.
Show resolved Hide resolved
}

if (textEdit.range.end.line > zeroBasedLineCount) {
end = updatedPosition;
}
davidwengier marked this conversation as resolved.
Show resolved Hide resolved

const updatedRange = new vscode.Range(start, end);
textEdit = new vscode.TextEdit(updatedRange, textEdit.newText);
}

const serializableTextEdit = convertTextEditToSerializable(textEdit);
serializableTextEdits.push(serializableTextEdit);
}
return serializableTextEdits;
}

private countLines(text: string) {
let lineCount = 0;
for (const i of text) {
Expand Down
15 changes: 0 additions & 15 deletions ...c/Microsoft.AspNetCore.Razor.VSCode/src/Formatting/RazorDocumentRangeFormattingRequest.ts
View file
Open in desktop

This file was deleted.

11 changes: 0 additions & 11 deletions .../Microsoft.AspNetCore.Razor.VSCode/src/Formatting/RazorDocumentRangeFormattingResponse.ts
View file
Open in desktop

This file was deleted.

19 changes: 0 additions & 19 deletions src/Razor/src/Microsoft.AspNetCore.Razor.VSCode/src/Formatting/RazorFormatOnTypeProvider.ts
View file
Open in desktop

This file was deleted.

Loading