Skip to content

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

Merged
merged 7 commits into from
May 2, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
132 changes: 98 additions & 34 deletions src/features/ExtensionCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -172,10 +173,11 @@ 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(`<${ExtensionCommandsFeature.name}>: ` +
"Unable to instantiate; language client undefined.");
return;
}

Expand Down Expand Up @@ -388,44 +390,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
Copy link
Member

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"

if (workspaceRootUri.scheme !== "file") {
return EditorOperationResponse.Completed;
}
newFileAbsolutePath = path.join(workspaceRootUri.fsPath, saveFileDetails.newPath);
}

await this.saveDocumentContentToAbsolutePath(currentFileUri, newFileAbsolutePath);
return EditorOperationResponse.Completed;
Copy link
Member

Choose a reason for hiding this comment

The 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(
`<${ExtensionCommandsFeature.name}>: Saving a document with scheme '${currentFileUri.scheme}' ` +
`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());
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked into it, but writeFile is an older node API that wants a callback. I have been contemplating turning the whole call into a Promise though. Thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

The 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);
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah that's handy! Thanks @SeeminglyScience

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Type system wasn't quite happy with resolve there, but I've ended up with something similar.

} catch (e) {
this.log.writeAndShowWarning(`<${ExtensionCommandsFeature.name}>: ` +
`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);
Copy link
Contributor

Choose a reason for hiding this comment

The 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 .then() continuations. That's probably the C# dev in me coming out but I think TS/JS devs will like it too.

Copy link
Contributor Author

@rjmholt rjmholt Apr 30, 2018

Choose a reason for hiding this comment

The 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 .then()s when I first tried SaveAs().

vscode.window.showTextDocument(newFile, { preview: true });
}

private normalizeFilePath(filePath: string): string {
const platform = os.platform();
Expand Down
3 changes: 2 additions & 1 deletion src/features/HelpCompletion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(`<${HelpCompletionFeature.name}>: ` +
`Bad TextDocumentChangeEvent message: ${JSON.stringify(changeEvent)}`);
return;
}

Expand Down
2 changes: 1 addition & 1 deletion src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down