fix: keep the rest of the command line when expanding a wildcard#603
Open
webpro wants to merge 1 commit into
Open
fix: keep the rest of the command line when expanding a wildcard#603webpro wants to merge 1 commit into
webpro wants to merge 1 commit into
Conversation
The wildcard expander matched the runner invocation with
/((?:npm|yarn|pnpm|bun) run|node --run|deno task) (\S+)([^&]*)/
and rebuilt the command as `${command} ${script}${args}`. Everything from
the first `&` onward was captured by nothing and dropped, so
concurrently 'npm run build:* && echo done'
ran the two builds and silently discarded `&& echo done`, exiting 0. The
same truncation cut
concurrently 'npm run build:* -- --grep "a & b"'
mid-string, leaving `/bin/sh` with an unterminated quote. The pattern is
also unanchored, so `echo "npm run build:*"` was expanded as if it were a
runner invocation and, finding no script ending in a quote, returned no
commands at all.
Locate the invocation in the parsed command instead, and substitute the
script in place using the source span of the glob, so every other byte of
the command line is preserved. concurrently's own `(!...)` omission
syntax is extglob, which the parser reads as one word, so it needs no
special handling.
This file contains hidden or 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
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.
The problem
ExpandWildcardfinds the runner invocation with a regex and rebuilds the command fromthe captured pieces:
([^&]*)stops at the first&, and nothing captures what follows, so it is dropped.The pattern is also unanchored, so it matches inside quoted text.
Three things go wrong, all reproducible with the published package:
A chained command is silently discarded.
An
&inside a quoted argument truncates the command mid-string.A runner named inside a quoted argument is expanded as if it were an invocation.
echo "npm run build:*"matches the pattern, finds no script ending in a quote, andreturns no commands at all, so the command disappears rather than running.
The change
Locate the invocation in the parsed command rather than in the raw text, then substitute the script using the source span of the glob:
Every other byte of the command line is preserved by construction, which is what fixes all three cases at once rather than one at a time. The runner lookup walks the parsed command, so a runner name inside a quoted argument is not a runner invocation and an invocation after
cd app &&still is.The
(!...)omission syntax needs no special handling: it is extglob, and the parser readswatch-*(!js)as a single word.About the dependency
This adds unbash, a zero dependency synchronous Bash parser with no WASM and no async initialisation. I wrote it. I also wrote knip, which depends on it, so it already runs against a large amount of real world shell: knip is at about 11.7M downloads a week and unbash at about 8.5M.
Worth noting given the two
shell-quoteadvisory bumps in #591 and #599:shell-quotestays, becauseexpand-arguments.tsuses itsquote(), which is a different job from parsing. This PR does not touch that.Verification
Both reproductions above were confirmed against the built binary before and after.