Skip to content

grep: expand bundled numeric context options - #76

Open
ColumbusLabs wants to merge 3 commits into
uutils:mainfrom
ColumbusLabs:agent/bundled-num-shorthand
Open

grep: expand bundled numeric context options#76
ColumbusLabs wants to merge 3 commits into
uutils:mainfrom
ColumbusLabs:agent/bundled-num-shorthand

Conversation

@ColumbusLabs

Copy link
Copy Markdown

Summary

  • expand numeric context shorthands embedded in short-option clusters
  • preserve standalone multi-digit context values and left-to-right option overrides
  • leave attached and separate values for value-taking options, long options, operands, post--- arguments, and non-UTF-8 arguments unchanged
  • cover the reported fuzzer case and parser non-regressions

The implementation is based only on the behavior specified in issue #68 and the original uutils grep code. No GNU source code was consulted or used.

Verification

  • focused numeric shorthand tests: 5 of 5 passed
  • full test suite: 11 unit and 94 integration tests passed
  • cargo clippy --all-targets --all-features -- -D warnings
  • cargo fmt --all -- --check
  • git diff --check

Closes #68

@codspeed-hq

codspeed-hq Bot commented Jul 24, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 10 untouched benchmarks
⏩ 17 skipped benchmarks1


Comparing ColumbusLabs:agent/bundled-num-shorthand (56b9aab) with main (aa7f3c9)

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.

Comment thread src/lib.rs Outdated
Comment thread src/lib.rs Outdated

@lhecker lhecker left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not entirely sure if I agree with the way this PR is written. It removes the Vec collect() optimization (os_args() returns a Vec IntoIterator), adds full UTF8 validation (is that necessary?), and is overall a lot more verbose than the old code.

I haven't thought it through, and so I may be mistaken, but I intuitively feel like the issue should be moderately easy to solve within the old code's structure.

@lhecker
lhecker requested a review from sylvestre July 27, 2026 15:44
Comment thread tests/test_grep.rs
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.

rejects a -NUM context shorthand bundled onto short options that GNU accepts

3 participants