Repository navigation
Add tilde expansion for windows native executables - #20402
Conversation
|
@microsoft-github-policy-service agree |
|
The place it is done for cmdlets like PowerShell/src/System.Management.Automation/namespaces/LocationGlobber.cs Lines 4542 to 4552 in dab6ca2 |
Steve Lee (SteveL-MSFT)
left a comment
There was a problem hiding this comment.
Thanks for the contribution! Please wrap as an ExperimentalFeature as it also helps advertise the new capability and allow changes on feedback not to be a breaking change.
f41b83a to
7aabb18
Compare
There was a problem hiding this comment.
This is where this feature gets a bit complicated and the original implementation was abandoned. Double quotes are expected to be expanded (like if your path has whitespace), but single quotes are treated as literal and never expanded. So this needs to be differentiated.
There was a problem hiding this comment.
Would you expect tilde expansion if it was surrounded with double quotes? Currently pwsh on linux doesn't expand tilde for double quotes:
Also I checked bash, fish, zsh with /bin/echo "~/" and none of them expand it (whereas /bin/echo ~ is expanded). So I think it's reasonable to just follow every other shell here, are there other considerations for how tilde should behave on windows?
Also in a related area, the tab completion around tilde doesn't seem quite right - the behaviour from powershell 5 seems more correct to me.
Some cases I tried out, where $ is cursor position:
| Case | bash --norc (5.2) |
pwsh -NoProfile (7.4.0) |
powershell -NoProfile (5.1) |
|---|---|---|---|
ls "~/my<tab> |
ls "/Users/user/my folder"/$ |
ls "~\my folder\$" |
ls "C:\Users\user\my folder\$" |
ls ~/my<tab> |
ls ~/my\ folder/$ |
ls '~\my folder\$' |
ls 'C:\Users\user\my folder\$' |
Seems like it is new (undesirable) behaviour in 7.4.0, some discussion about it here: #20750
There was a problem hiding this comment.
POSIX-like shells only ever expand an unquoted argument-initial ~, either in isolation or if followed by a an unquoted username (not implemented in PowerShell - see #12387) or an unquoted /, and then possibly followed by an mix of unquoted and quoted strings (any mix of '...', "...", with their usual semantics), so I think that's what PowerShell should do as well - with the addition of also recognizing unquoted \, as stated in the initial post.
Thus, to ensure expansion of ~-prefixed paths parts of which require quoting, only the part after ~/ must be quoted; e.g. /bin/echo ~/'a test' (ditto in PowerShell) or /bin/echo ~/a\ test (/bin/echo ~/a` test in PowerShell)
Note that these cases are currently broken in PowerShell (which currently surface on Unix only):
- Unix: Emulation of ~ (tilde) expansion for native utilities is syntactically incomplete #20754
- They're also broken with respect to tab-completion, which inappropriately turns
~/'a test'into'~/a test', for instance: Broken tab-completion with file or directory names with spaces in the context of tilde-prefixed paths (~) #20851
Indeed, the problem described in #20750 effectively broke tab-completion of ~ on Windows for native-program calls, due to - by well-meaning design - no longer instantly expanding ~ to the value of $HOME.
This PR would make that problem go away - but to resolve #20750 this way (which I personally prefer) requires it to become a stable feature right away.
Find a summary of the solution options at:
There was a problem hiding this comment.
Steve Lee (@SteveL-MSFT) i think this change just makes the UNIX and Windows behaviors similar, where ~ or ~/ would now behave consistently regardless of platform (it's cribbed from the code in lines 399-418). I would probably suggest #else rather than #endif at line 419 which might be clearer. We disambiguate between quoted and non-quoted ~ in lines after 344, so that would remain consistent. We don't currently treat ~ with regard to single and double quotes differently; if it's quoted, it's not expanded.
Now that I think on it, the concern I do have is that we would now have duplicated code which means a fix in one spot may not happen in the other. Perhaps this method should be refactored where we don't duplicate the code.
600b89d to
a7270cf
Compare
a7270cf to
a6be29c
Compare
|
This PR has Quantification details
Why proper sizing of changes matters
Optimal pull request sizes drive a better predictable PR flow as they strike a
What can I do to optimize my changes
How to interpret the change counts in git diff output
Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email) |
James Truher (JamesWTruher)
left a comment
There was a problem hiding this comment.
nice!
|
Steve Lee (@SteveL-MSFT) James Truher (@JamesWTruher) Dominic Slee (@domsleee) The fact that PowerShell on Linux quotes space-containing paths during tab expansion is broken behavior since 7.4.0, because it breaks translation of ~ when calling executables. This PR will now break Windows in the same way, because of the same tab expansion behavior. Additionally, trying to use PowerShell's native escaping for spaces to work around this behavior on Linux is also currently broken: In my unvarnished opinion, the dumpster fire around tab expansion behavior with ~ introduced in 7.4.0 was ill thought out, and this one PR is not remotely a complete fix, even on Windows. Frankly: where are the test cases for all of this? How was this feature allowed out into the wild without realizing how many things were being broken, and how is the team working to resolve them before 7.5.0? The fact that the team has been 100% silent on #20750 is extremely disappointing, given that the issue has been open since November 2023. Will anybody ever respond? |
|
While I share your frustration around the lack of responses / a decision on #20750, |
|
Michael Klement (@mklement0) Not to be overly pessimistic, but neither of your issues have any comment from anybody on the team, either. Right now you're we're basically hoping they agree and create (or allow) a fix in time for 7.5.0. |
|
Fair point, Brad Wilson (@bradwilson) - let's hope that this exchange helps. |
|
The behavior of this implementation of tilde expansion seems utterly unintuitive to me and breaks basic assumptions that I (and I assume many other PowerShell users) have: $Path = "~"
cmd /c echo $Path # C:\Users\admin
cmd /c echo "$Path" # ~One of the things I like about PowerShell is that I don't have to think about quoting rules – if I want to pass a variable as a parameter, I just pass a parameter and don't worry about quoting. This feature breaks that, and means that whenever I'm passing a variable to a native command, I have to actively think whether I want to expand ~ and quote accordingly. PowerShell is complex enough as it is, and I don't see any way this feature can pull its weight. I disagree with the original implementation of this feature, where I first encountered this feature when doing something like ssh <host> cp ... ~/...as part of a longer command and receiving mysterious errors that In POSIX shells, x1=~
x2="~"
echo $x1 # /home/...
echo "$x1" # /home/...
echo $x2 # ~
echo "$x2" # ~This is not ideal, but it is much more intuitive than expanding I'd personally happily sacrifice the resolution of ~ when calling native commands if it means that PowerShell quoting rules will remain simple and interact nicely with remote |
|
The Were you to run your example command on Unix, you would have to escape or quote the
Similarly, expansion still happening via variables - |
Yes, but I use PowerShell because it's better than Unix shells. :) More seriously, PowerShell takes different approach than Unix-style shells, and not having to quote variables is an important part of what makes PowerShell more intuitive and comfortable for daily usage. In Unix shells, you're expected to quote more-or-less everything, which is not expected in PowerShell, and this PR makes PowerShell quoting more complex, for imo lesser benefit. I understand the desire for consistent |

PR Summary
Attempted implementation for #20031.
Tilde expansion here is the same on windows as it is on unix, except that it also expands back slash.
i.e. ~, ~/...., and ~\.... are expanded.
PR Context
This comment: #20031 (comment)
My understanding is that tilde expansion was briefly introduced as part of a larger PR.
But that PR didn't handle double quotes properly, specifically
"~"should not be expanded on windows, so it was removed.There were also other issues with the
PSNativePSPathResolutionthat lead to its removal, specifically around determining if the PSDrive argument was a file or not.The failing example (from the previous PR) is:
For tilde expansion, we don't need to consider this information. Tilde (with no quotes) should be always expanded.
We can use an experimental feature here if required, not sure if it is needed 👍
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.PSNativeWindowsTildeExpansion? It is similar toPSNativePSPathResolution, but with reduced scope.(which runs in a different PS Host).