Skip to content

Find nearest existing resource for showOpenDialog in interactive forms - #6175

Merged
DanTup merged 1 commit into
masterfrom
fix-existing-file-location
Sep 15, 2026
Merged

DanTup merged 1 commit into
masterfrom
fix-existing-file-location

Conversation

@DanTup

@DanTup DanTup commented Sep 7, 2026

Copy link
Copy Markdown
Member

VS Code ignores the defaultUri passed to showOpenDialog if it does not exist. For interactive forms, we have a single default value for open+save dialogs. In the case you choose Existing File, it's very likely the default value (which may be a suggested target filename) does not exist, and this results in VS Code just selecting the last folder you had open.

By walking up the tree changing the default value to the first existing parent, we will start in that folder instead.

Fixes #6164

@DanTup DanTup added this to the v3.144.0 milestone Sep 7, 2026
@DanTup DanTup added is enhancement An enhancement or improvement that should be listed in release notes but is not a bug fix. in editor Relates to code editing or language features labels Sep 7, 2026
@DanTup
DanTup requested a lite review from Copilot September 7, 2026 18:10
@DanTup DanTup added the in commands Relates to commands (usually invoked from the command Palette) label Sep 7, 2026

This comment was marked as resolved.

VS Code ignores the `defaultUri` passed to `showOpenDialog` if it does not exist. For interactive forms, we have a single default value for open+save dialogs. In the case you choose Existing File, it's very likely the default value (which may be a suggested target filename) does not exist, and this results in VS Code just selecting the last folder you had open.

By walking up the tree changing the default value to the first existing parent, we will start in that folder instead.

Fixes #6164
@DanTup
DanTup force-pushed the fix-existing-file-location branch from ecfa9d8 to 0322863 Compare September 7, 2026 18:16
@DanTup

DanTup commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

@h9jiang do you have any thought/opinions on doing this?

@codecov

codecov Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.72727% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.77%. Comparing base (1451591) to head (0322863).

Files with missing lines Patch % Lines
src/extension/analysis/form.ts 72.72% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6175   +/-   ##
=======================================
  Coverage   70.77%   70.77%           
=======================================
  Files         170      170           
  Lines       13158    13169   +11     
  Branches     2649     2651    +2     
=======================================
+ Hits         9313     9321    +8     
- Misses       3324     3326    +2     
- Partials      521      522    +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@h9jiang h9jiang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this is a great idea, the destination and the source (default value provided by language server) normally has certain proximity. (I assumed that the language server will provide a uri that is close to the src) And it is also intuitive for user to navigate starting from the source to the dest like cd ../../something/new/foo.dart.

In addition, if the language server does not provide a default, should we consider setting the default to the workspace root?

@DanTup

DanTup commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

@h9jiang

In addition, if the language server does not provide a default, should we consider setting the default to the workspace root?

I don't know if that would necessarily be better than the last one you picked. If a server doesn't provide defaults and you are refactoring in some specific folder, the last one might be a better default. I think that's different to this case where the server is providing a default path, but VS Code is completely ignoring it because it doesn't already exist, so we're just trying to get as close to the one the server provided as we can.

I don't feel strongly though, hopefully in general servers will provide default values. I'm happy to go with your preference (also cc @bwilkerson in case he has thoughts or a preference).

@DanTup

DanTup commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

I'll land this as-is for now, and file a separate PR if we want to make the other change.

(or not... as GitHub seems to be falling over....)

@DanTup
DanTup merged commit 323dd39 into master Sep 15, 2026
24 checks passed
@DanTup
DanTup deleted the fix-existing-file-location branch September 15, 2026 11:17
@bwilkerson

Copy link
Copy Markdown

In addition, if the language server does not provide a default, should we consider setting the default to the workspace root?

Which workspace root? I usually have several, by which I mean around 25) folders open in my workspace. I don't know what the workspace root would be in that case.

I don't know if that would necessarily be better than the last one you picked.

