Format via stdin instead of document file path

Fixes double save issues as now we don't have to save the document to do formatting. Also fixes save on "Format Document" (now no save is done). Related to microsoft#1037
wader · Jul 5, 2017 · 5b87490 · 5b87490
1 parent 8344b4c
commit 5b87490
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 31 deletions.
diff --git a/src/goFormat.ts b/src/goFormat.ts
@@ -16,6 +16,7 @@ export class Formatter {
 	public formatDocument(document: vscode.TextDocument): Thenable<vscode.TextEdit[]> {
 		return new Promise((resolve, reject) => {
 			let filename = document.fileName;
+			let documentText = document.getText();
 			let formatTool = vscode.workspace.getConfiguration('go')['formatTool'] || 'goreturns';
 			let formatCommandBinPath = getBinPath(formatTool);
 			let formatFlags = vscode.workspace.getConfiguration('go')['formatFlags'] || [];
@@ -29,7 +30,7 @@ export class Formatter {
 			}
 			let t0 = Date.now();
 			let env = getToolsEnvVars();
-			cp.execFile(formatCommandBinPath, [...formatFlags, filename], {env}, (err, stdout, stderr) => {
+			let process = cp.execFile(formatCommandBinPath, [...formatFlags], {env}, (err, stdout, stderr) => {
 				try {
 					if (err && (<any>err).code === 'ENOENT') {
 						promptForMissingTool(formatTool);
@@ -41,7 +42,7 @@ export class Formatter {
 					};
 
 					let textEdits: vscode.TextEdit[] = [];
-					let filePatch = canFormatToolUseDiff ? getEditsFromUnifiedDiffStr(stdout)[0] : getEdits(filename, document.getText(), stdout);
+					let filePatch = canFormatToolUseDiff ? getEditsFromUnifiedDiffStr(stdout)[0] : getEdits(filename, documentText, stdout);
 
 					filePatch.edits.forEach((edit) => {
 						textEdits.push(edit.apply());
@@ -54,6 +55,7 @@ export class Formatter {
 					reject('Internal issues while getting diff from formatted content');
 				}
 			});
+			process.stdin.end(documentText);
 		});
 	}
 }
@@ -66,9 +68,7 @@ export class GoDocumentFormattingEditProvider implements vscode.DocumentFormatti
 	}
 
 	public provideDocumentFormattingEdits(document: vscode.TextDocument, options: vscode.FormattingOptions, token: vscode.CancellationToken): Thenable<vscode.TextEdit[]> {
-		return document.save().then(() => {
-			return this.formatter.formatDocument(document);
-		});
+		return this.formatter.formatDocument(document);
 	}
 }
 

diff --git a/src/goMain.ts b/src/goMain.ts
@@ -95,6 +95,7 @@ export function activate(ctx: vscode.ExtensionContext): void {
 	vscode.window.onDidChangeActiveTextEditor(getCodeCoverage, null, ctx.subscriptions);
 	vscode.workspace.onDidChangeTextDocument(parseLiveFile, null, ctx.subscriptions);
 
+	formatOnSaveWatcher(ctx.subscriptions);
 	startBuildOnSaveWatcher(ctx.subscriptions);
 
 	ctx.subscriptions.push(vscode.commands.registerCommand('go.gopath', () => {
@@ -302,41 +303,29 @@ function runBuilds(document: vscode.TextDocument, goConfig: vscode.WorkspaceConf
 	});
 }
 
-function startBuildOnSaveWatcher(subscriptions: vscode.Disposable[]) {
-
-	// TODO: This is really ugly.  I'm not sure we can do better until
-	// Code supports a pre-save event where we can do the formatting before
-	// the file is written to disk.
-	let ignoreNextSave = new WeakSet<vscode.TextDocument>();
-
-	vscode.workspace.onDidSaveTextDocument(document => {
-		if (document.languageId !== 'go' || ignoreNextSave.has(document)) {
+function formatOnSaveWatcher(subscriptions: vscode.Disposable[]) {
+	vscode.workspace.onWillSaveTextDocument(event => {
+		let document = event.document;
+		if (document.languageId !== 'go') {
 			return;
 		}
 		let goConfig = vscode.workspace.getConfiguration('go');
 		let textEditor = vscode.window.activeTextEditor;
-		let formatPromise: PromiseLike<void> = Promise.resolve();
 		if (goConfig['formatOnSave'] && textEditor.document === document) {
 			let formatter = new Formatter();
-			formatPromise = formatter.formatDocument(document).then(edits => {
-				return textEditor.edit(editBuilder => {
-					edits.forEach(edit => editBuilder.replace(edit.range, edit.newText));
-				});
-			}).then(applied => {
-				ignoreNextSave.add(document);
-				return document.save();
-			}).then(() => {
-				ignoreNextSave.delete(document);
-			}, () => {
-				// Catch any errors and ignore so that we still trigger
-				// the file save.
-			});
+			event.waitUntil(formatter.formatDocument(document));
 		}
-		formatPromise.then(() => {
-			runBuilds(document, goConfig);
-		});
 	}, null, subscriptions);
+}
 
+function startBuildOnSaveWatcher(subscriptions: vscode.Disposable[]) {
+	vscode.workspace.onDidSaveTextDocument(document => {
+		if (document.languageId !== 'go') {
+			return;
+		}
+		let goConfig = vscode.workspace.getConfiguration('go');
+		runBuilds(document, goConfig);
+	}, null, subscriptions);
 }
 
 function sendTelemetryEventForConfig(goConfig: vscode.WorkspaceConfiguration) {