Repository navigation
fix(grep): free -m for GNU --max-count instead of rtk --max - #3259
Merged
KuSh merged 1 commit intoAug 28, 2026
Merged
Conversation
`-m` was derived as the short for RTK's `--max` (a display cap), colliding with GNU grep where `-m N` = `--max-count` (stop after N matches per file). `rtk grep -m N` set RTK's display cap and never forwarded --max-count, so grep didn't actually stop early — a silent semantic mismatch (no parse error). Drop the short. `-m` now flows to extra_args and is forwarded verbatim to grep/rg (it is not a has_format_flag letter, so it takes the normal engine path, not passthrough), applying genuine --max-count while RTK still compacts. This makes Grep consistent with Rg, which never had the short and already forwarded `-m` correctly. `--max` keeps its long form and default 200. - src/main.rs: drop `short` from Grep::max + clap-parse regression test. - tests/grep_faithful_format_test.rs: end-to-end `rtk grep -m N` == `grep -n -m N` (leading, trailing, N >= total) plus the per-file multi-file case. - docs/usage/FEATURES.md: drop the `-m` short; fix stale default 50 -> 200.
breisnerlopez
force-pushed
the
fix/grep-m-max-count-collision
branch
from
August 1, 2026 11:53
cd4dfed to
3b6b812
Compare
dev1abz
added a commit
to dev1abz/rtk
that referenced
this pull request
Aug 7, 2026
3 tasks done
3 tasks done
Merged
5 tasks done
devnulled
added a commit
to devnulled/rtk
that referenced
this pull request
Aug 30, 2026
rtk's own tuning options on `Commands::Grep` declared short forms that
collide with native grep/rg flags of the same letter. This removes the two
that remain, and deletes one option outright.
`--max-len` loses `-l`. `-l` is grep's --files-with-matches, and the old
binding split behavior on whether the PATTERN parsed as a usize:
- Non-numeric: clap rejected it, run_fallback re-ran raw grep, and the
user got the right answer. Wasteful, not harmful.
- Numeric: clap ACCEPTED it, and the result was silently wrong.
# hit.txt contains "listen on port 8080 today"; miss.txt does not
$ rtk grep -l 8080 hit.txt miss.txt
before: (no output) exit=1 <- claims nothing matched
after: hit.txt exit=0
`-l 8080` set max_len=8080, leaving hit.txt as the pattern and miss.txt
as the only path. With a single file the filename becomes the pattern,
no path is left, and rtk falls back to stdin: exit 1 against /dev/null,
hangs on a terminal. Port numbers, years and error codes are ordinary
patterns, and the Claude Code hook rewrites plain `grep` into `rtk grep`
transparently -- so this reached users who never opted in.
Note this is not a savings story. `-l` is a has_format_flag passthrough
on the fixed path too (search.rs:548), so there is no compaction for
this flag either way. The bug was a wrong answer.
`--file-type` is removed entirely, with its `-t` short, rather than left as
a parse-only no-op. It was destructured as `file_type: _` at the call site
and never reached search::run, so it parsed and then did nothing.
`rtk rg -t rust` is unaffected: Rg has no such field and forwards `-t` to
real ripgrep.
Deliberately typed `fix`, not `fix!`, even though a documented long option
disappears. The option was inert -- it never filtered anything -- so no
working behavior is lost, and rtk-ai#3259 set the precedent for removing a
colliding grep short under plain `fix`. Recording the choice here because
silence would read as an oversight.
User-visible consequence: `rtk grep -t rust` goes from silently returning
unfiltered results to surfacing the engine's own error. BSD/macOS grep
prints `invalid option -- t`; GNU grep prints `invalid option -- 't'`.
`--max`/`-m` is NOT part of this change. rtk-ai#3259 removed that short already;
this branch is rebased onto it and preserves its rationale comment verbatim.
The struct NOTE deliberately does not claim to finish the job. clap's auto
`-h` still shadows grep/rg's --no-filename (that is rtk-ai#2532's scope), and
removing a clap short does not make a letter safe end to end: search.rs
shares one VALUE_FLAGS_SHORT table across both engines, so a letter taking
a value in rg but not grep still eats the next token inside a cluster --
`rtk grep -rt FOO .` consumes FOO as -t's value regardless of clap.
Adds clap-layer tests pinning the routing, including characterization tests
for behavior that was previously untested.
2 tasks done
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
2 participants
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.
Summary
rtk grep's-mshort (derived from--max, a display cap) collided with GNU grep where-m N=--max-count(stop after N matches per file).rtk grep -m N <pat>set RTK's display cap instead of forwarding--max-count, so grep never actually stopped early — a silent semantic mismatch (no parse error).-mnow flows toextra_argsand is forwarded verbatim to the realgrep/rg(it's not inhas_format_flag, so it takes the normal engine path, not passthrough), which applies genuine--max-countwhile RTK still compacts. This makesGrepconsistent withRg, which never had the short and already forwarded-mcorrectly.--maxkeeps its long form anddefault_value = "200".-mshort from the grep options table and correct its stale default (50→200, matching the code).Test plan
cargo fmt --all && cargo clippy --all-targets && cargo test— fmt clean, clippy 0 warnings, 2502 passed / 0 failed.test_try_parse_grep_dash_m_is_max_count— clap parsesgrep -m 5 <pat> <file>, keepsmaxat 200, puts-m 5inextra_args.dash_m_max_count_matches_grep_n— end-to-end:rtk grep -m Nis byte-identical togrep -n -m N(-mleading, trailing, and N ≥ total).dash_m_max_count_is_per_file_like_grep_n— multi-file, since--max-countis per file.rtk grep -m 2 . <file>showed every line (-mswallowed); after, it stops at 2 likegrep -m 2.