Skip to content

grep: cover zero-width EOF only-matching behavior#44

Merged
lhecker merged 2 commits into
uutils:mainfrom
wondr-wclabs:codex/only-matching-invert-zero-width
Jun 5, 2026
Merged

grep: cover zero-width EOF only-matching behavior#44
lhecker merged 2 commits into
uutils:mainfrom
wondr-wclabs:codex/only-matching-invert-zero-width

Conversation

@wondr-wclabs
Copy link
Copy Markdown
Contributor

Closes #28.

Summary

  • allow the match iterator to check offset == line.len() so EOF zero-width matches can be observed
  • keep the existing zero-width offset nudge so the iterator still advances past EOF afterward
  • add -o / -o -v regressions for $ and x* on empty lines

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Jun 5, 2026

Merging this PR will not alter performance

✅ 10 untouched benchmarks
⏩ 17 skipped benchmarks1


Comparing wondr-wclabs:codex/only-matching-invert-zero-width (5ea3667) with main (d556159)

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.

@wondr-wclabs wondr-wclabs force-pushed the codex/only-matching-invert-zero-width branch from cde7ac3 to 7c24ede Compare June 5, 2026 08:39
Comment thread src/matcher.rs Outdated
Comment on lines +212 to +213
// Searching at EOF is still meaningful for zero-width patterns such as
// `$` and `x*`. After emitting one, `offset` is nudged past EOF below.
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.

While I'm grateful for your other PRs, do you not review them?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You are right to push back here. I re-reviewed the diff and the implementation was already allowing the EOF search because Cursor::refill stops only once offset > line.len(). That means the original PR title/comment overstated the change: the useful part of this PR is regression coverage, not a behavior fix.

I removed the source comment and retitled the PR to make that scope explicit. The branch now only adds tests for -o/-v zero-width EOF behavior ($ and x*). Focused validation after the update: cargo test --test test_grep only_matching.

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.

You are right to push back here.

😭

@wondr-wclabs wondr-wclabs changed the title grep: handle zero-width matches at EOF grep: cover zero-width EOF only-matching behavior Jun 5, 2026
Copy link
Copy Markdown
Collaborator

@lhecker lhecker left a comment

Choose a reason for hiding this comment

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

Thank you for the improved test coverage.

Frankly, I'd appreciate if a human would communicate with me, however. No matter what language or style.

@lhecker lhecker merged commit bc61aec into uutils:main Jun 5, 2026
11 of 17 checks passed
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.

wrong exit code for -o -v with a zero-width match on an empty line (related to #18)

2 participants