Address KuSh review on #2465:
- Move the `exit_code >= 2` decision into a pure `is_grep_error_exit`
function and unit-test it directly (0/1 = normal, >=2 = error),
instead of faking the rg binary in an integration test.
- Drop the GREP_ERROR_EXIT const; the doc comment on the function
conveys the grep/rg exit convention.
- Remove tests/grep_error_test.rs (faked binary) in favour of the
unit test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wrap the rtk binary path expression per rustfmt to satisfy
`cargo fmt --all -- --check` on CI.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The test simulates a broken `rg` via a chmod'd shell script (from_mode,
PermissionsExt) which is unix-only. The unconditional `use std::os::unix`
broke compilation of the integration-test crate on the Windows CI target.
Gate the whole file so Windows stays green while preserving unix coverage.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Exit codes >= 2 signal real errors in grep/rg (bad regex, tool
crash, missing binary). rtk was only checking the narrow case
exit_code == 2 with non-empty stderr, letting any other error
exit masquerade as a clean "0 matches" false negative.
Now any exit code >= 2 surfaces the error to stderr instead of
printing a false zero-match result.
Fixes#2461
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The system-grep fallback (used when ripgrep is not installed) passed
-rnHZ, relying on -Z for the NUL filename separator the match parser
requires. -Z only means --null on GNU grep; on BSD/macOS grep it is an
alias for --decompress (zgrep mode), so output is plain
file:line:content with no NUL. parse_match_line() then matches zero
filenames and every result collapses into "N matches in 0 files" with
all lines hidden behind [+N more].
Use the long option --null instead, which both GNU and BSD grep define
as "print a zero-byte after the file name".
Related to #2310
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The left-to-right scanning logic now lives entirely in parse_cluster(), which
returns a ClusterResult enum (Boolean or ValueTaking). extract_pattern_path
just dispatches on the result — no loop logic inline.
parse_cluster() is the only place that touches cluster bytes, so the full
contract is testable in isolation:
parse_cluster("ecarrot") → ValueTaking { prefix: None, flag: 'e', inline: "carrot" }
parse_cluster("rne") → ValueTaking { prefix: Some("n"), flag: 'e', inline: "" }
parse_cluster("g*.rs") → ValueTaking { prefix: None, flag: 'g', inline: "*.rs" }
parse_cluster("rn") → Boolean(Some("n"))
strip_r() remains as the focused r/R-stripping helper used internally by
parse_cluster (and tested directly to document the "carrot"→"caot" danger).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The left-to-right cluster scan now calls strip_r() at emit time on the
accumulated flag-letter prefix (raw_prefix), never on inline value bytes.
This makes the safety contract explicit and testable: strip_r("carrot")
produces "caot", documenting exactly why it must not touch value bytes.
Also fix pre-existing unused-mut in aws_cmd.rs (has_output_flag).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Five regressions on grep_arg_parsing vs develop:
- `-ecarrot` matched "caot" — strip_r was applied to the inline value bytes.
Fix: scan cluster left-to-right; stop at first value-taking letter and
treat the remainder as the inline value without r/R stripping.
- `-g*.rs` glob became `*.s` — same root cause, inline glob value r-stripped.
- `--glob '*.md'` took `*.md` as a path — no VALUE_FLAGS_LONG existed; the
value token fell through to the positional collector.
Fix: add VALUE_FLAGS_LONG covering all rg long flags that take a value.
- `--max-count 1 fn file` took `1` as the pattern — same missing long flag.
- VALUE_FLAGS_SHORT expanded: add t/T (--type/--type-not), d (--max-depth),
M (--max-columns) so inline short forms and space-separated forms parse
correctly without mis-routing values to patterns or paths.
- --regexp (long form of -e) now routes to patterns like -e does.
- has_format_flag: add --count-matches, --json, --passthru, --files so
these output-format modes get passthrough treatment instead of silent
"0 matches in 0 files".
2183 tests pass.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rtk diff treated files whose changes were all classified as modified
(similar lines, e.g. "a: 1" vs "a: 2" in YAML/JSON) as identical,
because the identical check only looked at added/removed counts.
Report any non-empty change set as a difference, and exit 1 when
files differ per diff convention (0 when identical).
Fixes#2364
A combined cluster like -rne PATTERN or -rA 2 had the last flag's
value consumed only if the cluster was exactly 2 chars. Longer
clusters (e.g. -rne) fell through to process_flag, which stripped
-r and emitted -ne — then -e ended up in flags without a value,
causing rg to error.
New unified short-flag handler: for any single-dash cluster, inspect
the last byte. If it is -e or a VALUE_FLAGS_SHORT member, strip the
r/R chars from the prefix, emit the prefix as a flag (if non-empty),
and consume the next token as the value (pattern for -e, flag value
otherwise). Long flags (--foo) are still routed through process_flag.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Four bugs in `extract_pattern_path` caused `rtk grep` to misparse
common invocations:
- `-A 2 error src` → `2` was taken as the pattern (value-taking
flags not recognised)
- `-rn foo src` → `n` leaked to rg as `--replace` value (only
standalone `-r` was stripped, not combined clusters like `-rn`)
- `TODO src tests` → second path silently dropped (only one path
kept)
- `-i x agent -g '*.md'` → rg regex error (`*.md` became positional
before `-g` could consume it)
New implementation:
- `VALUE_FLAGS_SHORT`: explicit byte-set of 2-char flags that consume
one following token (`A`, `B`, `C`, `g`, `f`, `j`, `m`)
- `process_flag()`: strips `r`/`R` from any single-dash cluster,
drops `--recursive`; replaces the old inline `if arg == "-r"` loop
- `extract_pattern_path` returns `(Vec<String>, Vec<String>, Vec<String>)`
(patterns, paths, flags) — no more `Option<String>` for pattern
- `-e` values collected separately; if any `-e` is present, all
positionals are paths; otherwise first positional is the pattern
- All patterns forwarded to rg as `-e` flags (BRE `\|`→`|` inline)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- restore migration Outdated check in hook_check (binary registered +
legacy script present still reports Outdated until cleanup)
- add error context to resolve_claude_dir call sites (provider, integrity)
- use CLAUDE_DIR const instead of literal in telemetry + provider test
- simplify settings_path construction in print_manual_instructions
Refs #2087
rtk init -g ignored CLAUDE_CONFIG_DIR and always targeted ~/.claude.
Honor the env var Claude Code uses to relocate its config directory.
Priority: RTK_CLAUDE_DIR > CLAUDE_CONFIG_DIR > $HOME/.claude.
Empty values are treated as unset. Manual-step instructions now
print the resolved path instead of a hardcoded ~/.claude.
All callsites that previously hardcoded home.join(CLAUDE_DIR) now use
the shared resolve_claude_dir(): hook_check (rtk gain), integrity
(rtk verify), telemetry, and discover (rtk discover).
Closes#633
Match the join_with_overflow convention (ellipsis char, '+N more' label)
used by container, aws, gh and glab so the LLM sees one consistent
overflow style. The tee '[full output: …]' recovery hint already comes
from core tee_and_hint via RunOptions::with_tee, so capped failure output
carries the canonical recovery path unchanged.
Replace the per-filter LimitsConfig::mvn_max_failures field with a local
const MAX_MVN_FAILING_CLASSES = CAP_WARNINGS, matching the canonical cap
binding used by pytest/rspec/rake/runner. Align cap = 0 semantics with the
core policy (summary-only: no blocks emitted, tail still counts) instead
of the old opt-out meaning, so the future config surface lands without a
special case.
filter_quiet already dropped the help block Maven emits after
'Failed to execute goal' (See ..., -> [Help 1], Re-run Maven, help
URLs, bare [ERROR] dividers); the non-quiet paths kept it via the
[ERROR] catch-alls — keep_outside_block for test/package and
filter_compile's own [ERROR] branch.
Promote QUIET_BOILER_PREFIXES to a shared BOILER_PREFIXES (+
is_boilerplate(), which also covers the bare [ERROR] divider lines,
matching filter_quiet's existing drop) and reject boilerplate before
each [ERROR] catch-all. The list deliberately keeps the resume hint
([ERROR] After correcting the problems... / [ERROR] mvn <args>
-rf :module) — actionable signal for resuming a multi-module build —
and the Failed to execute goal terminator.
keep_continuation hygiene (defensive parity, not a demonstrated bug):
filter_surefire only reset the flag inside its kept branch, so a
dropped [ERROR] line would leave it stale and a following indented
line could be wrongly kept. No committed fixture exhibits the leak;
add the same end-of-loop fall-through reset filter_package already
has. filter_compile resets on its new boilerplate drop path.
Multifail-slice savings rise from 19.9% to 42.3%; threshold pinned
at >=30% to match the reactor-fail precedent. README: document the
stripping and drop the stale duration-normalisation bullet (that
behaviour was removed in 77e28d0).
test (windows-latest) failed on the two CRLF tests: with no
.gitattributes, the Windows runner's core.autocrlf=true checks fixtures
out with CRLF, include_str! embeds \r\n, and the tests'
replace('\n', "\r\n") synthesis produces \r\r\n. str::lines() strips
only the final \r\n of each pair, leaving a stray \r that $-anchored
regexes (MODULE_BANNER, BUILD_FOOT) reject — dropping the module banner
and BUILD SUCCESS in the CRLF leg only. Real Maven CRLF output (single
\r\n) was always handled; only the synthesized double-CR leg broke.
Root-cause fix: tests/fixtures/** -text — fixtures are byte-exact
filter inputs; eol conversion must never touch them. Defense-in-depth:
the two CRLF tests normalize the embedded fixture back to LF before
synthesizing, so they stay correct even on checkouts without
attributes (zip downloads, pre-existing autocrlf clones).
Also fixes the test doc comment, which claimed str::lines() keeps the
trailing \r of a \r\n pair — it strips it; the hazard is the doubled
\r\r\n, not single CRLF.
Surefire 3.x emits one blank-separated detail block per failing test
under a single class close line. The trail previously ended at the
first blank line and never re-armed, so failures after the first lost
their exception message (unindented -> dropped) and leaked the full
junit/jdk framework stack (indented -> kept by keep_continuation).
SurefireBlock now remembers the trail's keep/drop decision when it
ends at a blank line (trail_rearm) and re-enters the trail on the next
per-test subline with the same decision — a capped class drops all its
per-test blocks, not just the first. Any other non-blank line disarms
re-entry; RUNNING/commit/drop clear it so an unrelated class can never
inherit the decision. Extra blanks between per-test blocks stay armed.
Per-test sublines use '<<< ERROR!' for thrown (non-assertion)
exceptions — accept both markers via a shared is_per_test_subline()
(also reused by filter_quiet, which already re-armed correctly), and
widen the CLOSE regex to tolerate an ERROR! marker defensively
(Surefire 3.5.5 emits FAILURE! even for errors-only classes, per the
fixture capture; detection keys off the counts, not the marker).
Fixture captured from real Maven 3.9.9 / Surefire 3.5.5 output against
the new tests/fixtures/multifail-skeleton/ (CalcTest: assertion failure
+ thrown exception in one class; BoomTest: errors-only class). A
byte-exact pin locks the single-failure path unchanged.
Separator lines (═══, ---) and an emoji status marker cost tokens
without adding signal for the LLM — RTK output must never add noise
over raw. Semantic labels are kept; the emoji is swapped for plain
monochrome unicode.
aesoft