Skip to content

Fix --posix to disable the GNU regular expression escapes - #550

Open
LoukasPap wants to merge 1 commit into
uutils:mainfrom
LoukasPap:issue542
Open

Fix --posix to disable the GNU regular expression escapes#550
LoukasPap wants to merge 1 commit into
uutils:mainfrom
LoukasPap:issue542

Conversation

@LoukasPap

Copy link
Copy Markdown
Contributor

Fixes #542

Under --posix, GNU sed treats the GNU regular expression escapes as the literal character that follows the backslash. These are the operators \?, \+ and \|, the character classes \w, \W, \s and \S, and the empty-width matches \b, \B, \<, \>, \` and \'.

parse_regex_for_mode() now takes a POSIX parameter and emits the literal character for these escapes. The check comes before parse_char_escape(), otherwise \b is decoded as a backspace.

BRE and ERE

The two modes need the opposite spelling of the same literal:

  • In BRE the character is emitted bare, because bre_to_ere() escapes the ERE metacharacters afterwards. Emitting \? would make bre_to_ere() turn it back into the operator.
  • In ERE \?, \+ and \| keep their backslash, since a backslash is already what makes them literals there. Emitting them bare would make them operators.

The remaining escapes are letters and punctuation with no meaning of their own, so they are emitted bare in both modes.

Copilot AI lite review requested due to automatic review settings September 5, 2026 13:05
@codecov

codecov Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.96970% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.16%. Comparing base (77f442d) to head (f7a7703).

Files with missing lines Patch % Lines
src/sed/compiler.rs 92.85% 1 Missing ⚠️
src/sed/delimited_parser.rs 98.07% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #550      +/-   ##
==========================================
+ Coverage   83.04%   83.16%   +0.12%     
==========================================
  Files          13       13              
  Lines        7046     7109      +63     
  Branches      401      402       +1     
==========================================
+ Hits         5851     5912      +61     
- Misses       1192     1194       +2     
  Partials        3        3              
Flag Coverage Δ
macos_latest 83.79% <96.96%> (+0.12%) ⬆️
ubuntu_latest 83.99% <96.96%> (+0.11%) ⬆️
windows_latest 0.00% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new integration tests don’t yet cover all GNU escapes called out by #542 (notably \+ and \|), reducing end-to-end regression protection for the claimed fix.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates the sed script regex parser so that when --posix is enabled, GNU regex escape extensions (e.g., \?, \+, \|, \w, \s, etc.) are treated as literals rather than GNU-specific operators/classes/anchors, aligning behavior with GNU sed and fixing #542.

Changes:

  • Thread context.posix into regex parsing (parse_regex_for_mode) and handle GNU escape extensions as literals under POSIX mode (with special handling for ERE metacharacters ? + |).
  • Add end-to-end regression tests validating --posix disables key GNU regex escapes, plus unit tests covering BRE vs ERE emission rules.
File summaries
File Description
tests/by-util/test_sed.rs Adds integration tests for --posix behavior around GNU regex escape extensions.
src/sed/delimited_parser.rs Implements --posix parsing behavior for GNU regex escapes and adds targeted unit tests.
src/sed/compiler.rs Passes context.posix into regex parsing for addresses and substitution patterns.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/sed/delimited_parser.rs Outdated
Comment thread tests/by-util/test_sed.rs
@LoukasPap

Copy link
Copy Markdown
Contributor Author

@sylvestre While working on this I noticed that \b, \<, \>, \` and \' are also broken without --posix, where they never match:

$ printf 'cat catalog\n' | sed 's/\<cat\>/X/g'
X catalog
$ printf 'cat catalog\n' | ./target/debug/sed 's/\<cat\>/X/g'
cat catalog

The fix looks small, and differs per escape. All three engines support \<, \>, \b, \A and \z natively, so no lookarounds are needed:

  • \< and \> already reach Regex::new() unchanged. They break only because NEEDS_RE in fast_regex.rs does not list them, so the pattern is taken for a literal string and remove_escapes() drops the backslash, leaving <cat>. Widening \\[AzBb] to \\[AzBb<>] is enough.
  • \b is decoded as a backspace by parse_char_escape(), so it has to be intercepted before that call and emitted unchanged.
  • \` and \' have no equivalent in the Rust engines and need translating to \A and \z.

This is the separate problem from this PR, so I left it out. Do you want to add it in this PR or open a separate issue for it?

- Add a POSIX parameter to `parse_regex_for_mode()` that emits the literal
  character for `\?`, `\+`, `\|`, `\w`, `\W`, `\s`, `\S`, `\b`, `\B`, `\<`,
  `\>`, `` \` `` and `\'`
- Emit the character bare in BRE, where `bre_to_ere()` escapes it afterwards,
  and keep the backslash on `\?`, `\+` and `\|` in ERE
- Add unit and integration tests
Copilot AI review requested due to automatic review settings September 5, 2026 13:40

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is narrowly scoped, threads --posix consistently through the compiler, and includes both unit and integration tests covering the corrected behavior.

Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

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.

--posix should disable the GNU regular expression escapes (\? \+ \| \w \s)

2 participants