Repository navigation
sed: treat unknown regex escapes as literal characters - #664
Himanshu-singh04 wants to merge 2 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #664 +/- ##
==========================================
+ Coverage 95.25% 96.68% +1.43%
==========================================
Files 14 14
Lines 7500 7393 -107
Branches 442 437 -5
==========================================
+ Hits 7144 7148 +4
+ Misses 355 244 -111
Partials 1 1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| result.push(b'\\'); | ||
| // Unknown letter and zero escapes are literal in sed, even | ||
| // when the regex engine gives them another meaning. | ||
| if !(line.current().is_ascii_alphanumeric() |
There was a problem hiding this comment.
the double negation is hard to read
could be something like let c = line.current(); if c == '0' || (c.is_ascii_alphabetic() && !is_gnu_regex_escape(c, regex_mode)) then push only the char, no?
There was a problem hiding this comment.
Simplified the condition by naming the current character and testing the literal escape case directly in 44daae0.
| for (script, input, expected) in [ | ||
| (r"s/\D/X/", "aDb1\n", "aXb1\n"), | ||
| (r"s/\A/X/", "xAy\n", "xXy\n"), | ||
| (r"s/\0/X/", "x0y\n", "xXy\n"), |
There was a problem hiding this comment.
please also add a case with -E and one with an address, e.g. /\D/p
There was a problem hiding this comment.
Added an -E substitution case and an address case using /\D/p in 44daae0.
Fixes #658.
Unknown regex escapes were passed through to the Rust regex engine, which gave some of them a different meaning or rejected them. Emit unknown escaped ASCII letters and zero as literal characters while preserving sed's recognized regex escapes and backreferences.
Added an integration regression for
\D,\A,\0, and\p. The test failed on main and passes with the change.Validation:
cargo test --quiet(408 unit tests and 365 integration tests passed; 2 existing integration tests ignored)cargo fmt --checkcargo clippy --all-targets -- -D warningsgit diff --check