search

microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.01k stars 29.2k forks source link

File dialogs: validate `defaultFilePath` and `defaultFolderPath` for being valid #226581

Open babakfp opened 2 months ago

babakfp commented 2 months ago

I was able to reproduce this issue, https://github.com/microsoft/vscode/issues/210344.

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

Version: 1.92.2 (user setup) Commit: fee1edb8d6d72a0ddff41e5f71a671c23ed924b9 Date: 2024-08-14T17:29:30.058Z Electron: 30.1.2 ElectronBuildId: 9870757 Chromium: 124.0.6367.243 Node.js: 20.14.0 V8: 12.4.254.20-electron.0 OS: Windows_NT x64 10.0.22631

Steps to Reproduce:

https://github.com/user-attachments/assets/ee239dff-528e-40f0-9431-3d155ecddd7a

alexr00 commented 2 months ago

@bpasero this is probably coming from the history service. We could add a check to see if the file exists. Would you like to fix this in the history service or should I fix it the file dialog service?

bpasero commented 2 months ago

I believe the native dialog would refuse to accept an invalid path that does not exist, so I think the simple dialog could do the same. But I worry that we waste a fs call everytime the dialog opens...

alexr00 commented 2 months ago

Yes, I would need to make an fs call each time the dialog opens.

babakfp commented 2 months ago

Hi 👋

Sorry for the late response.

This is what I expect to happen:

  1. The name of the deleted folder not to get appended constantly.
  2. If the path doesn't exist, the suggested path to be what is in "files.dialog.defaultPath".
bpasero commented 2 months ago

I think its probably best to do some kind of validation at these methods:

https://github.com/microsoft/vscode/blob/4cec36a9d9bdfa4312921e8eb5e6b5c60f82c577/src/vs/workbench/services/dialogs/browser/abstractFileDialogService.ts#L60

https://github.com/microsoft/vscode/blob/4cec36a9d9bdfa4312921e8eb5e6b5c60f82c577/src/vs/workbench/services/dialogs/browser/abstractFileDialogService.ts#L79

And not do it in each component that asks for this path.

Treating as feature request.