Support expanding ~ in $env:PATH when doing command discovery #11552
Conversation
| foreach (string directory in tokenizedPath) | ||
| { | ||
| string tempDir = directory.TrimStart(); | ||
| if (tempDir.EqualsOrdinalIgnoreCase("~")) |
There was a problem hiding this comment.
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.
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.
My intent here is to only handle the beginning case which is the most common case. ~ in the middle doesn't make sense.
|
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.
I guess it is tabs here and below. Please fix indentations.
There was a problem hiding this comment.
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.
| 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.
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? |
iSazonov
added
CL-Engine
|
I think we need to document the feature. |
|
@PoshChan please retry windows |
|
@SteveL-MSFT, successfully started retry of |
TylerLeonhardt
left a comment
TylerLeonhardt
left a comment
There was a problem hiding this comment.
LGTM but maybe open an issue to support Tilda in other PATH-like variables.
|
Created #11570 |
|
🎉 Handy links: |
ghost
mentioned this pull request
8 participants
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.