Most applications that I can think of will open the file dialog to the last directory used in a file dialog. This often works well. If I'm opening a file there's a higher probability that I want to open a file near the previously opened file. The same is true for files I'm creating. For example, consider the 'move declarations to file' refactoring. If I'm using it to break up a large file then chances are good that I want to create multiple new files in the same directory.

@h9jiang

h9jiang commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Which workspace root?

I was more imagining the current workspace root that is aware by the extension. I think there are several natural ways for user to navigate through their current workspace. (their current repo).

  • The recommended uri: provided by the language server. (The best guess by language server).
  • The "source" uri: provided by the language server, in this case, the user navigate through the workspace by cd ../../../path/to/new/location. The user will specify the relative path from the source uri to the destination uri by go to the parent dir & parent's parent dir & enter a sub dir...
  • The current workspace root: in this case, because the language server did not provide a default value, so we ask the user to start with the workspace root or the last opened file recorded. So the user does not start with ~, which maybe far away from the workspace root.

I think previously recorded value makes a lot of sense and I think that might be very good solution as complicated refactoring may fail in the first time (like some merge conflicts) so the user might have to try again later.

@DanTup

DanTup commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

I was more imagining the current workspace root that is aware by the extension

LSP supports multiple workspace folders (InitializeParams.workspaceFolders) and I'm not sure from a bare command we know which one the user is in the context of. We could try to infer this from the active document, although even getting the active document in VS Code is a little complicated (because many non-editors like Output windows can be the window.activeTextEditor!).

I think previously recorded value makes a lot of sense and I think that might be very good solution as complicated refactoring may fail in the first time (like some merge conflicts) so the user might have to try again later.

In the case where the server didn't provide a default value, that's what we have today. But in the case where the server does provide a default, this PR tries to get us as close to that location as possible. This means if you retry a refactor, you would go back to the default and not the one that you'd previously selected (because of this PR). If we revert this PR, you'd get the same value as you previously selected on a retry, but you'd also get this behaviour for the first attempt, which could be a completely unrelated folder (for example if you were just working in a different project - if you haven't opened any folders via the Open Dialog at all in this session).

@h9jiang

h9jiang commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

In the case where the server didn't provide a default value, that's what we have today.

Sorry I missed that. I think this makes total sense. Thanks for the clarification.

@DanTup

DanTup commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

In the case where the server didn't provide a default value, that's what we have today.

Sorry I missed that.

To be clear (since it might not have been obvious), that behaviour comes from VS Code. The starting folder appears to be decided like:

  • if the extension provided a default URI and it exists, use that
  • otherwise, if the user has used the Open dialog in this session, use that
  • otherwise, do something unexpected 😄 (for me, it seems to open my home directory, which on Windows is not where I actually keep anything)... I don't know if this behaviour is from VS Code or the platform though

Other editors might behave differently here, but of course we can't influence them from this code, it would have to be handled in their own plugins accordingly.

gopherbot pushed a commit to golang/vscode-go that referenced this pull request Sep 15, 2026
…alog

VS Code ignores the defaultUri passed to showOpenDialog if it
does not exist. For interactive forms, we have a single default value
for open+save dialogs. In the case you choose Existing File, it's
very likely the default value (which may be a suggested target
filename) does not exist, and this results in VS Code just selecting
the last folder you had open.

By walking up the tree changing the default value to the first
existing parent, we will start in that folder instead.

This CL is a sync from:
Dart-Code/Dart-Code#6175

Change-Id: I3b254b29c4b0b091b0095a25afabc2cc366bde1b
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/832844
LUCI-TryBot-Result: [email protected] <[email protected]>
Reviewed-by: Alex Putman <[email protected]>
Auto-Submit: Hongxiang Jiang <[email protected]>
Reviewed-by: Danny Tuppeny <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in commands Relates to commands (usually invoked from the command Palette) in editor Relates to code editing or language features is enhancement An enhancement or improvement that should be listed in release notes but is not a bug fix.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using Move Top Level refactor doesn't override the suggested directory

4 participants