Make SaveAs work for untitled files #1305
Conversation
| } | ||
| // 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.
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.
There was a problem hiding this comment.
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().
src/features/ExtensionCommands.ts
Outdated
| // 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.
Try <${ExtensionCommands.name}>: ...
src/features/ExtensionCommands.ts
Outdated
| // TODO: Change this to be asyncronous | ||
| fs.writeFileSync(destinationAbsolutePath, oldDocument.getText()); | ||
| } catch (e) { | ||
| this.log.writeAndShowWarning("<ExtensionCommands>: " + |
src/features/HelpCompletion.ts
Outdated
| @@ -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.
Try <${HelpCompletion.name}>: ...
|
Oops, clicked the wrong button there. |
|
@rkeithhill, I ended up having to use `<${ExtensionCommandsFeature.name}>`My ideal would be to have some kind of I'm assuming extension methods aren't a thing, particularly on interfaces, but we could maybe use a utility function. Ideally I can submit a new PR that improves extension logging. |
src/features/ExtensionCommands.ts
Outdated
| } 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 |
There was a problem hiding this comment.
nit pick: comment language:
"we don't saving" should be "we don't support saving"
src/features/ExtensionCommands.ts
Outdated
| } | ||
|
|
||
| await this.saveDocumentContentToAbsolutePath(currentFileUri, newFileAbsolutePath); | ||
| return EditorOperationResponse.Completed; |
There was a problem hiding this comment.
nitpick, pull these two lines out of catch to be more DRY? ;)
src/features/ExtensionCommands.ts
Outdated
| // 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.
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.
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?
There was a problem hiding this comment.
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.
Ah that's handy! Thanks @SeeminglyScience
There was a problem hiding this comment.
Type system wasn't quite happy with resolve there, but I've ended up with something similar.
|
Let me run the tests I've got above before we merge |
Fixes #1303.
This makes the
$psEditor.GetEditorContext().CurrentFile.SaveAs("blah.ps1")function work for files with an untitled schema.It also adds some logging to make it more diagnosable (because the logic for dealing with VSCode's crazy URIs is a bit hairy).
Some cases to test (on various platforms) before we merge:
Save()SaveAs()with a relative path (should save to path relative to the file).Save()(should not work)SaveAs()with a relative path (should save relative to the workspace root).SaveAs()to an absolute path.Open a new "untitled" workspaceOpen VSCode with no workspace open, open an untitled document, change language mode to PowerShell andSaveAs()to an absolute path (should work).workspace is "untitled"there is no workspace open).