fix(grep): stop -l/-t shadowing native grep flags

.claude/hooks/rtk-suggest.sh

@@ -75,7 +75,11 @@ elif echo "$FIRST_CMD" | grep -qE '^cargo\s+fmt(\s|$)'; then
 elif echo "$FIRST_CMD" | grep -qE '^cat\s+'; then
   SUGGESTION=$(echo "$CMD" | sed 's/^cat /rtk read /')
 elif echo "$FIRST_CMD" | grep -qE '^(rg|grep)\s+'; then
-  SUGGESTION=$(echo "$CMD" | sed -E 's/^(rg|grep) /rtk grep /')
+  # Keep the engine. Mapping `rg` -> `rtk grep` swaps ripgrep for GNU grep, so
+  # rg-only flags break: `rg -t rust FOO` became `rtk grep -t rust FOO`, which
+  # grep rejects with `invalid option -- t`. `rtk rg` is the ripgrep-backed
+  # variant and takes rg's flags verbatim.
+  SUGGESTION=$(echo "$CMD" | sed -E 's/^rg /rtk rg /; s/^grep /rtk grep /')
 elif echo "$FIRST_CMD" | grep -qE '^ls(\s|$)'; then
   SUGGESTION=$(echo "$CMD" | sed 's/^ls/rtk ls/')
 elif echo "$FIRST_CMD" | grep -qE '^tree(\s|$)'; then

docs/usage/FEATURES.md

@@ -215,14 +215,14 @@ rtk grep <pattern> [chemin] [options]
 
 | Option | Court | Defaut | Description |
 |--------|-------|--------|-------------|
-| `--max-len` | `-l` | 80 | Longueur maximale de ligne |
+| `--max-len` |  | 80 | Longueur maximale de ligne (pas de raccourci, `-l` est reserve a `grep --files-with-matches`) |
 | `--max` |  | 200 | Nombre maximum de resultats (pas de raccourci, `-m` est reserve a `grep --max-count`) |
 | `--context-only` |  | non | Afficher uniquement le contexte du match (pas de raccourci, `-c` est reserve a `grep --count`) |
-| `--file-type` | `-t` | tous | Filtrer par type (ts, py, rust, etc.) |
-| `--line-numbers` | `-n` | oui | Numeros de ligne (toujours actif) |
 
 Les arguments supplementaires sont transmis a `rg` (ripgrep). Les flags qui changent le format de sortie (`-c`, `-l`, `-L`, `-o`, `-Z`) passent directement a `rg`/`grep` sans filtrage RTK.
 
