From c56d34474d1c5e710375dcc88bb6c0b50c95c8d5 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Fri, 27 Apr 2018 16:40:21 -0700 Subject: [PATCH 1/7] Add saveas support for untitled files --- src/features/ExtensionCommands.ts | 121 ++++++++++++++++++++++-------- 1 file changed, 89 insertions(+), 32 deletions(-) diff --git a/src/features/ExtensionCommands.ts b/src/features/ExtensionCommands.ts index 049c3d23be..fd2fd91284 100644 --- a/src/features/ExtensionCommands.ts +++ b/src/features/ExtensionCommands.ts @@ -8,6 +8,7 @@ import * as path from "path"; import * as vscode from "vscode"; import { LanguageClient, NotificationType, Position, Range, RequestType } from "vscode-languageclient"; import { IFeature } from "../feature"; +import { Logger } from "../logging"; export interface IExtensionCommand { name: string; @@ -388,44 +389,100 @@ export class ExtensionCommandsFeature implements IFeature { return promise; } + /** + * Save a file, possibly to a new path. If the save is not possible, return a completed response + * @param saveFileDetails the object detailing the path of the file to save and optionally its new path to save to + */ private async saveFile(saveFileDetails: ISaveFileDetails): Promise { - - // If the file to save can't be found, just complete the request - if (!this.findTextDocument(this.normalizeFilePath(saveFileDetails.filePath))) { - return EditorOperationResponse.Completed; + // Try to interpret the filepath as a URI, defaulting to "file://" if we don't succeed + let currentFileUri: vscode.Uri; + if (saveFileDetails.filePath.startsWith("untitled") || saveFileDetails.filePath.startsWith("file")) { + currentFileUri = vscode.Uri.parse(saveFileDetails.filePath); + } else { + currentFileUri = vscode.Uri.file(saveFileDetails.filePath); } - // If no newFile is given, just save the current file - if (!saveFileDetails.newPath) { - const doc = await vscode.workspace.openTextDocument(saveFileDetails.filePath); - if (doc.isDirty) { - await doc.save(); - } - - return EditorOperationResponse.Completed; + let newFileAbsolutePath: string; + switch (currentFileUri.scheme) { + case "file": + // If the file to save can't be found, just complete the request + if (!this.findTextDocument(this.normalizeFilePath(currentFileUri.fsPath))) { + return EditorOperationResponse.Completed; + } + + // If no newFile is given, just save the current file + if (!saveFileDetails.newPath) { + const doc = await vscode.workspace.openTextDocument(currentFileUri.fsPath); + if (doc.isDirty) { + await doc.save(); + } + + return EditorOperationResponse.Completed; + } + + // Make sure we have an absolute path + if (path.isAbsolute(saveFileDetails.newPath)) { + newFileAbsolutePath = saveFileDetails.newPath; + } else { + // If not, interpret the path as relative to the current file + newFileAbsolutePath = path.resolve(path.dirname(currentFileUri.fsPath), saveFileDetails.newPath); + } + + await this.saveDocumentContentToAbsolutePath(currentFileUri, newFileAbsolutePath); + return EditorOperationResponse.Completed; + + case "untitled": + // We need a new name to save an untitled file + if (!saveFileDetails.newPath) { + return EditorOperationResponse.Completed; + } + + // Make sure we have an absolute path + if (path.isAbsolute(saveFileDetails.newPath)) { + newFileAbsolutePath = saveFileDetails.newPath; + } else { + // If not, interpret the path as relative to the workspace root + const workspaceRootUri = vscode.workspace.workspaceFolders[0].uri; + if (workspaceRootUri.scheme !== "file") { + // We don't support URI schemes for file saves + return EditorOperationResponse.Completed; + } + newFileAbsolutePath = path.resolve(workspaceRootUri.fsPath, saveFileDetails.newPath); + } + + await this.saveDocumentContentToAbsolutePath(currentFileUri, newFileAbsolutePath); + return EditorOperationResponse.Completed; + + default: + // TODO log something here about an unsupported URI scheme + return EditorOperationResponse.Completed; } + } - // Otherwise we want to save as a new file - - // First turn the path we were given into an absolute path - // Relative paths are interpreted as relative to the original file - const newFileAbsolutePath = path.isAbsolute(saveFileDetails.newPath) ? - saveFileDetails.newPath : - path.resolve(path.dirname(saveFileDetails.filePath), saveFileDetails.newPath); - - // Retrieve the text out of the current document - const oldDocument = await vscode.workspace.openTextDocument(saveFileDetails.filePath); - - // Write it to the new document path - fs.writeFileSync(newFileAbsolutePath, oldDocument.getText()); - - // Finally open the new document - const newFileUri = vscode.Uri.file(newFileAbsolutePath); - const newFile = await vscode.workspace.openTextDocument(newFileUri); - vscode.window.showTextDocument(newFile, { preview: true }); + /** + * Take a document available to vscode at the given URI and save it to the given absolute path + * @param documentUri the URI of the vscode document to save + * @param destinationAbsolutePath the absolute path to save the document contents to + */ + private async saveDocumentContentToAbsolutePath( + documentUri: vscode.Uri, + destinationAbsolutePath: string): Promise { + // Retrieve the text out of the current document + const oldDocument = await vscode.workspace.openTextDocument(documentUri); + + // Write it to the new document path + try { + fs.writeFileSync(destinationAbsolutePath, oldDocument.getText()); + } catch (e) { + const x = 1; + return; + } - return EditorOperationResponse.Completed; - } + // Finally open the new document + const newFileUri = vscode.Uri.file(destinationAbsolutePath); + const newFile = await vscode.workspace.openTextDocument(newFileUri); + vscode.window.showTextDocument(newFile, { preview: true }); + } private normalizeFilePath(filePath: string): string { const platform = os.platform(); From 150232a89e5e8f8df63abbd9949cc7a8aa7a71b1 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Sat, 28 Apr 2018 10:45:36 -0700 Subject: [PATCH 2/7] Improve comment about not saving to non-file URI schemes --- src/features/ExtensionCommands.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/features/ExtensionCommands.ts b/src/features/ExtensionCommands.ts index fd2fd91284..fee763d709 100644 --- a/src/features/ExtensionCommands.ts +++ b/src/features/ExtensionCommands.ts @@ -443,8 +443,8 @@ export class ExtensionCommandsFeature implements IFeature { } else { // If not, interpret the path as relative to the workspace root const workspaceRootUri = vscode.workspace.workspaceFolders[0].uri; + // We don't saving to a non-file URI-schemed workspace if (workspaceRootUri.scheme !== "file") { - // We don't support URI schemes for file saves return EditorOperationResponse.Completed; } newFileAbsolutePath = path.resolve(workspaceRootUri.fsPath, saveFileDetails.newPath); From 33afa115a239b627e03121a8c313db0f9ec6089e Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Sat, 28 Apr 2018 11:26:33 -0700 Subject: [PATCH 3/7] Improve logging in extension commands --- src/features/ExtensionCommands.ts | 18 ++++++++++++------ src/features/HelpCompletion.ts | 3 ++- src/main.ts | 2 +- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/features/ExtensionCommands.ts b/src/features/ExtensionCommands.ts index fee763d709..b9b5f3a4df 100644 --- a/src/features/ExtensionCommands.ts +++ b/src/features/ExtensionCommands.ts @@ -173,10 +173,10 @@ export class ExtensionCommandsFeature implements IFeature { private languageClient: LanguageClient; private extensionCommands: IExtensionCommand[] = []; - constructor() { + constructor(private log: Logger) { this.command = vscode.commands.registerCommand("PowerShell.ShowAdditionalCommands", () => { if (this.languageClient === undefined) { - // TODO: Log error message + this.log.writeAndShowError(`: Unable to instantiate; language client undefined.`); return; } @@ -425,7 +425,7 @@ export class ExtensionCommandsFeature implements IFeature { newFileAbsolutePath = saveFileDetails.newPath; } else { // If not, interpret the path as relative to the current file - newFileAbsolutePath = path.resolve(path.dirname(currentFileUri.fsPath), saveFileDetails.newPath); + newFileAbsolutePath = path.join(path.dirname(currentFileUri.fsPath), saveFileDetails.newPath); } await this.saveDocumentContentToAbsolutePath(currentFileUri, newFileAbsolutePath); @@ -447,14 +447,18 @@ export class ExtensionCommandsFeature implements IFeature { if (workspaceRootUri.scheme !== "file") { return EditorOperationResponse.Completed; } - newFileAbsolutePath = path.resolve(workspaceRootUri.fsPath, saveFileDetails.newPath); + newFileAbsolutePath = path.join(workspaceRootUri.fsPath, saveFileDetails.newPath); } await this.saveDocumentContentToAbsolutePath(currentFileUri, newFileAbsolutePath); return EditorOperationResponse.Completed; default: - // TODO log something here about an unsupported URI scheme + // Other URI schemes are not supported + const msg = JSON.stringify(saveFileDetails); + this.log.writeVerbose( + `: Saving a document with scheme '${currentFileUri.scheme}' ` + + `is currently unsupported. Message: '${msg}'`); return EditorOperationResponse.Completed; } } @@ -472,9 +476,11 @@ export class ExtensionCommandsFeature implements IFeature { // Write it to the new document path try { + // TODO: Change this to be asyncronous fs.writeFileSync(destinationAbsolutePath, oldDocument.getText()); } catch (e) { - const x = 1; + this.log.writeAndShowWarning(": " + + `Unable to save file to path '${destinationAbsolutePath}': ${e}`); return; } diff --git a/src/features/HelpCompletion.ts b/src/features/HelpCompletion.ts index 0dad935e71..d1d4811980 100644 --- a/src/features/HelpCompletion.ts +++ b/src/features/HelpCompletion.ts @@ -56,7 +56,8 @@ export class HelpCompletionFeature implements IFeature { public onEvent(changeEvent: TextDocumentChangeEvent): void { if (!(changeEvent && changeEvent.contentChanges)) { - this.log.write(`Bad TextDocumentChangeEvent message: ${JSON.stringify(changeEvent)}`); + this.log.writeWarning(": " + + `Bad TextDocumentChangeEvent message: ${JSON.stringify(changeEvent)}`); return; } diff --git a/src/main.ts b/src/main.ts index abc30b56e2..b5e7a5db2b 100644 --- a/src/main.ts +++ b/src/main.ts @@ -121,7 +121,7 @@ export function activate(context: vscode.ExtensionContext): void { new ShowHelpFeature(), new FindModuleFeature(), new PesterTestsFeature(sessionManager), - new ExtensionCommandsFeature(), + new ExtensionCommandsFeature(logger), new SelectPSSARulesFeature(), new CodeActionsFeature(), new NewFileOrProjectFeature(), From e050aab45f931406d7a380a15b79f0796fd82a82 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Mon, 30 Apr 2018 09:15:39 -0700 Subject: [PATCH 4/7] Improve logging code --- src/features/ExtensionCommands.ts | 7 ++++--- src/features/HelpCompletion.ts | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/features/ExtensionCommands.ts b/src/features/ExtensionCommands.ts index b9b5f3a4df..388952d763 100644 --- a/src/features/ExtensionCommands.ts +++ b/src/features/ExtensionCommands.ts @@ -176,7 +176,8 @@ export class ExtensionCommandsFeature implements IFeature { constructor(private log: Logger) { this.command = vscode.commands.registerCommand("PowerShell.ShowAdditionalCommands", () => { if (this.languageClient === undefined) { - this.log.writeAndShowError(`: Unable to instantiate; language client undefined.`); + this.log.writeAndShowError(`<${ExtensionCommandsFeature.name}>: ` + + "Unable to instantiate; language client undefined."); return; } @@ -457,7 +458,7 @@ export class ExtensionCommandsFeature implements IFeature { // Other URI schemes are not supported const msg = JSON.stringify(saveFileDetails); this.log.writeVerbose( - `: Saving a document with scheme '${currentFileUri.scheme}' ` + + `<${ExtensionCommandsFeature.name}>: Saving a document with scheme '${currentFileUri.scheme}' ` + `is currently unsupported. Message: '${msg}'`); return EditorOperationResponse.Completed; } @@ -479,7 +480,7 @@ export class ExtensionCommandsFeature implements IFeature { // TODO: Change this to be asyncronous fs.writeFileSync(destinationAbsolutePath, oldDocument.getText()); } catch (e) { - this.log.writeAndShowWarning(": " + + this.log.writeAndShowWarning(`<${ExtensionCommandsFeature.name}>: ` + `Unable to save file to path '${destinationAbsolutePath}': ${e}`); return; } diff --git a/src/features/HelpCompletion.ts b/src/features/HelpCompletion.ts index d1d4811980..0892c8552c 100644 --- a/src/features/HelpCompletion.ts +++ b/src/features/HelpCompletion.ts @@ -56,7 +56,7 @@ export class HelpCompletionFeature implements IFeature { public onEvent(changeEvent: TextDocumentChangeEvent): void { if (!(changeEvent && changeEvent.contentChanges)) { - this.log.writeWarning(": " + + this.log.writeWarning(`<${HelpCompletionFeature.name}>: ` + `Bad TextDocumentChangeEvent message: ${JSON.stringify(changeEvent)}`); return; } From 8f8bd5a405e7616e24397f34137703e0a74f8017 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Tue, 1 May 2018 11:46:47 -0700 Subject: [PATCH 5/7] Address PR feedback --- src/features/ExtensionCommands.ts | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/features/ExtensionCommands.ts b/src/features/ExtensionCommands.ts index 388952d763..946e2382e2 100644 --- a/src/features/ExtensionCommands.ts +++ b/src/features/ExtensionCommands.ts @@ -428,9 +428,7 @@ export class ExtensionCommandsFeature implements IFeature { // If not, interpret the path as relative to the current file newFileAbsolutePath = path.join(path.dirname(currentFileUri.fsPath), saveFileDetails.newPath); } - - await this.saveDocumentContentToAbsolutePath(currentFileUri, newFileAbsolutePath); - return EditorOperationResponse.Completed; + break; case "untitled": // We need a new name to save an untitled file @@ -444,15 +442,13 @@ export class ExtensionCommandsFeature implements IFeature { } else { // If not, interpret the path as relative to the workspace root const workspaceRootUri = vscode.workspace.workspaceFolders[0].uri; - // We don't saving to a non-file URI-schemed workspace + // We don't support saving to a non-file URI-schemed workspace if (workspaceRootUri.scheme !== "file") { return EditorOperationResponse.Completed; } newFileAbsolutePath = path.join(workspaceRootUri.fsPath, saveFileDetails.newPath); } - - await this.saveDocumentContentToAbsolutePath(currentFileUri, newFileAbsolutePath); - return EditorOperationResponse.Completed; + break; default: // Other URI schemes are not supported @@ -462,6 +458,10 @@ export class ExtensionCommandsFeature implements IFeature { `is currently unsupported. Message: '${msg}'`); return EditorOperationResponse.Completed; } + + await this.saveDocumentContentToAbsolutePath(currentFileUri, newFileAbsolutePath); + return EditorOperationResponse.Completed; + } /** @@ -478,7 +478,14 @@ export class ExtensionCommandsFeature implements IFeature { // Write it to the new document path try { // TODO: Change this to be asyncronous - fs.writeFileSync(destinationAbsolutePath, oldDocument.getText()); + await new Promise((resolve, reject) => { + fs.writeFile(destinationAbsolutePath, oldDocument.getText(), (err) => { + if (err) { + return reject(err); + } + return resolve(); + }); + }); } catch (e) { this.log.writeAndShowWarning(`<${ExtensionCommandsFeature.name}>: ` + `Unable to save file to path '${destinationAbsolutePath}': ${e}`); From 77debfb937d7827e2fc844ce0e00a58a4460f862 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Wed, 2 May 2018 11:52:08 -0700 Subject: [PATCH 6/7] Add helpful messages --- src/features/ExtensionCommands.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/features/ExtensionCommands.ts b/src/features/ExtensionCommands.ts index 946e2382e2..e13f676eb4 100644 --- a/src/features/ExtensionCommands.ts +++ b/src/features/ExtensionCommands.ts @@ -408,6 +408,7 @@ export class ExtensionCommandsFeature implements IFeature { case "file": // If the file to save can't be found, just complete the request if (!this.findTextDocument(this.normalizeFilePath(currentFileUri.fsPath))) { + this.log.writeAndShowError(`File to save not found: ${currentFileUri.fsPath}.`); return EditorOperationResponse.Completed; } @@ -417,7 +418,6 @@ export class ExtensionCommandsFeature implements IFeature { if (doc.isDirty) { await doc.save(); } - return EditorOperationResponse.Completed; } @@ -433,6 +433,10 @@ export class ExtensionCommandsFeature implements IFeature { case "untitled": // We need a new name to save an untitled file if (!saveFileDetails.newPath) { + // TODO: Create a class handle vscode warnings and errors so we can warn easily + // without logging + this.log.writeAndShowWarning( + "Cannot save untitled file. Try SaveAs(\"path/to/file.ps1\") instead."); return EditorOperationResponse.Completed; } From 52705b74c7722ca7ef8247a886e5d009c2930772 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Wed, 2 May 2018 14:09:56 -0700 Subject: [PATCH 7/7] Add logging and save failure warnings --- src/features/ExtensionCommands.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/features/ExtensionCommands.ts b/src/features/ExtensionCommands.ts index e13f676eb4..c5863cb939 100644 --- a/src/features/ExtensionCommands.ts +++ b/src/features/ExtensionCommands.ts @@ -444,10 +444,20 @@ export class ExtensionCommandsFeature implements IFeature { if (path.isAbsolute(saveFileDetails.newPath)) { newFileAbsolutePath = saveFileDetails.newPath; } else { + // In fresh contexts, workspaceFolders is not defined... + if (!vscode.workspace.workspaceFolders || vscode.workspace.workspaceFolders.length === 0) { + this.log.writeAndShowWarning("Cannot save file to relative path: no workspaces are open. " + + "Try saving to an absolute path, or open a workspace."); + return EditorOperationResponse.Completed; + } + // If not, interpret the path as relative to the workspace root const workspaceRootUri = vscode.workspace.workspaceFolders[0].uri; // We don't support saving to a non-file URI-schemed workspace if (workspaceRootUri.scheme !== "file") { + this.log.writeAndShowWarning( + "Cannot save untitled file to a relative path in an untitled workspace. " + + "Try saving to an absolute path or opening a workspace folder."); return EditorOperationResponse.Completed; } newFileAbsolutePath = path.join(workspaceRootUri.fsPath, saveFileDetails.newPath);