-
Notifications
You must be signed in to change notification settings - Fork 511
Make SaveAs work for untitled files #1305
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
Changes from 3 commits
c56d344
150232a
33afa11
e050aab
8f8bd5a
77debfb
52705b7
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 |
---|---|---|
|
@@ -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; | ||
|
@@ -172,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(`<ExtensionCommands>: Unable to instantiate; language client undefined.`); | ||
return; | ||
} | ||
|
||
|
@@ -388,44 +389,106 @@ 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<EditorOperationResponse> { | ||
|
||
// 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.join(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; | ||
// We don't 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; | ||
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. nitpick, pull these two lines out of catch to be more DRY? ;) |
||
|
||
default: | ||
// Other URI schemes are not supported | ||
const msg = JSON.stringify(saveFileDetails); | ||
this.log.writeVerbose( | ||
`<ExtensionCommands>: Saving a document with scheme '${currentFileUri.scheme}' ` + | ||
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. Try 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. Ah nice! |
||
`is currently unsupported. Message: '${msg}'`); | ||
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<void> { | ||
// Retrieve the text out of the current document | ||
const oldDocument = await vscode.workspace.openTextDocument(documentUri); | ||
|
||
// Write it to the new document path | ||
try { | ||
// TODO: Change this to be asyncronous | ||
fs.writeFileSync(destinationAbsolutePath, oldDocument.getText()); | ||
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. Can we not already make this async? If it's too much work don't worry about it. Synchronously make the file, asynchronously add the content to the file? 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 looked into it, but 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. Sorry if this is already known, but FYI you can await inline promises, e.g: await new Promise<void>((resolve) => {
fs.writeFile(destinationAbsolutePath, oldDocument.getText(), resolve);
}); 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. Ah that's handy! Thanks @SeeminglyScience 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. Type system wasn't quite happy with |
||
} catch (e) { | ||
this.log.writeAndShowWarning("<ExtensionCommands>: " + | ||
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. Ditto above comment |
||
`Unable to save file to path '${destinationAbsolutePath}': ${e}`); | ||
return; | ||
} | ||
|
||
return EditorOperationResponse.Completed; | ||
} | ||
// Finally open the new document | ||
const newFileUri = vscode.Uri.file(destinationAbsolutePath); | ||
const newFile = await vscode.workspace.openTextDocument(newFileUri); | ||
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. Yea! Glad to see some async/await make its way into the extension. I find it easier to read than a bunch of 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. Yes, @SeeminglyScience set me on the right path there after I tried to put in a particularly twisted chain of |
||
vscode.window.showTextDocument(newFile, { preview: true }); | ||
} | ||
|
||
private normalizeFilePath(filePath: string): string { | ||
const platform = os.platform(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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("<HelpCompletion>: " + | ||
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. Try |
||
`Bad TextDocumentChangeEvent message: ${JSON.stringify(changeEvent)}`); | ||
return; | ||
} | ||
|
||
|
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.
nit pick: comment language:
"we don't saving" should be "we don't support saving"