fix(grep): normalize rg-compatible rewrite flags#1899
Conversation
|
|
There was a problem hiding this comment.
Pull request overview
This PR improves the command-rewrite registry so hook rewrites to rtk grep normalize common grep/rg flag orderings (notably moving compatible flags after <pattern> <path>), reducing clap parse failures and preserving intended searches for common flag shapes.
Changes:
- Add a
grep/rg-specific rewrite path that shell-splits the command, recognizes a set of supported flags, and reorders them into anrtk grep <pattern> [path] <extra_args...>form. - Introduce shell-quoting for rewritten args to produce a safely re-runnable command string.
- Add unit tests covering normalized flag order,
--globnormalization, and conservative skipping for$PATTERN/ empty quoted patterns.
| fn shell_quote_arg(arg: &str) -> String { | ||
| if !arg.is_empty() | ||
| && arg.chars().all(|c| { | ||
| c.is_ascii_alphanumeric() || matches!(c, '_' | '-' | '.' | '/' | ':' | '=' | '~') | ||
| }) | ||
| { | ||
| arg.to_string() | ||
| } else if !arg.is_empty() | ||
| && arg.chars().all(|c| { | ||
| c.is_ascii_alphanumeric() || c.is_ascii_whitespace() || matches!(c, '_' | '-' | '.') | ||
| }) | ||
| { | ||
| format!("\"{}\"", arg) | ||
| } else { | ||
| format!("'{}'", arg.replace('\'', "'\\''")) | ||
| } | ||
| } |
| 'i' | 'o' | 'c' | 'F' | 'w' | 'H' | 'h' => { | ||
| flags.push(format!("-{}", flag)); | ||
| true | ||
| } |
|
cc @pszymkowiak @aeppling — CLA is now signed, PR should be unblocked for review. @pszymkowiak this PR sits in the same hook-rewrite area you recently merged in #1718; the change here is the grep/rg flag-normalization half discussed in #1604/#1833/#1824. I kept it deliberately narrow: it normalizes common grep/rg flag placement before |
hhh2210
force-pushed
the
fix/grep-rg-flag-rewrite
branch
from
59d89be to
41f3551
Compare
|
Addressed the Copilot review points in the latest push:
I also added regression coverage for both cases. |
Keep grep-specific short flag normalization separate from ripgrep so hook rewrites do not change rg semantics. In particular, skip rg -h/-L/-r/-R while preserving compatible rg flags and grep -h/-L behavior.
Summary
grep/rgflags before routing hook rewrites tortk grep, so agent-generated searches stop producing clap parse failures for supported shapes.-rn,-A/-B/-C,-E,-i,-o,-c,-l/-L, and-g/--globby moving compatible flags after thertk greppositionals.Fixes #1824.
Partially addresses #1833 by reducing hook-rewrite fallbacks for common grep/rg flag shapes.
Refs #1604, #236.
Test plan
cargo fmt --all --checkcargo clippy --all-targets -- -D warningscargo testtarget/debug/rtk hook check 'grep -n -A 40 begin{abstract} /tmp/file.tex'->rtk grep 'begin{abstract}' /tmp/file.tex -A 40target/debug/rtk hook check 'grep -ic warning /tmp/bib.log'->rtk grep warning /tmp/bib.log -i -ctarget/debug/rtk hook check 'rg -n "Project Guidelines" --glob CLAUDE.md .'->rtk grep "Project Guidelines" . --glob CLAUDE.mdtarget/debug/rtk hook check 'rg --version'-> no rewritetarget/debug/rtk hook check 'rg "$PATTERN" src'-> no rewritetarget/debug/rtk hook check "rg '' src"-> no rewriteLocal environment used for manual verification: macOS 26.5 (build 25F71), arm64, Apple M4. CI is expected to cover the repository's broader platform matrix.
Notes
This intentionally stays scoped to rewrite normalization. It does not attempt to redesign
rtk grep, changefind, or close the non-existent-subcommand portion of #1604.For #1833 specifically, this PR reduces fallback/parse failures in the hook-generated command path, but it does not fully extend the
rtk grepargument parser, implement the GNU-grep literal-Fpreservation rule when-Eis absent, or resolve the directrtk grep -l/--max-lencollision.