+Il n'y a pas d'option `--file-type` : `-t TYPE` est propre a ripgrep et est transmis tel quel au moteur. `rtk rg -t rust` filtre donc par type, tandis que `rtk grep -t rust` remonte l'erreur de grep lui-meme (`invalid option -- 't'`), grep n'ayant pas ce flag.
+
 **Avant / Apres :**
 ```
 # rg "fn run" (20 lignes)                   # rtk grep "fn run" (10 lignes)

scripts/benchmark.sh

@@ -215,7 +215,7 @@ bench "git show" "git show HEAD --stat 2>/dev/null || true" "$RTK git show HEAD
 section "grep"
 bench "grep fn" "grep -rn 'fn ' src/ || true" "$RTK grep -rn 'fn ' src/"
 bench "grep struct" "grep -rn 'struct ' src/ || true" "$RTK grep -rn 'struct ' src/"
-bench "grep -l 40" "grep -rn 'fn ' src/ || true" "$RTK grep -rn 'fn ' src/ -l 40"
+bench "grep --max-len 40" "grep -rn 'fn ' src/ || true" "$RTK grep --max-len 40 -rn 'fn ' src/"
 bench "grep -c" "grep -ron 'fn ' src/ || true" "$RTK grep -rc 'fn ' src/"
 
 # ===================

scripts/test-all.sh

@@ -257,9 +257,29 @@ fi
 
 section "Grep"
 
-assert_ok      "rtk grep pattern"             rtk grep "pub fn" src/
-assert_contains "rtk grep finds results"      "pub fn" rtk grep "pub fn" src/
-assert_ok      "rtk grep with file type"      rtk grep "pub fn" src/ -t rust
+# All three need -r: `grep PATTERN <dir>` without it exits 2 ("Is a directory"),
+# which made the first two assertions red and the third one INERT -- assert_fails
+# passed on the directory error whether or not `-t` was handled correctly.
+assert_ok      "rtk grep pattern"             rtk grep -r "pub fn" src/
+assert_contains "rtk grep finds results"      "pub fn" rtk grep -r "pub fn" src/
+# `-t` is rg-only. rtk used to swallow it via its own --file-type option, which
+# never reached the engine, so the old assertion here pinned a silent no-op.
+#
+# This form is deliberate and fragile to get right. The pattern MUST match and
+# the path MUST be a single file with `-t` FIRST, so that the only reason the
+# command can fail is `-t` itself:
+#   - re-add --file-type with `short = 't'`  -> -t binds, "fn main" matches,
+#                                               exit 0, and assert_fails fires.
+#   - as shipped                             -> -t reaches grep, exit 2.
+# A directory path (exit 2, "Is a directory"), a non-matching pattern (exit 1),
+# or `-t` after another flag (trailing_var_arg swallows it before clap sees it)
+# all make this assertion inert -- it passes either way.
+assert_fails   "rtk grep -t rejected by grep"  rtk grep -t rust "fn main" src/main.rs
+if command -v rg >/dev/null 2>&1; then
+    assert_ok  "rtk rg with file type"         rtk rg "pub fn" src/ -t rust
+else
+    skip_test  "rtk rg with file type"         "rg not installed"
+fi
 
 section "Grep (extra args passthrough)"
 

src/main.rs

@@ -313,8 +313,47 @@ enum Commands {
 
     /// Compact grep - strips whitespace, truncates, groups by file
     Grep {
+        // NOTE: rtk's own tuning options here are intentionally long-only.
+        // Their natural short forms collide with native grep/rg flags of the
+        // same letter, which users routinely pass. A short option here shadows
+        // the native flag and either crashes clap or silently applies the wrong
+        // semantics, defeating the `extra_args` passthrough. Keep them long-only
+        // so native flags reach src/cmds/system/search.rs, the shared filter
+        // behind both `rtk grep` and `rtk rg`. (Continues the `-v`/`-n`/pattern/
+        // path cleanup from 84616d1.)
+        //
+        // This does NOT clear every shadowed short. Clap's auto `-h` still wins
+        // over grep/rg's --no-filename before the first positional, and after it
+        // `-h` reaches search.rs's --version/--help check and execs unfiltered.
+        // Help handling is #2532's scope, not this variant's -- do not "finish"
+        // it here by adding disable_help_flag without reading that PR first.
+        //
+        // Removing a short here also does not make the flag safe end-to-end.
+        // search.rs shares one VALUE_FLAGS_SHORT table across both engines, so a
+        // letter that takes a value in rg but not grep (`-t`, `-T`, `-r`) still
+        // eats the next token inside a cluster: `rtk grep -rt FOO .` consumes
+        // FOO as -t's value regardless of what clap does. That is a search.rs
+        // defect, tracked separately.
         /// Max line length
-        #[arg(short = 'l', long, default_value = "80")]
+        // No short: `-l` is grep's --files-with-matches. Binding it to --max-len
+        // (a `usize`) split behavior on whether the PATTERN parsed as a number.
+        //
+        // Non-numeric pattern: clap rejected it, run_fallback re-ran raw grep,
+        // and the user got the right answer. Wasteful, not harmful -- and note
+        // `-l` is a has_format_flag passthrough on the fixed path too, so no
+        // compaction was "lost"; there is none to lose for this flag.
+        //
+        // Numeric pattern: clap ACCEPTED it. `rtk grep -l 8080 a.txt b.txt` set
+        // max_len=8080 and left a.txt as the pattern, b.txt as the only path --
+        // no error, no fallback, silently empty output and exit 1 on a file that
+        // does match. With a single file it is worse: the filename becomes the
+        // pattern, no path is left, and rtk falls back to reading stdin -- which
+        // exits 1 on /dev/null and hangs outright on an interactive terminal.
+        // That silent wrong answer, reached through a hook the user never opted
+        // into, is the real reason this short had to go.
+        // Pinned by tests::test_grep_parse_files_with_matches_l (clap layer) and
+        // numeric_pattern_with_files_with_matches_flag (end to end).
+        #[arg(long, default_value = "80")]
         max_len: usize,
         /// Max results to show
         // No short: `-m` is GNU grep's --max-count (stop after N matches per
@@ -328,9 +367,11 @@ enum Commands {
         /// Show only match context (not full line)
         #[arg(long)]
         context_only: bool,
-        /// Filter by file type (e.g., ts, py, rust)
-        #[arg(short = 't', long)]
-        file_type: Option<String>,
+        // No --file-type / `-t`: `-t TYPE` is rg's type filter, and the option
+        // never did anything under `rtk grep` -- it was destructured away at the
+        // call site and never reached the engine. Removed rather than left as a
+        // parse-only no-op; `rtk rg -t rust` still filters by type, since it
+        // forwards `-t` to real ripgrep.
         /// Pattern, path, and any grep/rg flags (e.g. -v, -i, -A 3, --glob, --version)
         #[arg(trailing_var_arg = true, allow_hyphen_values = true)]
         extra_args: Vec<String>,
@@ -2005,7 +2046,6 @@ fn run_cli() -> Result<i32> {
             max_len,
             max,
             context_only,
-            file_type: _,
             extra_args,
         } => search::run(
             search::Engine::Grep,
@@ -3695,4 +3735,187 @@ mod tests {
             _ => panic!("Expected Init command"),
         }
     }
+
+    // --- grep argument routing (clap layer) ---
+    //
+    // The `Grep` variant funnels the pattern, path, and every native grep/rg
+    // flag into a single `trailing_var_arg` slot so `src/cmds/system/search.rs`
+    // -- the shared filter behind both `rtk grep` and `rtk rg` -- can parse them
+    // with full grep/rg semantics. Commit 84616d1 ("fix(grep): stabilize
+    // argument parsing") moved to this design but left the struct's own short
+    // options in place, where they still shadowed native flags of the same
+    // letter. These tests pin the routing.
+
+    /// Parse `rtk grep …` and return the captured `extra_args`, or `None` if
+    /// clap rejects the invocation (e.g. a colliding short option mis-binds).
+    fn grep_extra_args(args: &[&str]) -> Option<Vec<String>> {
+        match Cli::try_parse_from(args).ok()?.command {
+            Commands::Grep { extra_args, .. } => Some(extra_args),
+            _ => None,
+        }
+    }
+
+    // Characterization: behavior that must be PRESERVED (passes today, untested
+    // until now). Guards the 84616d1 design against regression.
+
+    #[test]
+    fn test_grep_parse_simple_pattern_path() {
+        assert_eq!(
+            grep_extra_args(&["rtk", "grep", "FOO", "src/"]).unwrap(),
+            vec!["FOO", "src/"]
+        );
+    }
+
+    #[test]
+    fn test_grep_parse_combined_short_cluster() {
+        // `-rn` is a native grep cluster (recursive + line-numbers); it must
+        // reach search.rs intact, not be intercepted by clap.
+        assert_eq!(
+            grep_extra_args(&["rtk", "grep", "-rn", "FOO", "src/"]).unwrap(),
+            vec!["-rn", "FOO", "src/"]
+        );
+    }
+
+    #[test]
+    fn test_grep_parse_value_flag_after_context() {
+        // `-A 3` (after-context) — the value must not be stolen by clap.
+        assert_eq!(
+            grep_extra_args(&["rtk", "grep", "-A", "3", "FOO", "file"]).unwrap(),
+            vec!["-A", "3", "FOO", "file"]
+        );
+    }
+
+    #[test]
+    fn test_grep_parse_invert_match_v_not_shadowed() {
+        // Regression guard for 84616d1: `-v` (invert-match) must reach grep,
+        // not be captured as rtk's top-level verbose flag.
+        assert_eq!(
+            grep_extra_args(&["rtk", "grep", "-v", "FOO", "file"]).unwrap(),
+            vec!["-v", "FOO", "file"]
+        );
+    }
+
+    #[test]
+    fn test_grep_parse_alternation_pattern_intact() {
+        // Extended-regex alternation must pass through untouched (#1436 lineage).
+        assert_eq!(
+            grep_extra_args(&["rtk", "grep", "-nE", "a|b", "file"]).unwrap(),
+            vec!["-nE", "a|b", "file"]
+        );
+    }
+
+    // The fix: native `-l`/`-m`/`-t` must route to search.rs, not be captured by
+    // the struct's own short options. `-l` mis-bound to `max_len: usize`, so clap
+    // rejected the pattern outright; `-m`/`-t` silently swallowed their value with
+    // the wrong semantics. (`-m` was fixed separately in #3259; pinned here too.)
+
+    #[test]
+    fn test_grep_parse_files_with_matches_l() {
+        // Native grep `-l` (files-with-matches). Must parse and pass `-l` through.
+        assert_eq!(
+            grep_extra_args(&["rtk", "grep", "-l", "FOO", "src/"]).unwrap(),
+            vec!["-l", "FOO", "src/"]
+        );
+    }
+
+    #[test]
+    fn test_grep_parse_type_t_forwarded() {
+        // `-t TYPE` is rg-only, so it must reach the engine rather than binding
+        // to an rtk option. search.rs lists `t` in VALUE_FLAGS_SHORT and forwards
+        // it verbatim: under `rtk rg` it filters by type, under `rtk grep` it
+        // surfaces grep's own `invalid option -- 't'` instead of being silently
+        // dropped.
+        assert_eq!(
+            grep_extra_args(&["rtk", "grep", "-t", "rust", "FOO", "src/"]).unwrap(),
+            vec!["-t", "rust", "FOO", "src/"]
+        );
+    }
+
+    #[test]
+    fn test_grep_file_type_option_is_gone() {
+        // `--file-type` was removed, not merely un-shorted. Guard against it
+        // creeping back: an unknown long flag is NOT a clap error here (the
+        // trailing slot accepts it), it must route to extra_args and be the
+        // engine's problem. If someone re-adds the option, clap binds it and
+        // extra_args loses these two tokens.
+        assert_eq!(
+            grep_extra_args(&["rtk", "grep", "--file-type", "rust", "FOO", "src/"]).unwrap(),
+            vec!["--file-type", "rust", "FOO", "src/"]
+        );
+    }
+
+    // --- additional grep characterization (preserved behavior, was untested) ---
+
+    /// Parse `rtk grep …` into the full `Grep` field set for inspection.
+    fn parse_grep(args: &[&str]) -> Result<(usize, usize, bool, Vec<String>), clap::Error> {
+        match Cli::try_parse_from(args)?.command {
+            Commands::Grep {
+                max_len,
+                max,
+                context_only,
+                extra_args,
+            } => Ok((max_len, max, context_only, extra_args)),
+            _ => unreachable!("parsed a grep command"),
+        }
+    }
+
+    #[test]
+    fn test_grep_long_options_bind_before_pattern() {
+        // rtk's tuning knobs are long-only (the short forms were removed to stop
+        // shadowing native grep flags). They must still bind when placed before
+        // the pattern, and only the pattern/path land in extra_args.
+        let (max_len, max, context_only, extra_args) = parse_grep(&[
+            "rtk",
+            "grep",
+            "--max-len",
+            "40",
+            "--max",
+            "5",
+            "--context-only",
+            "FOO",
+            "path",
+        ])
+        .unwrap();
+        assert_eq!(max_len, 40);
+        assert_eq!(max, 5);
+        assert!(context_only);
+        assert_eq!(extra_args, vec!["FOO", "path"]);
+    }
+
+    #[test]
+    fn test_grep_options_after_pattern_stay_in_extra_args() {
+        // Positional gotcha: once the pattern starts the trailing slot, later
+        // tokens — even rtk's own `--max-len` — are NOT parsed as options; they
+        // pass through verbatim. (Native grep ordering: flags come before the
+        // pattern.) The mechanism is `allow_hyphen_values` on `extra_args`, not
+        // `trailing_var_arg` — dropping the latter leaves this behavior intact,
+        // so do not read this test as a guard on `trailing_var_arg`.
+        let (max_len, _, _, extra_args) =
+            parse_grep(&["rtk", "grep", "FOO", "--max-len", "40"]).unwrap();
+        assert_eq!(
+            max_len, 80,
+            "option after pattern must NOT bind (default kept)"
+        );
+        assert_eq!(extra_args, vec!["FOO", "--max-len", "40"]);
+    }
+
+    #[test]
+    fn test_grep_version_routes_to_extra_args() {
+        // `--version` is not a subcommand flag (version isn't propagated), so it
+        // lands in extra_args, and search::run's --version/--help check execs it
+        // against the engine unfiltered.
+        assert_eq!(
+            grep_extra_args(&["rtk", "grep", "--version"]).unwrap(),
+            vec!["--version"]
+        );
+    }
+
+    #[test]
+    fn test_grep_double_dash_consumed_by_clap() {
+        // clap strips the `--` separator before populating trailing_var_arg.
+        // core::args_utils::restore_double_dash re-inserts it at runtime — this
+        // test pins the premise that helper depends on.
+        let (_, _, _, extra_args) = parse_grep(&["rtk", "grep", "--", "-v", "file"]).unwrap();
+        assert_eq!(extra_args, vec!["-v", "file"], "clap must consume the --");
+    }
 }