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 usage guard returns early when no hash and no --list is given, and
--list returns before this match, so the arm could never run. Removing it
keeps the usage string in one place.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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>
`!` and `(` are shell syntax as well as `test` syntax, so matching them on
the first token alone captured invocations the shell path ran correctly:
`rtk test ! false` returned 1 instead of 0, and `rtk test ! cargo --version`
stopped running cargo and reported `test: missing argument after '--version'`.
Recurse instead, so `!` and `(` mark a native expression only when what they
apply to is one. Measured against `/usr/bin/test` over 526 generated
expressions: unchanged at 212 mismatches (down from 411 before the fix), with
the shell-command forms back to their previous exit codes.
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>
known_ruff_subcommands_do_not_route_through_check iterates RUFF_SUBCOMMANDS,
so dropping an entry from the constant leaves it green. Pin the list literally
so a missing subcommand fails the suite.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The section landed in docs/TROUBLESHOOTING.md, a flat file removed in
a94e9493 when the docs were consolidated. Content there does not reach
the published guide, and the copy reintroduced install guidance that
develop had already pinned to --branch master.
Move the answer into docs/guide/resources/troubleshooting.md and drop the
duplicated Type Kit collision section, which that guide already covers.
The cache write/read breakdown is reported by `rtk cc-economics`, not by
`rtk gain`, so point readers at the command that actually shows it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`~ N→M` carries both files' numbering, but the legend still called `~` a file1
marker, and gating the note on `diff.added` dropped it from a deletions-plus-
crossing render entirely: two frames on screen, nothing saying so. The legend
now keys on the shapes actually listed. A crossed `~` earns its own clause,
`~ N→M spans both files`, owed whether or not a `+` is present, which also
tells the two `~` shapes apart when one listing carries both and nothing in
the markers does.
The `EditScript` refusal said "only in a.txt: 1 line, only in b.txt: 1 line"
for two 1.1MB minified files differing by one character. Those counts are
pre-pairing, so the two lines would have rendered as a single `~`; "only in"
is a claim about content and it was false in both halves. It now reads "1 line
changed in a.txt, 1 line changed in b.txt", which states what is known without
pairing them.
Tests: the crossed render asserts through `render_diff` rather than
`format_diff_changes`, which is where the legend is composed and why the
inconsistency shipped green. `render_file_diff`'s docstring states that rule
for render-shape assertions so the next one lands at the right level. New
cases cover the legend for a crossing beside deletions, beside insertions, and
beside a plain `~`.
The consumer-flag cases were a hand-maintained list that grew one literal per
review round, so each new spelling was found by review rather than by a test.
Generate the spellings from SAFE_PIPE_CONSUMERS instead: every unambiguous
long-option prefix getopt_long accepts, each in bare, double-quoted,
single-quoted, backslash-escaped and =value form, and every unsafe short flag
bare, quoted, escaped and bundled. A consumer added to the table later is
covered on arrival.
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.
dependabot[bot]