Update quoting for CMD #986
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ericcornelissen
force-pushed
the
cmd-quoting
branch
from
eeeb453
to
69275eb
Compare
This commit is a work in progress... CMD quoting is weird - It appears to be necessary to preserve whitespace but you can't disable variable expansion from within quotes (evidenced by the removal of `_common.cjs` line 68 to 70). The main progress of this commit is that it fixes the incorrect escaping of `%` when quoting for CMD. However, it lacks whitespace preservation which should really be kept - and, to reiterate, whitespace preservation is impossible without quoting in CMD, as far as I'm aware.
ericcornelissen
force-pushed
the
cmd-quoting
branch
from
69275eb
to
acd428c
Compare
(and correct `getExpectedOutput` for Unix shells.)
Turns out quoting is necessary in CMD for one important reason: to prevent argument splitting. The side affect of quoting whitespace seems to be that, in contrast to escaping for interpolation, double quotes must be escaped differently. Hence, this commit also re-introduces the `escapeArgForQuoted` function. This particular escaping gets a bit confusing when a pre-existing quote is next to a whitespace character because the inserted quotes from the actual quoting have an effect on those pre-existing quotes, so they need to be handled separately.
- Include cases where there's multiple for CMD. - Cover the caret character (`^`) when quoting for Windows. These cases are based on uncaught mutants.
Per the new quoting strategy for CMD, quoting does not necessarily mean the resulting string has a quote character at the start and end of the string. Thus, remove all such checks from the tests.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Relates to #982
Summary
This addresses incorrect escaping for
%when quoting an argument for CMD.Resources