Skip to content

Partially move document formatting to server side #874

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

Merged
merged 6 commits into from
Jun 14, 2017
Merged

Partially move document formatting to server side #874

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
31db2fd
Manually send textdocument/formatting request
Jun 13, 2017
9976c97
Add method to get formatting options from configuration
Jun 13, 2017
d56694f
Manually send textdocument/rangeFormatting request
Jun 13, 2017
3935924
Use range formatting for full document formatting
Jun 13, 2017
d5b153e
Format DocumentFormatter.ts file
Jun 13, 2017
601b26f
Remove unused items from DocumentFormatter.ts
Jun 13, 2017
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
277 changes: 46 additions & 231 deletions src/features/DocumentFormatter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,20 @@ import {
TextEditor,
TextLine
} from 'vscode';
import { LanguageClient, RequestType } from 'vscode-languageclient';
import {
LanguageClient,
RequestType,
DocumentFormattingRequest,
DocumentRangeFormattingParams,
DocumentRangeFormattingRequest
} from 'vscode-languageclient';
import { TextDocumentIdentifier } from "vscode-languageserver-types";
import Window = vscode.window;
import { IFeature } from '../feature';
import * as Settings from '../settings';
import * as Utils from '../utils';
import * as AnimatedStatusBar from '../controls/animatedStatusBar';

export namespace ScriptFileMarkersRequest {
export const type = new RequestType<any, any, void, void>("powerShell/getScriptFileMarkers");
}

export namespace ScriptRegionRequest {
export const type = new RequestType<any, any, void, void>("powerShell/getScriptRegion");
}
Expand All @@ -43,29 +46,6 @@ interface ScriptRegionRequestResult {
scriptRegion: ScriptRegion;
}

// TODO move some of the common interface to a separate file?
interface ScriptFileMarkersRequestParams {
fileUri: string;
settings: any;
}

interface ScriptFileMarkersRequestResultParams {
markers: ScriptFileMarker[];
}

interface ScriptFileMarker {
message: string;
level: ScriptFileMarkerLevel;
scriptRegion: ScriptRegion;
correction: MarkerCorrection;
}

enum ScriptFileMarkerLevel {
Information = 0,
Warning,
Error
}

