Repository navigation
sed: treat [.c.] and [=c=] in a bracket expression as the character c - #626
qodelicious wants to merge 2 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #626 +/- ##
==========================================
+ Coverage 95.39% 95.41% +0.01%
==========================================
Files 14 14
Lines 7665 7720 +55
Branches 457 463 +6
==========================================
+ Hits 7312 7366 +54
- Misses 352 353 +1
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:
|
| /// escaping ASCII punctuation so that it can't be read as class syntax. | ||
| fn push_class_member(result: &mut Vec<u8>, member: &[u8]) { | ||
| if let [b] = member | ||
| && b.is_ascii_punctuation() |
There was a problem hiding this comment.
i think [[.<.]] and [[=>=]] break now: \< / \> are word boundaries for the regex crate, rejected inside a class
maybe only escape what matters in a class (\ [ ] ^ - & ~ |)?
There was a problem hiding this comment.
Yeah \< and \> were the problem. Now only \ [ ] ^ - & ~ | get escaped inside a class, so [[.<.]] and [[=>=]] work again. Fixed in 834bf51.
| (r"s/[[.-.]]/X/g", r".=a]\^Xc[&~|9z"), | ||
| (r"s/[[.\.]]/X/g", r".=a]X^-c[&~|9z"), | ||
| (r"s/[[.^.][.[.]]/X/g", r".=a]\X-cX&~|9z"), | ||
| (r"s/[[.&.][.~.][.|.]]/X/g", r".=a]\^-c[XXX9z"), |
There was a problem hiding this comment.
please add < and > here, so we don't regress
There was a problem hiding this comment.
Added <> to the input plus cases for [[.<.]], [[=>=]] and both together. All 15 cases give the same output as GNU sed 4.9.
| CharacterMode::Byte => name.len() == 1, | ||
| CharacterMode::Utf8 => str::from_utf8(name).is_ok_and(|s| s.chars().count() == 1), | ||
| }; | ||
| is_single.then_some(name) |
There was a problem hiding this comment.
it only ever returns name, could just return a bool, no?
There was a problem hiding this comment.
It returns a bool now.
|
thanks for looking into this! |
|
Ran the GNU suite on the base (5302985) and on this branch. Both give 19 pass, 30 fail, 16 skip out of 65, and no test changed status. |
They were passed to the regex engine as they were, which read them as a nested set. So [[.a.]] also matched a dot, [[.-.]] did not match a dash, and names like [[.dzs.]] were accepted. Like GNU sed in the C locale, a collating symbol or equivalence class now has to name a single character, which becomes a literal member of the set. Anything else is "Invalid collation character".
Keep < and > as they are in [.c.] and [=c=], since \< and \> are word boundaries for the regex crate. Return a bool from the single character check.
834bf51 to
2ab9404
Compare
Merging this PR will improve performance by 4.5%1 warning
Performance Changes
Tip Curious why performance improved? Comment Comparing 
|
[.c.]and[=c=]inside a bracket expression were handed to the regex engine as they were, and it reads them as a nested set. So[[.a.]]also matched.,[[.-.]]didn't match-,[[.].]]didn't match],[b-[.d.]]was a regex error, and names like[[.dzs.]]or[[.space.]]were accepted and matched their letters.Now, like GNU sed in the C locale, a collating symbol or an equivalence class has to name exactly one character, and that character is added to the set as a literal (punctuation gets escaped so it can't turn into class syntax). So ranges like
[[.a.]-[.c.]]work too. An empty or multi-character name gives "Invalid collation character" like GNU. Character escapes inside the name are decoded like in the rest of the bracket, so[[.\n.]]is a newline and[[.\].]]is still an error, same as GNU. In the byte (C locale) mode a multibyte character counts as more than one, which is what GNU does there too.This covers the C locale part of #322. Real multi-character collating elements from locales like hu_HU, and the locale equivalence classes from #321, still aren't supported since there's no locale collation data.
Added
test_bracket_collating_symbol_and_equivalence_class,test_bracket_collating_symbol_with_character_escapeandtest_bracket_invalid_collation_character, plus unit tests indelimited_parser.rs. The outputs were checked against GNU sed 4.9. The first and third fail on main.cargo test,cargo fmt --checkandcargo clippy --all-targets --all-features -- -D warningspass.Related to #322