-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Support expanding ~ in $env:PATH when doing command discovery #11552
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
| foreach (string directory in tokenizedPath) | ||
| { | ||
| string tempDir = directory.TrimStart(); | ||
| if (tempDir.EqualsOrdinalIgnoreCase("~")) |
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.
This should only check the very first directory, right? I don't know if tilda can be in the middle...
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.
I'd expect that we expand tilde for all elements. There are other applications which add their paths.
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.
My intent here is to only handle the beginning case which is the most common case. ~ in the middle doesn't make sense.
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
|
If we do this for |
|
There are potentially other PATH values where we might want to apply this as well. I don't recall the exact names off the top of my head, but don't Mac and Linux have PATH-type environment variables that dictate where to look for native library dependencies to load? |
| } | ||
|
|
||
| BeforeAll { | ||
| $oldPath = $env:PATH |
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.
I guess it is tabs here and below. Please fix indentations.
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.
I noticed the file was mixed formatting, I'll reformat the entire file.
|
|
||
| Copy-Item -Path $pwsh -Destination "~/$pwsh2" | ||
| $testPath = Join-Path -Path "~" -ChildPath (New-Guid) | ||
| New-Item -Path $testPath -ItemType Directory |
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.
| New-Item -Path $testPath -ItemType Directory | |
| New-Item -Path $testPath -ItemType Directory > $null |
| foreach (string directory in tokenizedPath) | ||
| { | ||
| string tempDir = directory.TrimStart(); | ||
| if (tempDir.EqualsOrdinalIgnoreCase("~")) |
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.
I'd expect that we expand tilde for all elements. There are other applications which add their paths.
|
From https://github.com/dotnet/cli/issues/9321 I see that only $HOME works for zsh and MacOs. |
|
@vexx32, you're thinking of |
|
@SteveL-MSFT yeah I understand that. My concern is simply that we will now have two separate ways we handle the one "kind" of environment variable. It's certainly not desirable to promote this oddity too much, I'll agree, but I wonder whether it's better / simpler to be consistent here and have a single way of handling at least initial path resolution for paths in these variables? |
|
I think we need to document the feature. |
|
@PoshChan please retry windows |
|
@SteveL-MSFT, successfully started retry of |
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 but maybe open an issue to support Tilda in other PATH-like variables.
|
Created #11570 |
|
🎉 Handy links: |
PR Summary
The
~meaning user home path is a commonly used convention. Bash (and compatible shells) expands this automatically (along with other derivatives not covered in this PR). Dotnet global tools sets $env:PATH with a path to the tools starting with~. In PowerShell, that is treated as a literal so the tools aren't found. Fix is in command discovery, when processing $env:PATH, handle the case where a sub-path starts with~/or just contains~and expand it to be the user home path.PR Context
Fix #11531
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.