interface ScriptRegion {
file: string;
text: string;
Expand All @@ -77,11 +57,6 @@ interface ScriptRegion {
endOffset: number;
}

interface MarkerCorrection {
name: string;
edits: ScriptRegion[];
}

function toRange(scriptRegion: ScriptRegion): vscode.Range {
return new vscode.Range(
scriptRegion.startLineNumber - 1,
Expand All @@ -94,24 +69,6 @@ function toOneBasedPosition(position: Position): Position {
return position.translate({ lineDelta: 1, characterDelta: 1 });
}

function editComparer(leftOperand: ScriptRegion, rightOperand: ScriptRegion): number {
if (leftOperand.startLineNumber < rightOperand.startLineNumber) {
return -1;
} else if (leftOperand.startLineNumber > rightOperand.startLineNumber) {
return 1;
} else {
if (leftOperand.startColumnNumber < rightOperand.startColumnNumber) {
return -1;
}
else if (leftOperand.startColumnNumber > rightOperand.startColumnNumber) {
return 1;
}
else {
return 0;
}
}
}

class DocumentLocker {
private lockedDocuments: Object;

Expand Down Expand Up @@ -161,42 +118,27 @@ class DocumentLocker {
}

class PSDocumentFormattingEditProvider implements
DocumentFormattingEditProvider,
DocumentRangeFormattingEditProvider,
OnTypeFormattingEditProvider {
DocumentFormattingEditProvider,
DocumentRangeFormattingEditProvider,
OnTypeFormattingEditProvider {

private static documentLocker = new DocumentLocker();
private static statusBarTracker = new Object();
private languageClient: LanguageClient;
private lineDiff: number;

// The order in which the rules will be executed starting from the first element.
private readonly ruleOrder: string[] = [
"PSPlaceCloseBrace",
"PSPlaceOpenBrace",
"PSUseConsistentWhitespace",
"PSUseConsistentIndentation",
"PSAlignAssignmentStatement"]

// Allows edits to be undone and redone is a single step.
// It is usefuld to have undo stops after every edit while debugging
// hence we keep this as an option but set it true by default.
private aggregateUndoStop: boolean;

private get emptyPromise(): Promise<TextEdit[]> {
return Promise.resolve(TextEdit[0]);
}

constructor(aggregateUndoStop = true) {
this.aggregateUndoStop = aggregateUndoStop;
this.lineDiff = 0;
constructor() {
}

provideDocumentFormattingEdits(
document: TextDocument,
options: FormattingOptions,
token: CancellationToken): TextEdit[] | Thenable<TextEdit[]> {
return this.provideDocumentRangeFormattingEdits(document, null, options, token);
}
}

provideDocumentRangeFormattingEdits(
document: TextDocument,
Expand All @@ -215,7 +157,32 @@ class PSDocumentFormattingEditProvider implements
return this.emptyPromise;
}

let textEdits: Thenable<TextEdit[]> = this.executeRulesInOrder(editor, range, options, 0);

// somehow range object gets serialized to an array of Position objects,
// so we need to use the object literal syntax to initialize it.
let rangeParam = null;
if (range != null) {
rangeParam = {
start: {
line: range.start.line,
character: range.start.character
},
end: {
line: range.end.line,
character: range.end.character
}
};
};

let requestParams: DocumentRangeFormattingParams = {
textDocument: TextDocumentIdentifier.create(document.uri.toString()),
range: rangeParam,
options: this.getEditorSettings()
};

let textEdits = this.languageClient.sendRequest(
DocumentRangeFormattingRequest.type,
requestParams);
this.lockDocument(document, textEdits);
PSDocumentFormattingEditProvider.showStatusBar(document, textEdits);
return textEdits;
Expand Down Expand Up @@ -268,12 +235,6 @@ class PSDocumentFormattingEditProvider implements
});
}

private snapRangeToEdges(range: Range, document: TextDocument): Range {
return range.with({
start: range.start.with({ character: 0 }),
end: document.lineAt(range.end.line).range.end });
}

private getEditor(document: TextDocument): TextEditor {
return Window.visibleTextEditors.find((e, n, obj) => { return e.document === document; });
}
Expand All @@ -286,158 +247,12 @@ class PSDocumentFormattingEditProvider implements
PSDocumentFormattingEditProvider.documentLocker.lock(document, unlockWhenDone);
}

private executeRulesInOrder(
Copy link
Contributor

Choose a reason for hiding this comment

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

So happy this is gone now!

editor: TextEditor,
range: Range,
options: FormattingOptions,
index: number): Thenable<TextEdit[]> {
if (this.languageClient !== null && index < this.ruleOrder.length) {
let rule: string = this.ruleOrder[index];
let uniqueEdits: ScriptRegion[] = [];
let document: TextDocument = editor.document;
let edits: ScriptRegion[];

return this.languageClient.sendRequest(
ScriptFileMarkersRequest.type,
{
fileUri: document.uri.toString(),
settings: this.getSettings(rule)
})
.then((result: ScriptFileMarkersRequestResultParams) => {
edits = result.markers.map(marker => { return marker.correction.edits[0]; });

// sort in decending order of the edits
edits.sort((left: ScriptRegion, right: ScriptRegion) => {
return -1 * editComparer(left, right);
});


// we need to update the range as the edits might
// have changed the original layout
if (range !== null) {
if (this.lineDiff !== 0) {
range = range.with({ end: range.end.translate({ lineDelta: this.lineDiff }) });
}

// extend the range such that it starts at the first character of the
// start line of the range.
range = this.snapRangeToEdges(range, document);

// filter edits that are contained in the input range
edits = edits.filter(edit => range.contains(toRange(edit).start));
}

// We cannot handle multiple edits at the same point hence we
// filter the markers so that there is only one edit per region
if (edits.length > 0) {
uniqueEdits.push(edits[0]);
for (let edit of edits.slice(1)) {
let lastEdit: ScriptRegion = uniqueEdits[uniqueEdits.length - 1];
if (lastEdit.startLineNumber !== edit.startLineNumber
|| (edit.startColumnNumber + edit.text.length) < lastEdit.startColumnNumber) {
uniqueEdits.push(edit);
}
}
}

// reset line difference to 0
this.lineDiff = 0;

// we do not return a valid array because our text edits
// need to be executed in a particular order and it is
// easier if we perform the edits ourselves
return this.applyEdit(editor, uniqueEdits, 0, index);
})
.then(() => {
// execute the same rule again if we left out violations
// on the same line
let newIndex: number = index + 1;
if (uniqueEdits.length !== edits.length) {
newIndex = index;
}

return this.executeRulesInOrder(editor, range, options, newIndex);
});
} else {
return this.emptyPromise;
}
}

private applyEdit(
editor: TextEditor,
edits: ScriptRegion[],
markerIndex: number,
ruleIndex: number): Thenable<void> {
if (markerIndex >= edits.length) {
return;
}

let undoStopAfter = !this.aggregateUndoStop || (ruleIndex === this.ruleOrder.length - 1 && markerIndex === edits.length - 1);
let undoStopBefore = !this.aggregateUndoStop || (ruleIndex === 0 && markerIndex === 0);
let edit: ScriptRegion = edits[markerIndex];
let editRange: Range = toRange(edit);


// accumulate the changes in number of lines
// get the difference between the number of lines in the replacement text and
// that of the original text
this.lineDiff += this.getNumLines(edit.text) - (editRange.end.line - editRange.start.line + 1);
return editor.edit((editBuilder) => {
editBuilder.replace(
editRange,
edit.text);
},
{
undoStopAfter: undoStopAfter,
undoStopBefore: undoStopBefore
}).then((isEditApplied) => {
return this.applyEdit(editor, edits, markerIndex + 1, ruleIndex);
}); // TODO handle rejection
}

private getNumLines(text: string): number {
return text.split(/\r?\n/).length;
}

private getSettings(rule: string): any {
let psSettings: Settings.ISettings = Settings.load(Utils.PowerShellLanguageId);
let ruleSettings = new Object();
ruleSettings["Enable"] = true;

switch (rule) {
case "PSPlaceOpenBrace":
ruleSettings["OnSameLine"] = psSettings.codeFormatting.openBraceOnSameLine;
ruleSettings["NewLineAfter"] = psSettings.codeFormatting.newLineAfterOpenBrace;
ruleSettings["IgnoreOneLineBlock"] = psSettings.codeFormatting.ignoreOneLineBlock;
break;

case "PSPlaceCloseBrace":
ruleSettings["IgnoreOneLineBlock"] = psSettings.codeFormatting.ignoreOneLineBlock;
ruleSettings["NewLineAfter"] = psSettings.codeFormatting.newLineAfterCloseBrace;
break;

case "PSUseConsistentIndentation":
ruleSettings["IndentationSize"] = vscode.workspace.getConfiguration("editor").get<number>("tabSize");
break;

case "PSUseConsistentWhitespace":
ruleSettings["CheckOpenBrace"] = psSettings.codeFormatting.whitespaceBeforeOpenBrace;
ruleSettings["CheckOpenParen"] = psSettings.codeFormatting.whitespaceBeforeOpenParen;
ruleSettings["CheckOperator"] = psSettings.codeFormatting.whitespaceAroundOperator;
ruleSettings["CheckSeparator"] = psSettings.codeFormatting.whitespaceAfterSeparator;
break;

case "PSAlignAssignmentStatement":
ruleSettings["CheckHashtable"] = psSettings.codeFormatting.alignPropertyValuePairs;
break;

default:
break;
}

let settings: Object = new Object();
settings[rule] = ruleSettings;
return settings;
private getEditorSettings(): { insertSpaces: boolean, tabSize: number } {
let editorConfiguration = vscode.workspace.getConfiguration("editor");
return {
insertSpaces: editorConfiguration.get<boolean>("insertSpaces"),
tabSize: editorConfiguration.get<number>("tabSize")
};
}

private static showStatusBar(document: TextDocument, hideWhenDone: Thenable<any>): void {
Expand Down