-
Notifications
You must be signed in to change notification settings - Fork 196
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
[VS Code] Fix various formatting issues #8318
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,8 +46,24 @@ export class FormattingHandler { | |
virtualHtmlUri, | ||
formattingParams.options); | ||
|
||
if (textEdits === undefined) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This check should (hopefully) resolve #7038 |
||
return this.emptyFormattingResponse; | ||
} | ||
|
||
const htmlDocText = razorDocument.htmlDocument.getContent(); | ||
const zeroBasedLineCount = this.countLines(htmlDocText); | ||
const serializableTextEdits = Array<SerializableTextEdit>(); | ||
for (const textEdit of textEdits) { | ||
for (let textEdit of textEdits) { | ||
// The below workaround is needed due to a bug on the HTML side where | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should resolve dotnet/vscode-csharp#5561 |
||
// 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 serializableTextEdit = convertTextEditToSerializable(textEdit); | ||
serializableTextEdits.push(serializableTextEdit); | ||
} | ||
|
@@ -59,4 +75,32 @@ export class FormattingHandler { | |
|
||
return this.emptyFormattingResponse; | ||
} | ||
|
||
private countLines(text: string) { | ||
let lineCount = 0; | ||
for (const i of text) { | ||
if (i === '\n') { | ||
lineCount++; | ||
} | ||
} | ||
|
||
return lineCount; | ||
} | ||
|
||
private getLastLineLength(text: string) { | ||
let currentLineLength = 0; | ||
for (let i = 0; i < text.length; i++) { | ||
// Take into account different line ending types ('\r\n' vs. '\n') | ||
if (i + 1 < text.length && text[i] === '\r' && text[i + 1] === '\n') { | ||
currentLineLength = 0; | ||
i++; | ||
} else if (text[i] === '\n') { | ||
currentLineLength = 0; | ||
} else { | ||
currentLineLength++; | ||
} | ||
} | ||
|
||
return currentLineLength; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
/* -------------------------------------------------------------------------------------------- | ||
* Copyright (c) Microsoft Corporation. All rights reserved. | ||
* Licensed under the MIT License. See License.txt in the project root for license information. | ||
* ------------------------------------------------------------------------------------------ */ | ||
|
||
import * as vscode from 'vscode'; | ||
|
||
export class RazorFormatOnTypeProvider | ||
implements vscode.OnTypeFormattingEditProvider { | ||
|
||
public provideOnTypeFormattingEdits( | ||
document: vscode.TextDocument, | ||
position: vscode.Position, | ||
character: string, | ||
formattingOptions: vscode.FormattingOptions, | ||
cancellationToken: vscode.CancellationToken): vscode.ProviderResult<vscode.TextEdit[]> { | ||
return new Array<vscode.TextEdit>(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,7 @@ import { RazorDocumentHighlightProvider } from './DocumentHighlight/RazorDocumen | |
import { reportTelemetryForDocuments } from './DocumentTelemetryListener'; | ||
import { FoldingRangeHandler } from './Folding/FoldingRangeHandler'; | ||
import { FormattingHandler } from './Formatting/FormattingHandler'; | ||
import { RazorFormatOnTypeProvider } from './Formatting/RazorFormatOnTypeProvider'; | ||
import { RazorFormattingFeature } from './Formatting/RazorFormattingFeature'; | ||
import { HostEventStream } from './HostEventStream'; | ||
import { RazorHoverProvider } from './Hover/RazorHoverProvider'; | ||
|
@@ -144,6 +145,7 @@ export async function activate(vscodeType: typeof vscodeapi, context: ExtensionC | |
documentManager, | ||
languageServiceClient, | ||
logger); | ||
const onTypeFormattingEditProvider = new RazorFormatOnTypeProvider(); | ||
|
||
localRegistrations.push( | ||
languageConfiguration.register(), | ||
|
@@ -177,6 +179,13 @@ export async function activate(vscodeType: typeof vscodeapi, context: ExtensionC | |
vscodeType.languages.registerDocumentHighlightProvider( | ||
RazorLanguage.id, | ||
documentHighlightProvider), | ||
// 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. | ||
vscodeType.languages.registerOnTypeFormattingEditProvider( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should resolve #8175 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why dont we support on type formatting? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just don't think we ever did in VS Code? We should support it eventually and I don't think (crossing fingers) it should be too difficult. However, it would definitely require a decent amount of validation so I left it out of this PR for now. I filed a tracking issue here: #8329 |
||
RazorLanguage.documentSelector, | ||
onTypeFormattingEditProvider, | ||
''), | ||
documentManager.register(), | ||
csharpFeature.register(), | ||
htmlFeature.register(), | ||
|
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.
Added improved error handling in our folding range provider just in case.