diff --git a/src/main.rs b/src/main.rs index 992f865a2..6af291cea 100644 --- a/src/main.rs +++ b/src/main.rs @@ -322,6 +322,12 @@ enum Commands { /// Show line numbers (always on, accepted for grep/rg compatibility) #[arg(short = 'n', long)] line_numbers: bool, + /// Invert match — show non-matching lines (ripgrep --invert-match). + /// Long-only: the `-v` short form collides with the global `--verbose` + /// flag, so `rtk grep -v` is mapped to this via an argv rewrite before + /// parsing (see `rewrite_grep_invert`). + #[arg(long = "invert-match")] + invert_match: bool, /// Extra ripgrep arguments (e.g., -i, -A 3, -w, --glob) #[arg(trailing_var_arg = true, allow_hyphen_values = true)] extra_args: Vec, @@ -1399,11 +1405,70 @@ where } } +/// `rtk grep -v` must mean ripgrep's `--invert-match`, not the global +/// `--verbose` flag. `verbose` is declared as a global `-v` (main.rs), so clap +/// consumes the flag before it can reach grep's trailing args — making +/// `rtk grep -v foo file` return matching lines instead of inverting (#1477). +/// +/// Rewrite a standalone `-v` token that follows the `grep` subcommand into +/// `--invert-match` so it is forwarded to rg. A `-v` that *precedes* `grep` +/// (e.g. `rtk -v grep ...`) is left untouched — there it genuinely means +/// verbose. Users wanting a verbose grep can still pass `--verbose`. +/// +/// A `--` end-of-options separator after `grep` stops the rewrite: everything +/// past it is a literal pattern/path (e.g. `rtk grep -- -v` searches for the +/// literal string "-v"), matching GNU grep / getopt convention. +/// +/// Limitation: this is a token rewrite, not an arity-aware parser. A bare `-v` +/// used as the *value* of another forwarded flag (e.g. `rtk grep foo . -e -v`) +/// would also be rewritten. In practice rtk grep takes its pattern positionally +/// and does not accept `-e`, so this affects only a degenerate hand-built rg +/// passthrough; the common `-v` / `--invert-match` paths are correct. +fn rewrite_grep_invert(args: Vec) -> Vec { + // Locate the `grep` subcommand: the first non-flag token after argv[0]. + let mut grep_idx = None; + for (i, a) in args.iter().enumerate().skip(1) { + match a.to_str() { + Some("grep") => { + grep_idx = Some(i); + break; + } + // A different subcommand appeared first — nothing to rewrite. + Some(s) if !s.starts_with('-') => break, + // A leading global flag (e.g. `-v`, `--no-track`) — keep scanning. + Some(_) => continue, + // Non-UTF-8 token before the subcommand — bail out conservatively. + None => break, + } + } + let Some(gi) = grep_idx else { return args }; + + let mut past_separator = false; + args.into_iter() + .enumerate() + .map(|(i, a)| { + if i <= gi || past_separator { + return a; + } + if a.as_os_str() == "--" { + past_separator = true; + return a; + } + if a.as_os_str() == "-v" { + OsString::from("--invert-match") + } else { + a + } + }) + .collect() +} + fn run_cli() -> Result { // Fire-and-forget telemetry ping (1/day, non-blocking) core::telemetry::maybe_ping(); - let cli = match Cli::try_parse() { + let argv = rewrite_grep_invert(std::env::args_os().collect()); + let cli = match Cli::try_parse_from(&argv) { Ok(cli) => cli, Err(e) => { if matches!(e.kind(), ErrorKind::DisplayHelp | ErrorKind::DisplayVersion) { @@ -1799,17 +1864,26 @@ fn run_cli() -> Result { context_only, file_type, line_numbers: _, // no-op: line numbers always enabled in grep_cmd::run - extra_args, - } => grep_cmd::run( - &pattern, - &path, - max_len, - max, - context_only, - file_type.as_deref(), - &extra_args, - cli.verbose, - )?, + invert_match, + mut extra_args, + } => { + // Forward `--invert-match` to ripgrep. The flag is parsed natively + // (so position, `--`, and arity are handled by clap); we append it + // to extra_args, which grep_cmd::run passes through to rg. + if invert_match { + extra_args.push("--invert-match".to_string()); + } + grep_cmd::run( + &pattern, + &path, + max_len, + max, + context_only, + file_type.as_deref(), + &extra_args, + cli.verbose, + )? + } Commands::Init { global, @@ -2672,6 +2746,128 @@ mod tests { assert!(result.is_ok(), "git status should parse successfully"); } + // --- #1477: `rtk grep -v` means --invert-match, not --verbose --- + + fn rw(args: &[&str]) -> Vec { + rewrite_grep_invert(args.iter().map(OsString::from).collect()) + .into_iter() + .map(|a| a.to_string_lossy().into_owned()) + .collect() + } + + #[test] + fn test_grep_invert_rewrites_leading_v() { + assert_eq!( + rw(&["rtk", "grep", "-v", "foo", "file"]), + ["rtk", "grep", "--invert-match", "foo", "file"] + ); + } + + #[test] + fn test_grep_invert_rewrites_trailing_v() { + // Position-independent: grep appends extra args to rg either way. + assert_eq!( + rw(&["rtk", "grep", "foo", "file", "-v"]), + ["rtk", "grep", "foo", "file", "--invert-match"] + ); + } + + #[test] + fn test_grep_invert_preserves_verbose_before_grep() { + // A `-v` before the subcommand is the real global verbose flag. + assert_eq!( + rw(&["rtk", "-v", "grep", "foo", "file"]), + ["rtk", "-v", "grep", "foo", "file"] + ); + } + + #[test] + fn test_grep_invert_leaves_other_commands_untouched() { + assert_eq!( + rw(&["rtk", "read", "-v", "file"]), + ["rtk", "read", "-v", "file"] + ); + } + + #[test] + fn test_grep_invert_stops_at_separator() { + // `--` ends option processing: a following `-v` is a literal pattern, + // not invert-match (matches GNU grep: `grep -- -v` searches for "-v"). + assert_eq!( + rw(&["rtk", "grep", "--", "-v", "file"]), + ["rtk", "grep", "--", "-v", "file"] + ); + // A `-v` before `--` still inverts; only post-separator tokens are literal. + assert_eq!( + rw(&["rtk", "grep", "-v", "foo", "--", "-v"]), + ["rtk", "grep", "--invert-match", "foo", "--", "-v"] + ); + } + + #[test] + fn test_grep_invert_only_exact_v_token() { + // Combined/long flags are not a standalone `-v`; leave them alone. + assert_eq!( + rw(&["rtk", "grep", "-iv", "foo"]), + ["rtk", "grep", "-iv", "foo"] + ); + assert_eq!( + rw(&["rtk", "grep", "--verbose", "foo"]), + ["rtk", "grep", "--verbose", "foo"] + ); + } + + #[test] + fn test_grep_invert_parses_into_invert_match_flag() { + // End-to-end: after the rewrite, clap parses `-v` (now `--invert-match`) + // into the native invert_match flag — not the global verbose counter. + let argv = rewrite_grep_invert( + ["rtk", "grep", "-v", "foo", "file"] + .iter() + .map(OsString::from) + .collect(), + ); + let cli = Cli::try_parse_from(&argv).unwrap(); + match cli.command { + Commands::Grep { + pattern, + path, + invert_match, + .. + } => { + assert_eq!(pattern, "foo"); + assert_eq!(path, "file"); + assert!(invert_match, "`-v` must set the invert_match flag"); + } + _ => panic!("expected grep subcommand"), + } + assert_eq!( + cli.verbose, 0, + "`-v` must not bump the global verbose count" + ); + } + + #[test] + fn test_grep_invert_native_long_flag_leading() { + // The native `--invert-match` flag must parse *before* the positionals. + // Without the dedicated field it is an UnknownArgument here (extra_args + // is trailing_var_arg), which forced a fallback to raw grep. + let cli = Cli::try_parse_from(["rtk", "grep", "--invert-match", "foo", "file"]).unwrap(); + match cli.command { + Commands::Grep { + pattern, + path, + invert_match, + .. + } => { + assert_eq!(pattern, "foo"); + assert_eq!(path, "file"); + assert!(invert_match); + } + _ => panic!("expected grep subcommand"), + } + } + #[test] fn test_try_parse_init_agent_hermes() { let cli = Cli::try_parse_from(["rtk", "init", "--agent", "hermes"]).unwrap();