Conversation

andyleejordan

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://.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.

@andyleejordanandyleejordan added Issue-EnhancementA feature request (enhancement).Area-Configuration labels Aug 5, 2023
@andyleejordanandyleejordan requested a review from a team August 5, 2023 02:05
@andyleejordanandyleejordan force-pushed the andyleejordan/untildify branch from 09cfa4b to 3ef57f9 Compare August 5, 2023 02:07
@andyleejordan

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?

Base automatically changed from andyleejordan/better-finding to main August 8, 2023 13:30
@andyleejordanandyleejordan force-pushed the andyleejordan/untildify branch 2 times, most recently from c5a309c to 3bb69d7 Compare August 8, 2023 22:11
@andyleejordanandyleejordan requested a review from a team as a code owner August 8, 2023 22:11
@andyleejordan
Use untildify for ~ in cwd and additionalPowerShellExes
abb5ef6
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://.com/sindresorhus/untildify/releases/tag/v5.0.0
@andyleejordan
Add tests for validateCwdSetting
0094d35
Those were overdue!
@andyleejordan
Stop using this.sessionSettings.cwd directly
aa6dd8e
So we can remove weirdly placed `validateCwdSetting` calls and instead
use it exactly when required.
@andyleejordan
Support relative paths in cwd and add getChosenWorkspace
e3743c5
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`.
@andyleejordan
Fix up settings tests and add test for cwd relative path support
b98320e
@andyleejordanandyleejordan force-pushed the andyleejordan/untildify branch from 3bb69d7 to e63bdc5 Compare August 8, 2023 22:13
andyleejordan
andyleejordan commented Aug 8, 2023
View reviewed changes
@JustinGrote

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?

@andyleejordanandyleejordan force-pushed the andyleejordan/untildify branch from e63bdc5 to c0594bd Compare August 9, 2023 05:05
@ghost ghost added the Issue-BugA bug to squash.label Aug 9, 2023
@andyleejordan

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.

SeeminglyScience
SeeminglyScience approved these changes Aug 9, 2023
View reviewed changes

Choose a reason for hiding this comment

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

LGTM!

@andyleejordanandyleejordan added this pull request to the merge queue Aug 9, 2023
@JustinGrote

I'll run this one through the paces once the VSIX is built, I have a couple multi root workspaces I can test against.

@github-merge-queue-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 9, 2023
@JustinGroteJustinGrote mentioned this pull request Aug 9, 2023
Closed
2 tasks
@andyleejordan

@JustinGrote awesome! Please do, I'll have a 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!

@andyleejordanandyleejordan changed the title Use untildify for ~ in cwd and additionalPowerShellExes Support ~, ./ and named workspace folders in cwd Aug 9, 2023
@andyleejordanandyleejordan added this pull request to the merge queue Aug 9, 2023
@JustinGrote

LGTM with multi workspace that I have, just a comment on the UX on the original issue.

@andyleejordanandyleejordan removed this pull request from the merge queue due to a manual request Aug 9, 2023
@andyleejordan
Add prompt to save multi-root workspace choice to cwd
fe8cc41
So it's not just automatic, but can be done easily (and is an
appropriate value to sync with workspace settings).
@andyleejordan
Increase unit test timeout to 10 minutes
4832287
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.
@andyleejordanandyleejordan added this pull request to the merge queue Aug 9, 2023
@github-merge-queue-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 9, 2023
@andyleejordanandyleejordan added this pull request to the merge queue Aug 9, 2023
@github-merge-queue-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 9, 2023
@andyleejordanandyleejordan merged commit 4f47aa4 into main Aug 9, 2023
@andyleejordanandyleejordan deleted the andyleejordan/untildify branch August 9, 2023 20:19
Sign up for free to join this conversation on . Already have an account? Sign in to comment

@JustinGrote JustinGrote JustinGrote left review comments

@SeeminglyScience SeeminglyScience SeeminglyScience approved these changes

@JustinGrote JustinGrote

Area-Configuration Issue-BugA bug to squash.A feature request (enhancement).
Flying Fox
Status: Done
No milestone

Successfully merging this pull request may close these issues.

Allow means to avoid adding an absolute Cwd path to the workspace settings