Skip to content

grep: honor \s and \S in basic regexps - #108

Open
MsfPablo wants to merge 1 commit into
uutils:mainfrom
MsfPablo:bre-escape-s-whitespace
Open

grep: honor \s and \S in basic regexps#108
MsfPablo wants to merge 1 commit into
uutils:mainfrom
MsfPablo:bre-escape-s-whitespace

Conversation

@MsfPablo

@MsfPablo MsfPablo commented Aug 18, 2026

Copy link
Copy Markdown

Fixes #31.

Syntax::grep() leaves ONIG_SYN_OP_ESC_S_WHITE_SPACE off, so in BRE \s and \S parsed as the literal letters s and S. GNU grep supports both as extensions in basic regexps, the same way it does \w/\W (which oniguruma already enables for grep syntax). Enabled the operator for Basic mode, alongside the existing GNU buffer-anchor line just above it; Extended already gets it from Syntax::gnu_regex().

$ printf 'a b\nxy\n' | grep -e '\s'
a b

Checked the counts from the issue against GNU grep on input aS b / / x, across \s, \S, \w, \W in default, -G and -E modes — all twelve combinations agree, where before \s gave 0 and \S gave 1 in BRE.

cargo test: 95 passed. clippy -D warnings and cargo fmt --check clean.

GNU testsuite: net +1, but one new failure worth flagging

PASSED: +1   FAILED: -1
  + backslash-s-and-repetition-operators
  + multibyte-white-space
  - backslash-s-vs-invalid-multibyte

backslash-s-vs-invalid-multibyte checks that neither \s nor \S matches a lone invalid byte (\202) in a UTF-8 locale. It passed before only because \S was an inert literal S in BRE — the underlying behavior is already wrong in ERE on main:

$ printf '\202\n' > in
$ ./target/release/grep -E '^\S$' in ; echo $?    # main, unpatched: 0 (should be 1)

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.

@codspeed-hq

codspeed-hq Bot commented Aug 18, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 10 untouched benchmarks
⏩ 17 skipped benchmarks1


Comparing MsfPablo:bre-escape-s-whitespace (4a4aa13) with main (5882b3d)

Open in CodSpeed

Footnotes

  1. 17 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@github-actions

Copy link
Copy Markdown

GNU grep testsuite comparison:

Test results comparison:
  Current:   TOTAL: 128 / PASSED: 79 / FAILED: 28 / SKIPPED: 21
  Reference: TOTAL: 128 / PASSED: 78 / FAILED: 29 / SKIPPED: 21

Changes from main branch:
  TOTAL: +0
  PASSED: +1
  FAILED: -1

New test failures (1):
  - backslash-s-vs-invalid-multibyte

Test improvements (2):
  + backslash-s-and-repetition-operators
  + multibyte-white-space

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.

\s and \S are not honored in basic-regexp (-G) mode like GNU

1 participant