grep: honor \s and \S in basic regexps - #108
Open
MsfPablo wants to merge 1 commit into
Open
Conversation
Merging this PR will not alter performance
Comparing Footnotes
|
|
GNU grep testsuite comparison: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #31.
Syntax::grep()leavesONIG_SYN_OP_ESC_S_WHITE_SPACEoff, so in BRE\sand\Sparsed as the literal letterssandS. GNU grep supports both as extensions in basic regexps, the same way it does\w/\W(which oniguruma already enables forgrepsyntax). Enabled the operator forBasicmode, alongside the existing GNU buffer-anchor line just above it;Extendedalready gets it fromSyntax::gnu_regex().Checked the counts from the issue against GNU grep on input
aS b//x, across\s,\S,\w,\Win default,-Gand-Emodes — all twelve combinations agree, where before\sgave 0 and\Sgave 1 in BRE.cargo test: 95 passed. clippy-D warningsandcargo fmt --checkclean.GNU testsuite: net +1, but one new failure worth flagging
backslash-s-vs-invalid-multibytechecks that neither\snor\Smatches a lone invalid byte (\202) in a UTF-8 locale. It passed before only because\Swas an inert literalSin BRE — the underlying behavior is already wrong in ERE onmain:So this does not introduce the bug, it just stops hiding it in BRE — the same invalid-multibyte / non-ASCII character-class handling tracked in #64. Fixing it belongs there rather than here, since it needs a decision about how invalid bytes are classified generally, not just for
\s.Happy to hold this until #64 lands if you would rather not take the one test regression in the interim.