The style-edition reformat touched 69 files without changing behaviour,
which buries the real authorship of those lines. GitHub's web blame applies
.git-blame-ignore-revs automatically; locally it needs blame.ignoreRevsFile
set, so CONTRIBUTING now says so where contributors first clone.
Only the reformat is listed. The let-chain commit also touches many files
but changes control flow and drops a crate-wide lint allow, so its
authorship is real and should stay visible in blame.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
show_claude_config read the Cursor hook file only to compute _is_thin, which
nothing reads. The `?` on that read meant an unreadable hook file aborted
`rtk init --status` before it printed the rest of the report -- a status
command failing on the thing it is meant to report about. The executable-bit
check above it is what the branch actually uses.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Let-chains are edition 2024 syntax, so clippy::collapsible_if now fires on
every nested `if let` / `if` pair, and with warnings = "deny" they are build
errors rather than suggestions. Applied with cargo clippy --fix, which
handles all of them in one pass, then cargo fmt to re-indent the collapsed
bodies.
The deepest cases flatten four levels of nesting into one condition:
if let Some(cutoff_time) = cutoff
&& let Ok(meta) = fs::metadata(file_path)
&& let Ok(mtime) = meta.modified()
&& mtime < cutoff_time
{
continue;
}
Drops the crate-wide clippy::collapsible_if allow added in the previous
commit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Bump edition 2021 -> 2024. rust-version stays at 1.91, already well above
the 1.85 floor the edition needs; docs/guide/resources/troubleshooting.md
still claimed 1.70+, which is where a failed `cargo install --git` lands.
Four things the edition forces:
- std::env::{set_var,remove_var} are unsafe in 2024 with no safe std
replacement. Rather than wrap the test call sites in unsafe -- which the
crate denies and .semgrep.yml flags -- route them through temp-env, a
dev-only dependency whose closure API is safe and which restores the
previous value even when the body panics. The hand-rolled CLAUDE_DIR_LOCK
and PI_DIR_LOCK guards existed only to serialise those mutations and are
now redundant; CWD_LOCK and TEST_ENV_LOCK stay, they order more than the
env var itself.
- unsafe_op_in_unsafe_fn is on by default, so the libc calls in the proxy
signal handler and in stream.rs's relay handler need explicit unsafe
blocks, scoped to the libc calls themselves.
- `gen` is a reserved keyword, so the closure by that name in diff_cmd.rs
becomes make_lines.
- Tightened tail-expression temporary scopes let clippy prove the binding in
setup_test_env is inlinable, so let_and_return now fires there.
if_let_rescope changes when the scrutinee temporary drops in an if let/else.
The two sites in show_claude_config take cargo fix --edition's match rewrite,
which keeps the 2021 drop timing.
rustfmt.toml is kept rather than dropped: cargo fmt passes --edition from
Cargo.toml, but a bare rustfmt invocation has no crate context and falls
back to edition 2015, which cannot parse the let-chains the next commit
introduces. Pinning it there keeps format-on-save and pre-commit hooks in
agreement with CI.
clippy::collapsible_if is allowed crate-wide for now; the follow-up commit
adopts let-chains and removes the allow.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every ecosystem mod.rs under src/cmds/ generated its module list with
automod::dir!(). rustfmt only follows literal `mod` items and cannot expand
macros, so `cargo fmt` never reached any of those files -- and neither did
CI's `cargo fmt --all -- --check` gate, which has been silently passing
over half the crate.
Demonstrated by appending `fn __probe( )->u8{let x=1;x}` to a file
under src/cmds/: with automod, `cargo fmt --all -- --check` reports no
diff; with an explicit `pub mod`, it reports the diff and `cargo fmt --all`
fixes it. The preceding commit had to invoke rustfmt directly for the same
reason; from here `cargo fmt` covers the crate on its own.
Listing modules by hand trades one failure mode for another, so build.rs
now guards the new one: automod compiled any stray .rs file, whereas an
explicit list silently drops a file whose `pub mod` line is forgotten --
never compiled, never linted, tests never run, and check-test-presence.sh
still reporting PASS because it only greps the file for #[cfg(test)]. The
build now fails with the missing declaration named.
Declaring the modules explicitly also exposed clippy::module_inception on
cmds::git::git, which clippy skips inside macro-generated code. git.rs is
renamed to git_cmd.rs, which resolves the lint and matches the convention
its siblings already follow -- diff_cmd, gh_cmd, glab_cmd, gt_cmd. Docs
that point at the module are updated; sample command output that merely
shows a "git.rs" path is left alone.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pin style_edition = "2024" in rustfmt.toml and reformat, ahead of the
edition 2024 bump. Setting style_edition explicitly decouples the reformat
from the crate edition, so this lands on edition 2021 and CI's
`cargo fmt --all -- --check` still passes.
Run through rustfmt directly rather than `cargo fmt`. Every module under
src/cmds/ is currently declared by automod::dir!, and rustfmt only follows
literal `mod` items, so `cargo fmt` cannot see those files -- the next
commit makes them reachable. Roughly two thirds of the diff is therefore in
src/cmds/, mostly the version-sorted imports style edition 2024 introduces.
No logic changes.
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>
The pipeline diagram, the discover walkthrough and the `rtk read` option
table still described the `--max-lines` mapping this branch replaced, and
the repo's own suggest hook still proposed `--max-lines N` for `head -N`
— the mapping this branch exists to correct.
Drop the `head -` disjunct in the rewrite guard: every string with that
prefix also has the `head ` prefix that sits next to it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`get_by_command` moved to the volume-weighted rate while this branch was
open (#891), summing every call and guarding the division at group level.
The two telemetry queries now use the same shape: the row-level
`WHERE input_tokens > 0` is gone, so a call with no input that still
printed something counts against its command exactly as it does in the
By Command table, and `HAVING SUM(input_tokens) > 0` keeps commands that
never had any input out of both, as before.
`low_savings_commands` lists net-regressing commands: `sav > 0.0` became
`sav <> 0.0`, so only exact 0% stays with `passthrough_top`. A command
that emits more than it saves is the filter to fix first, and with the
weighted rate the old bound also dropped commands the unweighted query
used to list.
The two regression tests opened the on-disk tracker, so every `cargo test`
wrote ten synthetic rows (one at 100k input tokens) into whichever history
database `Tracker::new()` resolves to, on a developer machine the real one.
They now use `Tracker::new_in_memory()` like the rest of the module, and
`test_avg_savings_per_command_inner_weighted` only asserted a `0..=100`
range that the unweighted query also satisfied. Both tests now assert the
exact weighted figures, check each listed rate against `get_summary()`,
and fail when either query goes back to `AVG`, to the row filter, or to
the positive bound.
Tests are numbered 18 and 19 after the ones develop already carries as
14-17. The `low_savings_commands` doc no longer points at a PR, and both
telemetry field tables describe the new semantics.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Move the weighted-rate regression test off the real tracking database, which
it wrote two rows into on every run, and onto an in-memory tracker. Add the
long-tailed pair that pins the weighted rate against the mean of percentages,
the same assertion through get_summary(), and a zero-input group: without the
CASE guard that last one fails on a NULL rate, and no other test covers it.
Name the 4th field of GainSummary::by_command for what it now holds, and state
the rate formula on the CommandStats warning in percentage order, with the zero
guard, so that following it verbatim cannot reintroduce the NULL.
Head the column Total% rather than Rate, keeping the shape of the Avg% it
replaces and naming the total the figure is now taken over.
Drop the hand-written CHANGELOG entry: release-please generates it from the
commit subject, and CONTRIBUTING.md asks for that file to be left alone.
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>
`gh pr checks` spells a pending bucket `pending`, never `*`, and prints five
tab-separated fields with the description last. The transition test used a
shape gh does not emit; the new case carries the trailing description column
and a pending row that resolves across polls.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two defects in `filter_ast_grep`, both verified against ast-grep 0.45.3.
`ast-grep scan` diagnostics parse only on their locator line, so filtering
line by line kept ` ┌─ a.rs:2:13` and dropped the rule id, severity,
message and source line — at exit code 0, so it read as success. The
whole-output fallback never fired because `order` was not empty.
`unparsed_signal()` now passes any shape through untouched when a single
non-blank line fails to parse, which is what `search.rs` already does for
grep/rg. This also covers `--heading` mode and Windows drive-letter paths,
where `[^:]+` cannot match `C:\src\a.rs`.
The per-file overflow hint was computed as `entries.len() - max_per_file`,
which ignores `max_total` cutting a file short: a file under its own cap
lost its remainder with no hint at all, and a file over it under-reported
the drop. Both now count against what was actually printed.
The hint said "matches" while counting lines. ast-grep prints one line per
matched source line and a structural match spans several, so a repo search
reported "19 more matches" where five matches remained.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`RtkRule::pipeline_final_safe` was replaced by the `PipelineSafety` enum
on develop (#3171). The ast-grep rule still set the old field, so the
branch merged cleanly but failed to compile (E0560).
`ProducerOnly` keeps the rule's original intent: safe as a pipeline's
first stage, never as its final stage, since run() execs with stdin null.
Adding a producer-safe rule also requires listing it in
`test_pipeline_producer_safe_rule_set`, which pins the exact set.
`savings_pct` was left at the 60.0 default; the repo-wide measurement is
84.6%, which is what README and what-rtk-covers.md already claim.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codex and Vibe both return empty rule vectors; keeping them as separate
arms multiplies branches that have to stay in sync. Group them, sorted,
with one comment covering both.
Co-Authored-By: Claude Opus 5 (1M context) <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>
`RunOptions::stdout_only()` is documented as "stdout-only to filter, stderr
passthrough", but the passthrough half was never implemented: stderr was captured
and dropped on every path except `skip_filter_on_failure`. A tool that reports on
stderr and leaves stdout empty therefore produced nothing at all.
golangci-lint is the visible case -- a config or build error goes to stderr, the
filter is handed an empty stdout, and the never-worse guard replaces its
"JSON parse failed" fallback with the empty string, so both streams end up
silent. The same shape applies to every other stdout-only filter: ruff, pytest,
rspec, rubocop, gh, glab, prettier, tree, wc.
Savings are also measured against running the command directly, so the stderr
that is forwarded has to be counted as emitted. Comparing stdout to stdout while
printing stderr as well booked a passed-through stream as if it had been
filtered away.
The guard keeps comparing against stdout. stderr is forwarded verbatim on both
sides, so it cancels: never-worse reduces to `filtered <= stdout`, and widening
the baseline to the combined output would let rtk emit more than the command it
replaces.
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>
`tokens_saved_24h` and `total_tokens_saved` both return `i64` from an unclamped
`SUM(saved_tokens)` -- deliberately, unlike the `as usize` readers that take
`.max(0)` for their unsigned API. A window whose filters emitted more than they
saved sums honestly negative, so `assert!(saved_24h >= 0)` panics on any DB
holding a net-negative row.
No ordering invariant replaces the floor: with a positive row inside the 24h
window and a larger negative one outside it, saved_24h (90) exceeds saved_total
(-110). Nothing about either value is assertable, so the test stops binding them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
dependabot[bot]