-
Notifications
You must be signed in to change notification settings - Fork 521
Support ~, ./ and named workspace folders in cwd
#4687
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
Conversation
09cfa4b to
3ef57f9
Compare
I definitely thought we had an issue (possibly closed) for this...it's come up at least, a number of times. @JustinGrote any idea where that went?
c5a309c to
3bb69d7
Compare
This makes them way less annoying. I'm ok taking a dependency on untildify as it's a very simple package, and the Python extension for VS Code also uses it. However, it must remain at v4.0.0, as the latest version, v5.0.0, is pure ESM and therefore cannot be loaded by VS Code. https://github.com/sindresorhus/untildify/releases/tag/v5.0.0
Those were overdue!
So we can remove weirdly placed `validateCwdSetting` calls and instead use it exactly when required.
This cleans up how we remember which workspace the user chose, and sets us up to save that information not in the `cwd` setting. Refactors and fixes `validateCwdSetting` to treat the empty string as undefined for `cwd`.
3bb69d7 to
e63bdc5
Compare
I definitely thought we had an issue (possibly closed) for this...it's come up at least, a number of times. @JustinGrote any idea where that went?
Maybe you are thinking of #3827 which was closed?
e63bdc5 to
c0594bd
Compare
Hm, that wasn't it @JustinGrote. Maybe it was just on Discord. Anyway, this resolves #4557 now since it no longer writes the cwd to the workspace settings. For multi-root workspaces, instead a name can be set (like 'vscode-powershell' for this repo) and then that workspace folder will be used. For a single-root workspace, a relative path will be resolved to its root.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
I'll run this one through the paces once the VSIX is built, I have a couple multi root workspaces I can test against.
@JustinGrote awesome! Please do, I'll have a pre-release out soon. Main thing with multi-root workspaces is that this is only supporting putting the name of the workspace as the cwd. No relative paths to it or anything, just a choice. But it's not an absolute path any more!
cwd and additionalPowerShellExes (削除ここまで)~, ./ and named workspace folders in cwd (追記ここまで)
LGTM with multi workspace that I have, just a comment on the UX on the original issue.
So it's not just automatic, but can be done easily (and is an appropriate value to sync with workspace settings).
And Mocha's debugger hook-up timeout to 30 seconds. Now unit tests can be debugged easily, and the timeout is still appropriate enough for CI.
Uh oh!
There was an error while loading. Please reload this page.
This makes them way less annoying. I'm ok taking a dependency on untildify as it's a very simple package, and the Python extension for VS Code also uses it. However, it must remain at v4.0.0, as the latest version, v5.0.0, is pure ESM and therefore cannot be loaded by VS Code.
https://github.com/sindresorhus/untildify/releases/tag/v5.0.0
Now also handles paths relative to a single workspace folder, or understands the name of a workspace folder in multi-root workspaces. Resolves #4557.