Skip to content

fix(search): stop rtk grep from claiming the engine's short flags - #3664

Closed
dwin-gharibi wants to merge 1 commit into
rtk-ai:developfrom
dwin-gharibi:fix/grep-flag-shadowing
Closed

dwin-gharibi wants to merge 1 commit into
rtk-ai:developfrom
dwin-gharibi:fix/grep-flag-shadowing

Conversation

@dwin-gharibi

Copy link
Copy Markdown

Closes #3663.

Summary

  • rtk grep no longer claims -l, -m or -t — those letters are grep's
    --files-with-matches, grep's --max-count and rg's --type. The knobs keep their long
    names (--max-len, --max, --file-type).
  • disable_help_flag on Grep and Ls, so clap stops claiming -h: it is --no-filename
    for grep and --human-readable for ls, not a help request. Follows the existing
    Commands::Psql precedent.
  • -h is still treated as help for rg (where it genuinely is), and for grep it now
    suppresses the filename prefix in the compressed output, since RTK forces -H on the engine
    for its own parsing.

Test plan

  • cargo fmt --all && cargo clippy --all-targets && cargo test
  • Manual testing: rtk <command> output inspected

Unit tests (added, TDD — red before green)

src/main.rs:

  • grep_does_not_swallow_engine_short_flags — -l, -m, -t all reach extra_args.
    Fails on develop.
  • dash_h_reaches_the_proxied_tool — rtk grep -h … and rtk ls -h parse instead of
    triggering clap's help. Fails on develop.
  • grep_long_options_still_parse — control: --max-len 40 --max 5 still bind rtk's own
    fields. Passes before and after.

src/cmds/system/search.rs:

  • no_filename_flag_suppresses_the_prefix — -h, --no-filename and the -rh cluster
    suppress the filename prefix; -H still forces it on. Fails on develop.

Behavioural check against real grep / ls

Command Before After
grep -l 404 a.log b.log no output, exit 1 2 filenames, identical
grep -m 1 hit m.txt 4 lines 1 line, identical
grep -m 2 hit m.txt 4 lines 2 lines, identical
grep -h 404 a.log b.log rtk help page, exit 0 2 lines, no prefix, identical
grep -hn 404 a.log b.log already correct identical
grep --no-filename 404 a.log b.log already correct identical
ls -h rtk help page directory listing

Controls confirming nothing else moved: grep 404 a b (prefix still shown), grep -H 404 a,
grep -n 404 a, grep -c 404 a b, grep -l GET a b, grep -m1 hit m.txt — all identical
before and after. rtk grep --max-len 40 --max 5 404 a.log still honours rtk's own options.

Suite

cargo test --all: 2629 passed, 0 failed (2625 on develop, +4 from this PR).
cargo clippy --all-targets: clean. cargo fmt --all: clean.

Commands::Grep declared -l (--max-len), -m (--max) and -t (--file-type),
and no proxy subcommand except Psql disabled clap's automatic -h. Those
letters belong to the proxied tool: -l is grep's --files-with-matches, -m
its --max-count, -t rg's --type, and -h grep's --no-filename.

When clap could parse the next token as the option's value it consumed
both and the engine never saw the flag, so grep -l 404 a.log b.log
searched b.log for the literal a.log and returned nothing, and grep -m 1
returned every match. When it could not, clap errored and run_fallback ran
the raw command, which is why the bug looked intermittent.

Drop the short forms, keep the long ones, and disable the help flag on Grep
and Ls. -h stays a help request for rg, where it genuinely is one; for grep
it now suppresses the filename prefix that rtk adds via its forced -H.
Copilot AI lite review requested due to automatic review settings August 23, 2026 18:01

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@KuSh

KuSh commented Sep 27, 2026

Copy link
Copy Markdown
Collaborator

#3259 (merged 2026-08-29) dropped the -m short, #2628 (merged 2026-08-31) dropped -l and -t, and #3681 (merged 2026-09-12) moved search.rs onto the shared arg tokenizer, where -h already means no-filename for grep, so the search.rs hunks here target code that has been replaced. What develop still lacks is #[command(disable_help_flag = true)] on the Grep and Ls subcommands in src/main.rs: rtk grep -h 404 a.log b.log and rtk ls -h still print rtk's help and exit 0, which is #3663. Could you rebase onto current develop and narrow the PR to those two attributes plus the dash_h_reaches_the_proxied_tool parse test? It conflicts with develop in src/main.rs and search.rs. Thanks for the fix.

@KuSh

KuSh commented Oct 18, 2026

Copy link
Copy Markdown
Collaborator

Thank you for this. The fix landed through #4348 (merged 2026-10-14), which stops rtk claiming -h on grep and ls, together with #3259 (2026-08-28) and #3681 (2026-09-12), which free -m, -l and -t for the engine and honour -h and --no-filename in the compressed output. The parse and display tests this PR adds are already covered by tests on develop, and the commands in your table now behave like native grep and ls. If a case is still missed, please comment or reopen.

@KuSh KuSh closed this Oct 18, 2026
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.

rtk grep claims -h, -l, -m and -t, which belong to grep/rg

3 participants