Skip to content
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

Have workspace.save and workspace.saveAs methods that return the URI #178713

Closed
karrtikr opened this issue Mar 30, 2023 · 11 comments · Fixed by #179091
Closed

Have workspace.save and workspace.saveAs methods that return the URI #178713

karrtikr opened this issue Mar 30, 2023 · 11 comments · Fixed by #179091
Assignees
Labels
api api-finalization feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders verification-needed Verification of issue is requested verified Verification succeeded workbench-untitled-editors Managing of untitled editors in workbench window

Comments

@karrtikr
Copy link
Contributor

karrtikr commented Mar 30, 2023

Does this issue occur when all extensions are disabled?: Yes

Version: 1.77.0-insider (user setup)
Commit: 7f329fe
Date: 2023-03-29T09:56:00.839Z
Electron: 19.1.11
Chromium: 102.0.5005.196
Node.js: 16.14.2
V8: 10.2.154.26-electron.0
OS: Windows_NT x64 10.0.22518
Sandboxed: Yes

Steps to Reproduce:

  1. Checkout the save() API here: https://code.visualstudio.com/api/references/vscode-api#TextDocument
  2. Have an untitled file open, and attempt to save the document using the API
  3. Textdocument.isClosed property is set to true

Also TextDocument.uri object is not updated to point to the updated saved URI. For Python extension it leads to issues like the following where wrong URI is sent to terminal, also notice that the file disappears:

screen-capture

@karrtikr
Copy link
Contributor Author

Follow up question: how can one know the URI of the file after save?

@karrtikr
Copy link
Contributor Author

Here's the workaround we're currently using:

const deferred = createDeferred<Uri>();
this.documentManager.onDidSaveTextDocument((e) => deferred.resolve(e.uri));
const commandManager = this.serviceContainer.get<ICommandManager>(ICommandManager);
await commandManager.executeCommand('workbench.action.files.save', file);
const savedFileUri = await deferred.promise;

@jrieken
Copy link
Member

jrieken commented Mar 31, 2023

The uri of a document doesn't change and it is expected that saving an untitled documented emits a close event for it. There is unfortunately no easy way to know what the new uri is, using the save or open events is the best you have. To double-check you can compare the contents

@bpasero
Copy link
Member

bpasero commented Mar 31, 2023

Related: #124117

We could still make this a bit easier on extensions and return a URI from a command such as workbench.action.files.save or workbench.action.files.saveAs. Would that be helping?

As for the underlying issue: code editor text models cannot change their model Uri. As such, we cannot transform an existing model from untitled to file URI. I still think it would be great to have though because there are other issues that originate from this limitation such as not being able to preserve the undo/redo buffer: #17977

@bpasero bpasero added api under-discussion Issue is under discussion for relevance, priority, approach workbench-untitled-editors Managing of untitled editors in workbench window labels Mar 31, 2023
@karrtikr
Copy link
Contributor Author

karrtikr commented Mar 31, 2023

That would be great.

As such, we cannot transform an existing model from untitled to file URI

Gotcha. Right now from a user's perspective TextDocument is able to update other properties such as isUntitled, isClosed but not the URI itself.

@karrtikr
Copy link
Contributor Author

karrtikr commented Mar 31, 2023

it is expected that saving an untitled documented emits a close event for it.

@jrieken Intiutively it feels like TextDocument.save should be the same as workbench.action.files.save, so I believe doc should be updated to add these nuances there:

image

Ideally, I would have expected it to return something like Uri | undefined instead of boolean to indicate the success.

@bpasero
Copy link
Member

bpasero commented Mar 31, 2023

Btw, it is possible for a user to cancel the dialog to pick a save location for an untitled file, so you almost need 3 states (error, success, cancelled). But anyway, API changes like this are not in the cards and would require new methods.

I can see if the command can return the URI.

@bpasero bpasero added feature-request Request for new features or functionality and removed api under-discussion Issue is under discussion for relevance, priority, approach labels Apr 3, 2023
@bpasero bpasero added this to the Backlog milestone Apr 3, 2023
@bpasero bpasero changed the title Text document save() API closes the Untitled document which is saved Return resulting URI from commands that save the active editor Apr 3, 2023
bpasero added a commit that referenced this issue Apr 20, 2023
…178713) (#179091)

* Return resulting `URI` from commands that save the active editor (fix #178713)

* 💄

* address feedback

* change to real proposed API

* cleanup
@VSCodeTriageBot VSCodeTriageBot added the unreleased Patch has not yet been released in VS Code Insiders label Apr 20, 2023
@bpasero bpasero removed this from the Backlog milestone Apr 21, 2023
@VSCodeTriageBot VSCodeTriageBot added insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Apr 21, 2023
@karrtikr
Copy link
Contributor Author

The APIs works perfectly for me 👍

@github-actions github-actions bot locked and limited conversation to collaborators Jun 4, 2023
@bpasero bpasero reopened this Oct 10, 2023
@VSCodeTriageBot VSCodeTriageBot removed the insiders-released Patch has been released in VS Code Insiders label Oct 10, 2023
@bpasero bpasero added api-finalization insiders-released Patch has been released in VS Code Insiders labels Oct 10, 2023
@bpasero bpasero modified the milestones: April 2023, November 2023 Oct 10, 2023
@bpasero bpasero removed their assignment Nov 6, 2023
@bpasero bpasero modified the milestones: November 2023, Backlog Nov 6, 2023
@bpasero bpasero self-assigned this Dec 7, 2023
@bpasero bpasero modified the milestones: Backlog, December 2023/January 2024 Dec 7, 2023
@bpasero bpasero closed this as completed in d5307c5 Dec 7, 2023
@VSCodeTriageBot VSCodeTriageBot added unreleased Patch has not yet been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Dec 7, 2023
@bpasero
Copy link
Member

bpasero commented Dec 8, 2023

@karrtikr the saveEditor API is has now been finalised and will be available in stable for our January release early February.

@bpasero bpasero added the verification-needed Verification of issue is requested label Jan 20, 2024
@bpasero
Copy link
Member

bpasero commented Jan 20, 2024

Verification:

@jrieken jrieken added the verified Verification succeeded label Jan 24, 2024
@aiday-mar aiday-mar added this to the December / January 2024 milestone Feb 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api api-finalization feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders verification-needed Verification of issue is requested verified Verification succeeded workbench-untitled-editors Managing of untitled editors in workbench window
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants