Skip to content

fix(run): let parameters expand in the command shell, not the ZUnit shell - #25

Merged
ss-o merged 1 commit into
mainfrom
bug-24
Sep 3, 2026
Merged

fix(run): let parameters expand in the command shell, not the ZUnit shell#25
ss-o merged 1 commit into
mainfrom
bug-24

Conversation

@ss-o

@ss-o ss-o commented Sep 3, 2026

Copy link
Copy Markdown
Member

Adds $ to the requoting escape set in run. Two characters of source, plus two regression tests.

The defect

run builds its command as text and evals it, requoting every word that is not a shell operator:

${MATCH//(#b)([\"\`\\])/\\${match[1]}}

", backtick and backslash are escaped. $ is not, so a $ inside a quoted word is expanded by the eval in the ZUnit shell and never reaches the command:

run zsh -f -c 'typeset inner=INNER_VALUE; print -r -- "got=[${inner}]"'
ZUnit Result
zdharma/zunit v0.8.2 got=[INNER_VALUE]
this fork, before got=[]
this fork, after got=[INNER_VALUE]

Upstream is unaffected because it never evals: output=$("${cmd[@]}" 2>&1).

Why not just adopt upstream's approach

The eval is deliberate and earns its place. dont_quote is what lets run accept redirections, ;, |, && and var=... assignments, which upstream cannot handle, and the leading newline is what puts a line number into run:1: command not found: .... Both behaviours have tests here. The narrow fix keeps them.

Why it mattered

The failure is silent. The parameter empties rather than erroring, so assertions fail for reasons that look unrelated to quoting, or pass vacuously.

Found via z-shell/zd: tests/snippets.zunit test 4 fails under this fork with no such file or directory: "/._zi/mirror-backend" because ${object_path} empties, while the same test passes in that repo's CI, which installs upstream v0.8.2. With this change the suite goes from 3/4 to 4/4 locally.

Verification

Both new tests in tests/run.zunit were run against the unmodified source and observed failing, then passing with the change:

against unmodified source:
  not ok 4 - Command parameters expand in the command shell, not the ZUnit shell
  not ok 5 - A var= assignment reaches the command shell intact

with the change:
  1..125   (no failures)

Test non-existent command still asserts run:[0-9]+: command not found:, so the line-number behaviour the newline exists for is unchanged. Suite goes from 123 to 125 passing.

Closes #24

…hell

`run' builds its command as text and evals it, requoting every word that is not
a shell operator. That requoting escaped `"', backtick and backslash but not
`$', so a `$' inside a quoted word was expanded by the eval in the ZUnit shell
and the command saw an empty value:

    run zsh -f -c 'typeset inner=INNER_VALUE; print -r -- "got=[${inner}]"'
    got=[]        # this fork
    got=[INNER_VALUE]   # zdharma/zunit v0.8.2

Upstream avoids it by never evaluating, running "${cmd[@]}" directly. The eval
here is deliberate and worth keeping: dont_quote is what lets `run' accept
redirections, `;', `|', `&&' and var=... assignments, and the leading newline is
what puts a line number in `run:1: command not found: ...'.

Add `$' to the escaped set at both requoting sites, so a `$' in a quoted word
survives the eval and reaches the command's own shell.

The failure was silent: the parameter emptied rather than erroring, so
assertions failed for reasons that looked unrelated to quoting, or passed
vacuously. z-shell/zd tests/snippets.zunit test 4 fails under this fork with
`no such file or directory: "/._zi/mirror-backend"' because ${object_path}
empties, while passing in CI, which installs upstream v0.8.2. With this change
that suite goes from 3/4 to 4/4 locally.

Both new tests in tests/run.zunit were observed failing against the unmodified
source and passing with the change. Error messages keep their line number and
the suite goes from 123 to 125 passing.

Closes #24
@ss-o
ss-o merged commit aff1e35 into main Sep 3, 2026
4 checks passed
@ss-o
ss-o deleted the bug-24 branch September 3, 2026 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

run expands parameters in the ZUnit shell instead of the command's own shell

1 participant