std::process::Command wraps an argument in quotes only when it contains a
space or a tab, so an argument like a"b reaches the raw command line bare.
MSVCRT and UCRT parse that back correctly, Cygwin and MSYS do not, which is
why `rtk grep -c '"type"'` silently matched nothing (#3727). Re-encode with
libuv's quote_cmd_arg rules, and only when the argument contains a quote, so
every argument that works today keeps its exact bytes. Batch programs stay
on the old path: raw_arg would bypass std's CVE-2024-24576 mitigation, and
gradlew.bat and mvnw.cmd both reach run_passthrough.
tree forwards the user's own arguments the same way (#4101). MSYS2 ships an
MSYS-linked tree, so it belongs with ls, wc and find rather than with the
native-only spawn sites, which are deliberately left alone.
`rtk gain` printed its own missing-hook line from a bare `status()` match,
outside `maybe_warn`, so neither `hooks.suppress_hook_warning` nor
`RTK_SUPPRESS_HOOK_WARNING` reached it — on the report the no-hook audience
reads most. Gate that arm on the same helper; the outdated-hook prompt stays
visible, as it does everywhere else.
The new unit tests exercise the parser alone, so folding env and config
together with `||` — which drops the falsy force-off — left the whole suite
green. Drive the composition through the binary instead, and give the env
table the `=0` row that overrides the config flag.
The suite pins `XDG_CONFIG_HOME`/`XDG_DATA_HOME` to the temporary home and
seeds the config in both the XDG and the macOS location, so the loader cannot
resolve past it: `dirs::config_dir` is `~/.config` on Linux but
`~/Library/Application Support` on macOS, and a runner that exports either
`XDG_*` reached the developer's own config before.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pins both directions: the commands that report hook state themselves stay
quiet, and the filtered commands still warn. Also pins the reason the
suppression is worth having — `maybe_warn` touches the once-a-day marker, so
warning on a meta command would leave the next filtered command silent.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`rewritten_head_spellings_match_native_on_non_utf8_files` isolates
`CLAUDE_CONFIG_DIR` but reads the ambient `XDG_CONFIG_HOME`, so a config
carrying `exclude_commands = ["head", "tail"]` suppresses the rewrite and
the exit-code-3 assertion fails. Clean runners have no such config, so the
test passes in CI and fails only for the developers most likely to have
opted out of these very rewrites.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Filtering a failing `gh pr checks` makes the summary the only thing the user
sees, so a bucket it does not count disappears. `parse_pr_check_line` dropped
every row that was not pass/fail/pending, which turned a cancelled run -- the
usual outcome when a new push supersedes CI -- into "Passed: 0, Failed: 0",
and left 5 of PR 3982's 15 checks out of the totals.
Count the remaining buckets and report them, so the summary always adds up to
the checks gh listed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Six cases over a fake tool on PATH, covering both halves of the contract: the
tool's own stderr has to reach the user, and rtk must not invent a stdout
message in its place.
The prettier case is the one that is easy to get wrong. It writes its report to
stderr and nothing to stdout even on a successful run, so a filter's empty-input
placeholder ("Error: prettier produced no output") would be printed over a run
that worked. An empty stdout from a command that printed nothing on stdout is
the correct answer.
Also pinned: stderr is not replayed onto stdout on top of being forwarded, a
genuinely silent command stays silent, forwarding is not golangci-specific, and
exit codes propagate -- including golangci-lint's exit 1, which means issues
were found and is reported without failing the build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
develop rewrote `run_show`'s routing underneath this branch (#3265, eight
commits) into `ShowRoute` with a `cat-file` probe for blob detection. That
structure is newer and better than what this branch had, so it is kept whole
and this branch's contribution is folded into it rather than the other way
round.
`commit_or_stat_route` now classifies with the tokenizer instead of scanning
strings. develop matched `a == "--stat"` and `a.starts_with("--pretty")`,
which reads a pathspec named `--stat` past the boundary as the flag and
`--prettyish` as `--pretty`, and covers three stat spellings where the set of
shapes the compaction cannot render is larger. `git show -- --stat` takes the
compact path again.
`consumes_next_token_as_value` is reimplemented on `log_takes_value` rather
than restored as its own table. develop's copy is a subset -- it omits
`--max-count`, `--ignore-matching-lines`, `--min-age`, `--max-age` and
`--stat-graph-width` -- and keeping both would be the two-lists-drift this
branch exists to remove. Its callers are unchanged, so blob-show keeps its
own grammar rules; the attachment distinction (`-M50` attaches, `-M 50` does
not) is what the tokenizer adds.
This branch's `is_blob_show_arg` and free-positional blob pre-filter are
dropped in favour of develop's, which also handles index and merge-stage
blobs (`:file`, `:2:file`) and probes with `cat-file` rather than trusting
the shape of the argument.
Verified: seven blob-show invocations byte-identical to develop, including
the `-wG a:b HEAD:blob` cluster their walker exists for; 3536 unit tests and
every integration suite green; clippy clean.
develop's `show_positionals` carries a `TODO(after #3681)` to replace its
hand-rolled short-cluster walk with the ValueSpec factorization. Left alone
deliberately -- that is follow-up on develop, as its author intended, not
something to change inside this merge.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`rtk hook check` called `registry::rewrite_command` directly, with no
permission verdict and none of the gates the hooks apply. It therefore
reported a rewrite for command substitutions, file redirects and heredocs
that both hook paths refuse to touch -- the diagnostic disagreed with the
thing it exists to diagnose, and did so in the direction that matters, by
claiming RTK would rewrite a command it deliberately leaves alone.
Route it through `hooks::decision` so it answers the same question, and
report a deny rule distinctly from "no rewrite" rather than collapsing both
into one message. Both still exit 1.
That makes the answer agent-dependent, so `--agent` stops being discarded.
`AgentPath` records what actually differs between agents, which is whose
permission rules their hook consults: the six that decide in-process via
`rtk hook <agent>` use their own host's rules, the five whose plugin shells
out to `rtk rewrite` get Claude's (that entry point cannot be told who is
asking), and the six that install only a rules file have no hook and so no
rules at all. Every install target resolves -- including `codex` and
`openclaw`, which are install flags rather than `AgentTarget` variants --
and only a genuine typo is rejected.
What does *not* differ is a rewrite that changed nothing: every agent
discards it, the in-process hosts in `hook_cmd` and the others in their own
plugin, since `hooks/opencode/rtk.ts`, `hooks/pi/rtk.ts` (shared with omp)
and hermes' `__init__.py` all gate on `rewritten != command`. `AgentPath`
suppresses it for every variant. Only the bare `rtk rewrite` CLI reports the
no-op, and no agent consumes that answer raw.
Consulting no rules and ignoring `--agent` is what made the diagnostic
contradict every host: under a Claude deny rule for `git status` it reported
`rtk git status` while `rtk hook claude` refused the command outright; it
reported a rewrite for `rtk git status`, which no agent applies; and since
`--agent` selected nothing, the answer described no host in particular.
The expectations pinned in the characterization commit are updated here, in
the same commit, so the behavior change is visible as a diff rather than as a
test that quietly stopped asserting. That harness now also asserts the hook
exits 0 and never panics: a crash produces empty stdout, which would let every
"expect no output" assertion pass vacuously. The agent list is derived from
`AgentTarget::value_variants()` so a new variant fails the test instead of
silently becoming unanswerable.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`rtk rewrite`'s exit-code protocol is a public contract -- the claude and
cursor shell hooks, the opencode and pi TypeScript plugins, the hermes
Python adapter and openclaw all branch on it -- but no Rust test ever
called `run()`. `rewrite_cmd`'s own `exit_code_protocol` module asserts
against a locally re-implemented `expected_exit_code()` table, so the real
mapping could change without a single failure, including the #1155
invariant that a `Default` verdict must exit 3 and never 0.
Add an integration test that spawns the binary in a sandboxed
HOME/XDG_CONFIG_HOME/CLAUDE_CONFIG_DIR with project-level permission rules,
and pins the actual (exit code, stdout) pairs for allow, ask, deny,
compound deny, passthrough, default, compound rewrite, fd-dup redirect,
unattestable constructs and heredocs.
Alongside it, pin the two decision paths against each other on one corpus.
`rtk rewrite` and `rtk hook claude` answer the same question through two
independently written flows; the corpus asserts they agree, and a separate
test pins the one place they don't -- an already-RTK-prefixed command,
which the in-process hook defers on and `rtk rewrite` reports as an
ask-rewrite with the command unchanged. Modelled on registry.rs's
`segmenter_consistency` module.
Also pin `rtk hook check`, which had no test at all. It calls
`rewrite_command` directly with no verdict and none of the hooks' gates, so
it reports a rewrite for command substitutions and file redirects that both
hook paths refuse to touch.
No source change: this characterizes today's behavior so the decision-flow
consolidation can be shown to preserve it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A non-numeric pattern stops at clap's parse error and falls back to raw
grep, so rtk and grep agree byte-for-byte whether or not `-l` is bound to
--max-len: the three existing cases still pass with the bug reintroduced.
Only a numeric pattern reaches the wrong answer -- rtk printed nothing and
exited 1 where grep listed the file.
Drop the clap-level test and the comment sentence with it. Develop's
test_grep_parse_files_with_matches_l already fails under the same mutation,
and -L never had a short binding on --max-len, so src/main.rs is untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The blocker hid for three rounds because the fuzzer only ever paired a colon-carrying
flag cluster (`-wG x:y`, `-pS url:1`, …) with a blob-LESS `HEAD` (the CommitMisroute
trap). It never placed such a cluster BEFORE a real blob, which is exactly where the
walker gap bit.
Adds four crossed kinds:
* ClusterLargeUtf8 — cluster + large UTF-8 blob → MUST window (0 silent loss);
* ClusterLatin1 — cluster + Latin-1 blob → byte-identical to `git show <same args>`,
no U+FFFD mojibake;
* TextconvLargeUtf8 / PathspecLargeUtf8 — `--textconv` or a trailing `-- <path>` →
never windowed, byte-identical passthrough.
Plus a dedicated end-to-end test that `-wG x:y HEAD:large.txt` windows and that its
recovery hint (which names only the blob arg) reconstructs the blob byte-for-byte. The
extended fuzzer FAILS on the pre-fix code (verified: cluster windowing regression +
Latin-1 fidelity break) and passes on the fix. Seed and iteration count unchanged.
Hermetic temp repo with objects of every relevant type (large/small UTF-8,
Latin-1 and UTF-16 blobs, a tree, commits). A fixed-seed PRNG generates 600
'git show <random flags> <arg>' invocations and checks rtk's classification
(windowed / passthrough / commit-diff) against 'git cat-file -t' ground truth:
0 misroutes (never window a non-blob), 0 silent losses (a large recoverable
UTF-8 blob always windows), and byte-identical passthrough for declined blobs.
A companion test proves the recovery hint reconstructs a windowed blob
byte-for-byte.
The blob-vs-non-blob classifier tried to mirror git's flag grammar and was
wrong three rounds running: it missed --ignore-matching-lines and the combined
short-flag clusters (-pS, -wG, -pI, -pwG, -wpG) whose tail git re-parses, so a
colon-carrying flag VALUE was windowed as if it were a blob.
Ask git instead. show_route stays a cheap pure pre-filter (does the first
positional look like rev:path?); run_show then confirms authoritatively with
'git cat-file -t <arg>' via git_cmd(global_args) and only windows an exact
'blob'. tree/commit/tag or a non-zero exit route to the normal commit-diff /
passthrough path. This subsumes and removes the raw.starts_with("tree ")
content-sniff (a false positive for Newick/NEXUS blobs). Marked with a
TODO(after #3681) so a flag pre-filter can later avoid the probe on the common
path.
Fidelity invariant ('byte-identical unless we successfully windowed'): window
ONLY content that is valid UTF-8 byte-for-byte, so the 'git show rev:path |
tail -n +N' hint always reconstructs exactly. Latin-1, UTF-16/BOM, binary and
lossy-UTF-8 now pass through byte-identically instead of being transcoded and
windowed (which broke recovery and could inflate output past git). This orphans
decode_output and its Latin-1/CJK/BOM helpers in core::stream, which are
removed. The recovery hint now carries the command's global args (-C, -c,
--git-dir, --work-tree) so it is runnable from outside the repo.
N1: emit small blobs (<= 8 KiB) as git's raw bytes with no decode, so a
passthrough blob keeps its BOM.
N2: derive the recovery hint from the blob arg itself
(`git show <rev>:<path> | tail -n +N`) instead of a tee file, so the
windowing works for blobs larger than the tee's max_file_size cap; drop
the MaxRecoverable cap and tee::max_recoverable_bytes entirely.
N3: track savings against the raw bytes git wrote, not the transcoded text,
via TimedExecution::track_bytes (avoids overstating the reduction).
N5: decode captured stderr with decode_process_output, not from_utf8_lossy.
N6: skip windowing when stdout is not a TTY — truncation changes the bytes a
piped consumer receives, not just the on-screen presentation.
N8: window the token-savings test against a committed deterministic fixture
instead of include_str!(main.rs).
N9: replace bare see-also doc pointers with the real rationale.
N10: surface git's stderr on success too, not only on failure.
N4/N7 do not apply: with N2 this path no longer writes a tee file, so there
is no tee-write ordering to fix nor a tee.mode to honour here.
`git show <rev>:<path>` dumps raw file content that RTK previously passed
through unfiltered (0% savings). Large text blobs are now capped to an 8 KiB
byte budget with a `tail -n +N` recovery pointer (the full blob is tee'd),
mirroring the commit-diff path. Small blobs, tree listings, binary, and
blobs too large for the recovery file to store intact are passed through
unchanged.
Blobs are captured as raw bytes and decoded encoding-aware: ISO-8859 /
Latin-1 files (e.g. Oracle PL/SQL packages) are transcoded losslessly
instead of being corrupted into U+FFFD by lossy UTF-8 decoding, which also
unblocks their compression. Ambiguous single-byte encodings (CP1252 range)
and binary content are passed through raw rather than guessed.
Address review:
- benchmark.sh: with trailing_var_arg on extra_args, --max-len after the
positionals was swallowed and forwarded raw to grep (unknown option ->
bench FAIL). Move it before -rn/pattern so rtk parses it; verified 320KB
-> 14.8KB (~95%) and that --max-len 40 vs 200 changes output size.
- grep_faithful_format_test: the inline cmp closure duplicated the existing
assert_eq_grep helper (rtk_grep + grep_plain, same stdout/exit asserts);
drop it in.
`-l` was bound to RTK's `--max-len`, colliding with GNU grep where `-l`
means `--files-with-matches`. Running `grep -l <pattern>` made clap read
the pattern as a usize and error (`invalid value '<pattern>' for
'--max-len'`), dropping the command to a raw-grep fallback at 0% savings.
Drop the short binding. `-l`/`-L` now flow to `extra_args`, where the
existing `has_format_flag` already routes them to the grep passthrough
(GNU semantics, minimal output). `--max-len` keeps its long form and its
default of 80; nothing internal used the short.
- src/main.rs: remove `short = 'l'` from `Grep::max_len` + clap-parse
regression test.
- tests/grep_faithful_format_test.rs: end-to-end test asserting
`rtk grep -l/-L` is byte-identical to `grep -l/-L`, covering `-l`
leading and trailing.
- scripts/benchmark.sh: the max-len bench used the old `-l 40` alias ->
`--max-len 40`.
- docs/usage/FEATURES.md: drop the `-l` short from the grep options table.
The unit tests exercise `is_native_test_expression` directly, so inverting or
dropping the branch that calls it left the suite green while `rtk test -d dir`
went back to `sh: 0: Illegal option -d`.
Run the binary instead: native expressions carry the system `test` exit code,
`!` is neither answered backwards nor allowed to swallow a command, argument
boundaries survive, and a command still reaches the test runner.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#3788 rewrote the piped-stream half of diff_cmd.rs (condense_stdin,
condense_unified_diff_strict) while this branch rewrote the file-versus-file
half (Hunk, compute_diff, the renderers). The two conflicts are positional:
both sides add code at the same offset. Resolved as a union -- develop's
imports plus this branch's, and both blocks kept whole.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
sqlfluff exits 1 whenever it finds violations, which is the normal case this
filter exists for. Two changes in the last push read that exit code as failure
and so disabled the filter exactly when it had something to compress:
- `sqlfluff_cmd` used `early_exit_on_failure()`, which dumps the injected JSON
raw. On a 51-file corpus that is 9.8 KB where plain `sqlfluff lint` is 1.1 KB.
- `lint_cmd` substituted `result.stderr`, which is always empty: sqlfluff writes
even fatal errors to stdout (verified against 2.3.5 and 4.3.0 - `Error:
Unknown dialect 'NOPE'` on stdout, stderr empty, exit 2). Every violation was
discarded in favour of a 25-byte `SQLFluff: failed (exit 1)`.
Both are one bug reached two ways, because both entry points restated the same
routing and format-flag logic and were then edited differently. Extract it into
`sqlfluff_cmd::plan`, which states once how rtk invokes sqlfluff and how it
reads the result back; `lint_cmd` now calls it instead of keeping a copy. The
exit code only disambiguates output the filter could not parse, so a fatal error
reaches the user verbatim while violations are still summarized.
Also in the report:
- Rank the `Violations:` section worst-file-first like every section above it.
Iterating sqlfluff's emission order and taking the first 50 dropped exactly
the file the summary ranked first.
- Build only the lines that get printed, instead of every line to keep fifty.
- Omit positions sqlfluff did not report rather than fabricating `:0:0`.
- Keep two path segments when no dbt root matches, so `migrations/orders.sql`
and `reports/orders.sql` stay distinct in the section meant to be opened.
- Treat a missing `fixes` key as unknown rather than zero. sqlfluff 2.x omits it
entirely, which silently suppressed the `sqlfluff fix` hint.
- Rename the savings test to the bound it actually asserts.
Tests cover both regressions against real captured output: 4.3.0 for the current
field names and fix data, 2.3.5 for the legacy names and the absent `fixes` key.
Four behaviours that fuzzing found and that unit tests over the predicates
could not have caught, because each is about which git child RTK believes
rather than about how one flag classifies: the error `git diff -Uabc
nonexistent-ref` reports, the body surviving `--color` in all three
spellings plus the `color.ui` config form, the log limit announcing itself
only when it took something, and `--quiet` losing to a patch request from
either side while `-s` folds in order.
They run against the real binary in a temp repo with LC_ALL=C pinned, in
CI rather than behind #[ignore], because every one of them was a regression
that the suite as it stood reported as green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A fuzz batch against raw git and develop caught five places where the
tokenizer is now right and the code reading it still assumed the old,
looser parse.
`diff` and `show` reported success for commands git refuses. Both run git
twice, and the stat probe runs with the patch-shape flags stripped, so it
succeeds on input the real command rejects -- `git diff -Uabc` exits 129,
`git show -pq` exits 128, and RTK compacted the empty result into a tidy
diffstat and exit 0. The child that runs the user's arguments verbatim is
git's actual verdict on the command, so its failure now propagates with its
stderr. This is also the `run_diff` half of #3843.
`--line-prefix` joins the shapes RTK cannot represent. It prefixes every
output line, the `diff --git` and `@@` markers included, so the compaction
matched nothing and printed a stat header over an empty `Changes:` section.
The attached spelling did this on develop too; the separate spelling only
started once the value was correctly linked to its flag rather than read as
a flag of its own.
grep's `-T`/`--initial-tab` joins them for the same reason: it pads and tabs
every match line, so RTK's forced `-H --null -n` parse read nothing back and
leaked the injected flags, a raw NUL included, into the output. The long
spelling did this on develop; the short one was reachable only once `-T`
stopped being treated as ripgrep's value-taking `--type-not` for grep too.
`checkout -B` claimed the branch name from the arguments *before* reading
what git said, so a created branch lost its `(new)` marker. `-B` creates or
resets and only git knows which, so the arguments are the fallback for when
the English scan misses -- never a short-circuit past it. Reading the glued
`-Bfoo` correctly is what made this reachable; the string scan it replaced
could not see the flag at all.
golangci-lint's value-taking short flags are solo-only. Cobra rejects a
value-taking shorthand inside a cluster ("unknown shorthand flag: 'c' in
-c", 2.13.1), so reading `-Egosec`'s trailing `c` as `--config` swallowed
`run`, and losing the subcommand lost every filter that depends on finding
it. `ValueSpec::solo_only` already expressed exactly this for `git log -n`.
`--results-directory` past the `--` is read per runner mode. In MTP bridge
mode the runner reads its own copy from there and that is where the TRX
lands; in Classic the flag belongs to the test app and dotnet writes to
./TestResults regardless. Reading it as dotnet's own in Classic suppressed
RTK's injection, so the TRX RTK itself asked for by injecting `--logger trx`
was left behind in the user's project, unclaimed and never cleaned up --
measured against a real SDK 9. Both lookups share one scope rule, since
scoping only the presence check would splice the app's path into dotnet's
own arguments; the runner mode is now resolved once instead of per consumer,
which also drops a duplicate filesystem walk.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`-W`/`--function-context` does not make `git log` emit a patch -- checked
against git 2.53, where it is byte-identical to plain log -- so routing it raw
turned `rtk git log -W` into a 1 MB unfiltered dump of the whole history. The
flags that *do* imply a patch there are `-U<n>` and the combined-diff forms
`--cc`/`-c`/`--remerge-diff`, whose patch the log compaction was discarding
with no tee to recover it from.
That raw route is `run_passthrough`, which streams straight to the terminal, so
RTK's default limit has to be in the args or it never applies at all: every one
of those spellings printed the entire history, 411k lines against 50 for plain
`rtk git log`. It now carries `-10` whenever the user named no limit of their
own -- the limit only, since `--no-merges` would gut `--cc`/`-c`, whose whole
purpose is the merge diff.
`--exit-code` and `--quiet` are `diff`'s, not `log`'s. Both leave `git log`'s
output byte-identical to plain log, so there is no shape for it to escape to,
and claiming them cost the whole history for nothing. In `diff` they do earn
the raw route: the body is replaced by a report and git exits non-zero, which
RTK's stat probe inherited, its early return on failure then throwing the
entire diff away. They join `--check`, whose handling already existed for the
same reason.
`--quiet` then splits `diff` from `show`, so each states its own raw-output
grammar rather than sharing one. In `show` it is a synonym of `-s` -- exit 0,
body suppressed -- so it belongs on the compact path with `-s`/`--no-patch`,
where the summary is exactly what it asks for; claiming it raw-passed the very
header its synonyms compact.
That split's short-flag rule holds for `-p`/`-u`/`-U`, which only restate
`diff`/`show`'s patch default, but not for `-c`: it is `--cc`'s combined-diff
form, and `compact_diff` reads a combined diff's two marker columns as one, so
`git show -c` on a merge came back as `+54 -8` against git's own 156
insertions and 0 deletions. It takes the raw route its long form already took.
`--diff-merges=<format>` names those same shapes a second way, so it routes on
the format rather than on the flag: every value but `off`/`none` emits a patch,
which `log` cannot compact, while only `c`/`cc`/`combined`/`dense-combined`
produce the two marker columns, which is all `show` needs the raw route for.
Both value spellings count -- git takes the format attached or as the next
token, and `git log --diff-merges c` is accepted.
`--unified=5` and `-U5` are one option and now take one path.
`run_show`'s format gate keeps `--oneline` out, unlike `run_log`'s: there the
gate routes the whole command to a raw passthrough, and `--oneline` only means
the user's one-line summary outranks RTK's own one-line summary. Claiming it
gave up compaction entirely to fix that -- 116 KB against 31 KB on a real
commit, on the one metric this tool exists for.
`git worktree`'s write actions compact to "ok" again unless the user asked for
a report with `--dry-run`/`-n` or `--verbose`/`-v`. Keeping every non-empty
report was meant for `prune --dry-run`, whose list is the point of the command,
but `add` also speaks on success: its two progress lines came back instead of
"ok", and in the wrong order, since one is on stdout and the other on stderr.
search: the `=`-unwrapping applies to the attached spelling only. Applied to a
separate-token value it ate the first character of the user's pattern, so
`rtk rg -e '=='` searched for `=` and matched lines the real command does not.
And the filename negation is engine-specific in both directions -- grep's `-h`
is `--no-filename` where rg's is `--help`, and rg's `-I` is `--no-filename`
where grep's is `--binary-files` -- so rg's spelling was forwarded, beat RTK's
`--with-filename`, and ran the whole search a second time. `-N` and
`--no-line-number` are ripgrep's alone for the same reason: grep 3.12 exits 2
on both, so recognising them there reported a match for a command the engine
refuses to run.
Recursion is not a filename request. `-r`/`-R`/`--recursive` shared the
last-spelling-wins slot with `-H`/`-h`, which made the result depend on typing
order -- `grep -rh` dropped the prefix and `grep -hr` kept it, where real grep
drops it either way, because `-r` only makes the search span several files.
It now feeds the fallback that the paths already feed, and only `-H`/`-h`
override.
dotnet: the `-l` alias is single-dash only. `--l` is not a spelling dotnet
accepts, so treating it as the logger suppressed RTK's injection for a command
that then wrote no TRX at all.
Two tests were asserting less than they appeared to: the stdin fixture lived
inside the directory being searched, so an engine that ignored stdin and walked
the cwd still produced the asserted line, and the blob-show test built its
tokens with log's grammar where `run_show` uses diff's.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The insert point added last round tried to place RTK's `--no-patch --stat`
where it would win git's last-one-wins rule. It could not: git accepts options
after a revision, so `git diff HEAD~1 -p` still outranked it, and splicing at
the first free positional broke two other things. `rtk git diff -pl 100` died
with `fatal: ambiguous argument '100'` because every short flag in a cluster
shares one source_index, so dropping the arg took `-l` with it; and
`rtk git diff --max-age N` died with `fatal: '--no-patch': not a number of
seconds since epoch`, because a value-taking flag missing from the table left
its value looking like a positional and RTK spliced its own flags in between.
So the header goes back in front, and the user's patch-shape flags are removed
from it instead -- rebuilt per token, so `-pl 100` keeps its `-l`. With nothing
left to outrank, position stops mattering and a gap in the table is harmless
again. `--max-age`/`--min-age` are added regardless, and the insert point is
gone.
`git show -s` printed the diffstat it exists to suppress: run_show never
consulted suppresses_diff_body, and moving RTK's `--stat` in front let it win.
It now prints the summary and stops, as git does.
golangci: only a sink pointing at **stdout** collides with RTK's own, so
`--output.text.path /tmp/report.txt` gets RTK's JSON injected alongside it
again rather than leaving nothing to parse. The nine sink names are enumerated
rather than prefix-matched, per this PR's own rule 6.
search: `-h`/`--no-filename` is honoured when printing instead of being
forwarded -- as the later flag it beat RTK's forced `-H`, every line failed the
NUL parse, and the entire search ran a second time (verified: one engine exec
now, two before). A value-taking flag left without its value hands the command
over untouched, so `grep --regexp` keeps its own "requires an argument" error
instead of succeeding silently. And rg's `-A=1` is unwrapped where rg would
unwrap it; GNU grep rejects that spelling, so its args are left alone.
Refuted against a real .NET 9 SDK rather than changed: `dotnet test`'s `-r` is
`--runtime`, not `--results-directory`, and `--`'s payload goes "to the
application that is being run", so a forwarded `--logger trx` is not dotnet's
logger and RTK still has to inject its own.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`git diff <pathspec>` died outright: the stat header spliced `--no-patch
--stat` at the `--` boundary, which is args.len() when there is none, so git
saw an option after a non-option argument and refused the whole command. The
insert point now stops at the first positional too -- a git rule, not a shared
one, so it lives in git.rs rather than being forced on dotnet, whose own test
caught me doing exactly that.
That same header overrode flags whose entire job is to suppress the diff body:
`git diff --check` reported a diffstat and exit 0 where git reports whitespace
errors and exit 2. `--check`/`-s`/`--no-patch` now take the passthrough path,
which in turn had to stop discarding stdout on a non-zero exit -- `--check`
prints its report and *then* exits 2.
Three more from the same batch: stripping `--no-compact` by a single index
leaked the second copy of a repeated flag to git; ignoring `--` only when it
was the sole token dropped `git status -sb --` off the compact path; and
`git worktree -- add x` answered with a worktree list where real git errors.
`paths.is_empty()` meant "the engine walked the cwd" for rg but "read stdin"
for grep, so grep prefixed every match with `(standard input):` whenever stdin
was a terminal. The new stdin tests also guard on the engine they drive, the
convention the other search integration tests document.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A review round found the same defect wearing four different shapes, so the
fixes are structural rather than per-site.
The tool's own args end at `--`. Msbuild keeps classifying past the boundary
because it forwards arguments, so every dotnet lookup read what the user meant
for the test runner: `dotnet test -- --logger trx` suppressed RTK's own logger
injection and `dotnet build -- -bl` its binlog. `before_dashdash` now scopes
the five dotnet lookup primitives at once, and `--report-trx` -- legitimate in
either region -- says so where it reads both.
Injected flags belong before that boundary too. `dotnet format -- ./src` put
`--verify-no-changes` after it, where dotnet parks it unread: format rewrote
the tree while RTK reported check mode. `git diff --stat` had the same shape.
Both go through `injection_point` now.
Detection and the action on it have to share one rule. `--no-compact` was
detected as a token and stripped as a string, deleting a pathspec of that name;
`--write` was detected bare-and-unforwarded but stripped everywhere. They now
strip the tokens detection actually saw.
`stash` broke restore_double_dash's precondition: clap carves its subcommand
out of the same trailing region, so restoring from the remainder sliced one
token short -- `git stash -- -p` ran an interactive `push -p` where real git
errors on the pathspec. The region is reassembled before restoring.
Also from the round, each verified against the real tool: `-f`/`--file`
supplies patterns like `-e`, so `grep -f pats.txt a.txt` no longer answers "no
matches" (or hangs) by stealing the path as the pattern; grep's `-NUM` is
`--context=NUM`, so its blocks keep their `--` separators; a single matching
file found by walking the cwd keeps its name, as real rg prints it; `git diff
-p` stopped printing the patch raw and then again compacted, 2.4x the raw
output; `/Other.csproj` is a path, not an MSBuild switch, so it no longer
adopts an unrelated project's runner mode; and `/l:trx` is MSBuild's logger
assembly, not dotnet's `--logger`, which the tokenizer now records as
`Token::slash`.
git's remaining string-scanning handlers move to the tokenizer with the rest:
`git status --` keeps its compact path, a blob spec is a free positional rather
than any arg containing a colon, and a worktree action is the subcommand rather
than any arg spelling one.
The two stdin regressions move to tests/ where CI actually runs them -- they
were `#[ignore]`d, and CI never passes `--ignored` -- and the show_line/context
tests now drive the production detector instead of test-only twins of it, which
let a deleted production arm stay green. The twins are gone.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The test itself is #[cfg(unix)] (it spawns a /bin/sh stub using unix file
permissions), but its Command import and shell_quote helper weren't gated
the same way -- on Windows the test compiled out entirely, leaving both
genuinely unused and tripping -D unused-imports / -D dead-code.
Nest both inside the test function's body instead of gating them separately:
since the whole function is already #[cfg(unix)], that one attribute now
covers everything, and Linux/macOS behavior is unaffected (cargo test --all
still passes the same 2765 tests).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
run_diff's wants_stat and run_show's wants_stat_only only recognized
--stat/--numstat/--shortstat, narrower than requests_raw_diff_shape
(already used by run_log in this same branch), which also covers
--dirstat/--name-only/--name-status/--raw/--summary/--patch(-with-raw/
-with-stat)/-p/-u. `rtk git diff --name-only` (or --raw, -p, etc.) fell
through to RTK's default stat+compacted-diff path instead of a raw
passthrough of git's own output for that flag.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
run_diff was the one migrated handler still using raw arg == "--stat"
string scans for wants_stat/wants_compact, missed when run_show and
run_log got the same fix earlier -- a file literally named "--stat"
after -- was misdetected as the real --stat flag and routed to the
raw-passthrough branch instead of RTK's stat+compacted-diff path.
Migrated onto arg_tokenizer, sharing log_takes_value like run_show does.
Also documents (arg_tokenizer.rs) a known, deliberately-unfixed
limitation surfaced by review: a single-segment Msbuild-dialect '/'
path (e.g. "/app", "/tmp") is structurally indistinguishable from a
switch name -- confirmed via Docker that real dotnet/MSBuild itself
only resolves this with a filesystem stat() call, which this tokenizer
can't replicate as a pure function.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
run_branch never called restore_double_dash despite git branch using
trailing_var_arg, and has_action_flag/has_list_flag/has_positional_arg
were raw, --boundary-unaware string scans (has_positional_arg in
particular: `!a.starts_with('-')`). A branch name starting with '-'
after -- was misclassified as a flag rather than the positional name to
create, so rtk silently ran a harmless empty `git branch -a` list
instead of attempting the creation.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Mirrors the existing git_log_dash_p_pathspec_after_double_dash test:
stages a file literally named "--stat" past the -- separator and asserts
`rtk git show -- --stat` stays on RTK's compacted-diff path instead of
being misdetected as the --stat summary flag.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
tokenize/tokenize_dialect were hardcoded to &[String], forcing search.rs's
extract_pattern_path to clone every arg into a fresh Vec<String> just to
satisfy that signature even for its one production caller, which already
owns a Vec<String> from restore_double_dash (worked around last commit
by splitting into a test-only generic wrapper + an _owned zero-copy
variant). Genericizing tokenize/tokenize_dialect/push_atomic_flag over
T: AsRef<str> removes the need for that split entirely: String and &str
both satisfy the bound, so every existing call site (all of which pass
&[String]) needed zero changes, and search.rs collapses back to one
extract_pattern_path<T: AsRef<str>> used directly by both its production
call site and its ~29 &[&str]-literal tests, with no cloning either way.
Not extended to OsStr/OsString: unlike str, OsStr exposes almost no
string-manipulation API by design (no strip_prefix, split_once,
char-boundary slicing), so tokenizing it would mean re-deriving that
machinery byte-by-byte the way clap_lex does internally -- a much bigger
change for a case rtk doesn't hit today (its own CLI parsing already
assumes UTF-8 args for every subcommand this module serves).
Also documents why tokenize() doesn't just call restore_double_dash (or
std::env::args()) internally: restore_double_dash's result has to be an
owned Vec<String> the caller holds in its own `let`, and Token<'a>
borrows straight from `args` -- tokenizing a Vec<String> built inside
this module would tie every Token to a value dropped when the function
returns. Same root cause as why this module doesn't build on clap_lex.
Also: dotnet_cmd.rs's has_trx_logger_arg no longer needs an intermediate
`let has_trx = ...; has_trx` binding to dodge a tail-position temporary
lifetime issue -- an explicit `return` avoids it more simply. And added
a regression test + doc trail for the earlier -5x malformed-input
question: verified against real git (`git log -5x` fails with "fatal:
'5x': not an integer", exit 128) that run_log's internal limit
computation for this input is never observable either way, since
run_log bails out on !result.success() before the formatting code that
would use it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Nicolas Le Cam