Stop globbing of quoted arguments to native commands#5188
Stop globbing of quoted arguments to native commands#5188lzybkr merged 2 commits intoPowerShell:masterfrom
Conversation
28e326e to
1534378
Compare
a3613a5 to
ad12184
Compare
|
@daxian-dbw - I think this is ready for review. |
|
Must ... have ... this ... fix. :-) |
|
@lzybkr I was out in training last Friday. Will get this done today. |
There was a problem hiding this comment.
Is sep = " " really needed here? It seems we just need var sep = " " right before the foreach
There was a problem hiding this comment.
Well, it's more tidy - it avoids putting 2 spaces between the previous token - either the command name or an argument.
There was a problem hiding this comment.
Thanks for clarifying #Closed.
daxian-dbw
left a comment
There was a problem hiding this comment.
LGTM, but have one more comment about expanding ~.
There was a problem hiding this comment.
Now we will expand ~ even if arg is not a bare word. Do we want to expand ~ unconditionally?
There was a problem hiding this comment.
Another question: it seems to me that after this change we don't glob for variables. Is it intentional?
For example, before the change
PS> $var = '*.txt'
PS> /bin/echo $var
a.txt b.txt c.txt
with the change
PS> /bin/echo $var
*.txt
There was a problem hiding this comment.
Thanks for catching, I missed this when reviewing Bruce's changes, but you're right, we don't want to expand ~ unconditionally. I'll fix it.
There was a problem hiding this comment.
And I see bash also expands variables unless quoted, so we should probably match that behavior too.
There was a problem hiding this comment.
@daxian-dbw - updated. If you sign off, I'll merge - I want to squash the last 2 commits and rebase 2 commits instead squashing the 3 commits.
dd4d8ad to
8871dd5
Compare
Previously we used IScriptPosition for context (e.g. error reporting) during parameter binding, but in some cases we want more information, so we'll use the Ast instead. This change just adds the Ast, it doesn't make explicit use of it.
Also fix some minor issues with exceptions being raised when resolving the path - falling back to no glob. Fix: PowerShell#3931 PowerShell#4971
|
🎉 |
Fix globbing so that quoted args are not expanded.
Also Fix some minor issues with exceptions being raised when resolving
the path - falling back to no glob.
The bulk of the change is in the first commit, which passes the
Astornullinstead of theIScriptExtentwhen constructing the parameters and arguments that are used by the parameter binder.Fix #3931,#4971