Skip to content

param-op: an escaped ordinary character in a substitution pattern matches it (#750) - #754

Merged
berrym merged 1 commit into
masterfrom
fix/750-replacement-unescape
Aug 16, 2026
Merged

param-op: an escaped ordinary character in a substitution pattern matches it (#750)#754
berrym merged 1 commit into
masterfrom
fix/750-replacement-unescape

Conversation

@berrym

@berrym berrym commented Aug 16, 2026

Copy link
Copy Markdown
Owner

Closes #750 — but not the way the issue was filed. I filed it; the premise was wrong. See below.

The real defect (consensus-backed)

In ${var/pattern/replacement} the pattern is a pattern, so \X means "a literal X". The substitution path did not honor that for an ordinary character:

$ lush -c 'v=abc; printf "%s" "${v//\b/-}"'
abc                             # bash, zsh: a-c

lush_pattern_substitute chooses between the matcher and a plain substring search by looking for *, ?, [ or an extglob opener. \b has none, so it took the substring path and hunted for the two bytes \b. An escaped metacharacter (\*, \[) was fine, because the metacharacter itself satisfied the check.

Adding a backslash to that condition routes it to lush_pattern_match, which already implements the rule — case abc in a\bc) and ${v#\a} both work today. Dequoting here would have been a second copy of a rule with one canonical home.

This is the same defect [ was added to that condition for; the comment above it records that [bd] "never matched because the literal string never contained [bd]". One character class over, same shape.

The replacement half: curated, not fixed

The issue claimed ${path//X/\/} was the only way to get a literal /. Measurement says otherwise:

lush bash zsh
${v//X//} a/b a/b a/b
${v//X//usr/lib} a/usr/libb a/usr/libb a/usr/libb
${v//X/\a} a\ab aab a\ab

The delimiter is the first unescaped /, so every later / is already literal — a literal slash needs no escape at all, in one unquoted expression. And on the escape question bash and zsh diverge, with lush already following zsh.

So unescaping the replacement would have moved lush from zsh's model to bash's, with no capability gained and no rationale recorded — a curated default flipped by accident. Owner call taken: keep the literal rule, and document it so it stops reading as a bug (it is now pinned by tests, recorded in SEMANTICS §3.10, and explained above the splitter).

Also corrects a comment introduced with #684 that said bash and zsh drop the backslash. zsh keeps it — that error is what made #750 look like a defect.

Verification

  • 14 new checks: escaped ordinary characters unanchored / replace-first / doubled / a space / both anchors / a non-occurring char; the three escaped metacharacters that must not regress; three pinning the curated replacement rule.
  • 7 of the file's 40 fail against the parent build.
  • Full suite 197/197. ASan 65/65; the test 1/1 under ASan.
  • Every example in the new documentation was executed and matched as written.

One harness fix rolled in: check_agree now single-quotes its value. Unquoted, a b became a command prefix and a*b was globbed inside the array literal — which null_glob then emptied. Both looked like code defects and were purely the harness.

…ches it

In `${var/pattern/replacement}` the pattern is a PATTERN, so `\X` means "a
literal X". The substitution path did not honor that for an ordinary
character:

    v=abc; ${v//\b/-}
      before: abc          bash, zsh: a-c

lush_pattern_substitute decides between the pattern matcher and a plain
substring search by looking for `*`, `?`, `[` or an extglob opener. A pattern
like `\b` has none of those, so it took the substring path and hunted for the
two BYTES `\b`, which never occur. An escaped METAcharacter (`\*`, `\[`) was
unaffected, because the metacharacter itself satisfied the check.

The fix adds a backslash to that same condition, so the pattern routes to
lush_pattern_match -- which already implements the escape rule, as
`case abc in a\bc)` and `${v#\a}` both demonstrate. Dequoting here instead
would have been a second copy of a rule that has one canonical home.

This is the same defect the `[` in that condition was added for: the comment
above it records that `[bd]` patterns "never matched because the literal
string never contained `[bd]`". One character class over, same shape.

bash and zsh agree, and lush's own behavior for every other operator agrees,
so the fix is mode-invariant and gates nothing.

The REPLACEMENT half is deliberately NOT changed, and is now documented as
curated rather than left to be re-discovered. It is literal text: a backslash
in it survives, matching zsh where bash unescapes. Issue #750 was filed by me
as a defect on the assumption that `${path//X/\/}` was the only way to reach a
literal `/`; measuring showed `${v//X//}` already does, in lush and in all
three references, because the delimiter is the FIRST unescaped `/` and every
later one is already literal. No capability is missing, so the literal rule
costs nothing and keeps the replacement free of escape rules to remember.
Recorded in SEMANTICS section 3.10 and above the splitter, and pinned by
tests so a future change toward bash has to argue with a test rather than
silently flip a curated default.

Also corrects a comment introduced with #684 that said bash AND zsh drop the
backslash in a replacement. zsh keeps it; that error is what made #750 look
like a defect.

tests/integration/test_vector_substitution_spec.c: 14 checks -- escaped
ordinary characters unanchored, replace-first, doubled, a space, both anchors,
a non-occurring char, the three escaped metacharacters that must not regress,
and three pinning the curated replacement rule. 7 of the file's 40 fail
against the parent build. The shared harness now single-quotes its value:
unquoted, `a b` became a command prefix and `a*b` was globbed inside the array
literal, which null_glob then emptied -- both looked like code defects and were
purely the harness.
@berrym
berrym merged commit 18713f8 into master Aug 16, 2026
6 checks passed
@berrym
berrym deleted the fix/750-replacement-unescape branch August 16, 2026 15:27
@codecov

codecov Bot commented Aug 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

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.

the replacement half of ${v/pat/repl} is not unescaped

1 participant