[iliaal]

A grep anchored to the start of a line finds nothing in rtk git diff output:

raw:  git diff | grep -c "^-"   -> 3
rtk:  git diff | grep -c "^-"   -> 0

compact_diff indents every hunk-body line two spaces to nest it under the filename (format!(" {}", line)), which moves the content off column 0 and breaks the ^ anchor.

Two things worth stating precisely, because both were initially diagnosed wrong on our side:

• Nothing is dropped. The content is on screen the whole time, and --stat plus the per-file +N -M tally report accurately. grep -c DELETED_LINE_A (unanchored) returns 1 both raw and filtered. It is a rendering issue, not data loss.
• It is not deletion-specific. ^+ breaks identically — measured raw ^-=3 / ^+=2 versus rtk ^-=0 / ^+=0. There is no +/- asymmetry.

The cost is a silent false negative for any anchored grep over a diff: auditing "was a test expectation weakened" with git diff tests/ | grep "^-" returns empty while --stat reports deletions in the same range, and empty is exactly the answer that ends the investigation.

Fix: hunk bodies emit at column 0 in git's own unified shape. rtk's own annotations keep the indent, so the same greps never count them as diff lines.

After:

rtk:  git diff | grep -c "^-"   -> 2

2 is the exact deletion count; raw's 3 includes the --- a/f.txt header, so the piped grep was never a correct count even unfiltered.

What the anchored audit does and does not promise

Review surfaced three holes in the original "grep -c '^-' gives you the exact deletion count" claim. All three are closed, two by fixing the code and one by stating the limit here.

Content that looks like a file header. The +++ / --- guards ran inside the hunk-body branch, where they can only match content: git emits the file headers before the first @@. So an added ++i; or a removed -- sql comment was dropped from the body and left out of the tally, and a diff doing both rendered as a bare hunk header with nothing under it. The guards are gone.

Combined diffs. diff --cc sections, which git emits for unmerged paths during a merge, rebase, cherry-pick, or stash pop, never matched the diff --git reset, so in_hunk stayed set across sections and the --- / +++ headers of every file after the first reached the body. Once hunk bodies emit at column 0, those headers anchor too: a two-file conflict with zero deletions returned ^- = 1. The reset fires on any diff -- prefix now, so the headers land before the first @@ again and the same repo returns 0.

Combined diffs put the marker in column 2. diff --cc carries one marker column per parent, so a line changed against only one of them ( +ours, -theirs) does not anchor. It is counted in the +N -M tally now, but grep "^+" cannot see it without rewriting the line, so docs/usage/FEATURES.md states the limit next to the others. Against git diff --numstat on three complete commit ranges the tally is exact.

Past 100 change lines in a hunk, the count plateaus. A 300-deletion hunk yields grep -c "^-" = 100 while the tally correctly says +300 -300. The truncation note carries the remainder and is split by sign — ... (200 deletions truncated) — but it is indented by design, so the anchored grep cannot see it. If you gate CI on grep -c "^-", gate on the +N -M tally instead, or pass --no-compact.

Two notes on the budgets. Exempting leading context from max_lines makes it a soft cap: output can exceed it by up to 10%, so the git stash show -p call site that passes 100 can see 110. Separately, and pre-existing on develop, the previous file's tally and the next file's header are pushed before the budget is checked, so a many-file diff can carry two entries past max_lines. And a hunk now ends when the line counts in its @@ header are consumed, so trailing text after a diff — an mbox envelope, a -- signature, prose — is outside every hunk rather than read as its body. gh pr diff --patch additionally passes through unfiltered: GitHub's .patch is an mbox, not a unified diff.

A hunk's declared length is per parent, and a combined hunk spends one budget per marker column. In a combined diff a space means opposite things depending on the line: on a change line the line is in that parent unchanged, on a removal line it is absent from it. Collapsing the columns into one add/delete pair lost that, so the first parent's budget never converged on real conflict output and the hunk never closed by count — everything above about keeping envelopes and signatures out of the body silently did not apply to diff --cc, which is what git emits for every unmerged path. The reverse case closed early and dropped the body: @@@ -1,2 -1,4 +1,2 @@@ over two removals from the second parent alone rendered as a bare hunk header, with no tally and no truncation note. A header whose range count disagrees with its column count is reconciled against the columns, since only a column can be charged.

Word diffs pass through unfiltered. --word-diff, --word-diff-regex and --color-words drop the marker column and put [-removed-] / {+added+} inline, so a body line is arbitrary content in the position the compactor reads as a marker. Slicing those bytes as bytes rather than as a str fixes the crash on a leading multi-byte character — rtk git diff --word-diff exited 101 with empty stdout — but not the misparse: a row starting + counts as an addition and spends the wrong budget, one starting \ is dropped as a no-newline annotation, and a real git show --word-diff=plain row reading diff --git [-a/src/main.rs... opens a new file section. There is no faithful compaction of a word diff, so run_diff, run_show and git stash show -p hand these modes straight through. rtk pipe --filter git-diff has no flags to read and still misparses word-diff text piped into it; compact_diff reads a unified or combined diff, and that is now what its callers give it.

Section paths are read without guessing at the separator. Under the default core.quotepath git escapes a non-ASCII path and wraps it in ", which removes the b/ the plain form splits on, so the fallback returned both paths as the section header. And b/ is not a reliable separator anyway: it appears inside the path for a file under a directory named x b, and --no-prefix or a custom --src-prefix leaves none at all. Anything but a rename names the same path twice, so the halves are the same length and the separating space sits dead centre; splitting there covers all of those and does not care what the prefixes are. A rename's halves differ in length and fall through to the b/ split, which names the destination.

Leading context is also emitted now. It was silently dropped before the first + or - of each hunk, which contradicted the "nothing is dropped" claim above. It keeps the last three lines before the change rather than the first, so the body is contiguous with it under -U10 or --function-context, and is capped at max_lines / 10 across the diff and exempt from max_lines so it cannot displace change lines. On a five-commit diff of this repo it costs 0.1 points of byte reduction against master (71.7% versus 71.8%) while taking anchorable change lines from 0 to 201.

This does not make the output an applyable patch — rtk still drops the diff --git / --- / +++ headers, so a patch parser rejects it regardless. Use rtk proxy git diff when you need a faithful patch. The indent was never what made that case fail.

docs/usage/FEATURES.md is updated to real captured output; its illustration showed change lines indented under a src/main.rs (+5/-2) header that no version of rtk emits.

Testing

cargo fmt --check, cargo clippy --all-targets --all-features -- -D warnings, and cargo test --all are clean on this branch, apart from two hooks::rewrite_cmd::tests::unattestable_passthrough failures that reproduce with this branch's changes stashed and that it does not touch. cmds::git::git is 119 tests.

Ordinary unified diffs come out byte-identical to the previous head: checked on a 26,561-line real diff of this repo, on a 390-line one, and on a 300-case generated corpus covering additions, deletions, replacements, empty rows, Unicode content, leading + / - / \, multiple hunks and multiple files. The one intentional divergence is a hunk that ends without a change line, which now emits its buffered context instead of a bare header.

[KuSh]

Requesting changes. The de-indent itself is correct and verified — I built the branch and confirmed rtk git diff | grep -c '^-' returns the true deletion count where it previously returned 0, that rtk's own +N -M tally and truncation notes correctly stay indented so they aren't miscounted, and that no caller re-parses this output (pipe_cmd.rs:54, gh_cmd.rs:952, glab_cmd.rs:490 all just print it). Token cost goes down slightly, so never_worse is unaffected. Full suite green.

This is the version of the column-0 fix being taken forward — the competing #3646 is closed in its favour.

What needs another pass is the contract the PR body sells. "grep -c '^-' gives you the exact deletion count" is the promise, and there are three holes in it, two of them on lines this PR edits. Details inline; the third is here because it isn't tied to a single line:

Past max_hunk_lines (100) the count silently stops being exact. Verified: a 300-deletion hunk yields grep -c '^-' = 100 while the tally correctly says +300 -300. The ... (500 lines truncated) note is deliberately indented — correctly, so the same grep doesn't count it — which means the anchored audit you're recommending has no way to see that it was truncated at all, and the note merges additions and deletions into one number. Either a per-sign note (... (N- deletions, N+ additions truncated)) or an explicit caveat in the PR body would close it honestly.

None of this makes the PR worse than the status quo — it's strictly better. It's just that the moment an audit is documented as trustworthy, people write CI gates against it.

[KuSh]

Thanks for the rework — the core fix is right, and I verified it: on ordinary git diff output ^- / ^+ anchor again, and dropping the +++ / --- guards also fixes the silent content loss I flagged earlier (a deleted line starting with --, an added ++i;). hunk_truncation_note splitting by sign is a good call, and the flush points all match the previous behaviour. The new tests cover the right things, keep1 included.

One blocking issue, one open question, one docs item.

Blocking — combined diffs (diff --cc) still emit false ^- / ^+ lines. Details inline. This one is introduced by the PR, not pre-existing: on master those header lines came out indented, so ^- did not match them.

Open question — the leading-context budget. Details inline and on the existing thread; it addresses what I asked for, but it now competes with change lines for the global max_lines budget.

Docs — docs/usage/FEATURES.md:333-341. The rtk git diff Avant / Apres block shows change lines indented with + / - / ~ under a src/main.rs (+5/-2) header. That illustration was already drifting before this PR, but this PR changes the emitted shape again (hunk bodies and @@ headers now at column 0, +5 -2 as a trailing indented line). Since the whole point of this PR is that the emitted shape is greppable, please update that block to match what compact_diff now produces — otherwise the one page a user would read to learn the format still teaches the wrong anchors.

[iliaal]

Thanks — all three inline points and the truncation gap are fixed in 4efc0d9.

+++ / --- guards. You're right that they can only match content inside a hunk, and you're right about the consequence. Both guards are gone from the in_hunk branch. test_compact_diff_keeps_content_starting_with_plus_or_minus covers your exact case: a diff removing -- sql comment and adding ++i; now renders both lines and reports +1 -1.

Leading context. I didn't want to ship a PR whose body says "nothing is dropped" while hunk_shown > 0 silences the context before a hunk's first change, so I fixed it here rather than deferring — but not by deleting the condition, for the reason you gave. Leading context gets its own budget of three lines, matching git's default -U3, and it doesn't touch hunk_shown, so a long run of context can't crowd out the change it frames. test_compact_diff_leading_context_has_its_own_budget puts 20 context lines ahead of 100 deletions and asserts all 100 deletions still render. The anchorable test now asserts on keep1 as well. If #1855 wants to generalize the budget, this shouldn't get in its way.

The comment. Correct on both counts — it narrated history and it sat on the @@ branch while describing the branch below. Replaced with a single line stating the invariant, on the in_hunk branch. The narration in the test is gone too; the reasoning lives in the commit message and the PR body.

Truncation past max_hunk_lines. Took the per-sign note: ... (30 deletions, 30 additions truncated), or the single-sign form when only one side is cut. It stays indented, so the anchored greps still skip it. That's the honest version of the contract — the note can't be seen by grep '^-' by design, so it has to tell a reader who does look how many deletions they didn't see, and one merged count couldn't. test_compact_diff_truncation_note_splits_by_sign pins it.

cargo fmt --all --check, cargo clippy --all-targets --all-features -- -D warnings, and cargo test --all are clean on the branch, apart from two hooks::rewrite_cmd::tests::unattestable_passthrough failures I reproduced with my changes stashed — they predate this PR and it doesn't touch rewrite_cmd.rs.

I left condense_unified_diff alone: same class of bug, but it belongs with #3769 rather than in a PR about compact_diff's indent.

[iliaal]

Thanks — the diff --cc catch is right, and it's mine: on master those headers came out indented, so this branch is what let them anchor. Fixed in 201b550.

Combined diffs. Reproduced your exact numbers on a two-file conflict with zero deletions — ^- = 1, ^+ = 9, with a literal --- a/z.txt and +++ b/z.txt at column 0. The reset now fires on line.starts_with("diff --"), so --git, --cc, and --combined all clear in_hunk, and the path extraction falls back to the trailing token when there's no b/ to split on. Same repo now gives ^- = 0, ^+ = 8, the 8 being the real ++-prefixed conflict lines. test_compact_diff_combined_diff_headers_are_not_hunk_content uses a two-section diff --cc fixture and asserts no ^- match and no +++ b/ in the body.

Leading context. You're right that the comment justified a problem that never existed — before this branch leading context was never emitted, so it couldn't crowd anything out of max_hunk_lines. The comment now states the real trade-off.

I took the exemption rather than dropping the feature, with one change to your suggestion. A pure exemption has a hole: a diff of many small hunks spends three exempt lines on each, so the output can overrun max_lines several times over. So leading context is exempt from max_lines and capped at max_lines / 10 across the whole diff. Change lines are never displaced; the overrun is bounded at 10%.

Measured on a five-commit diff of this repo, max_lines = 500:

build	change lines	bytes	reduction vs raw
master (1c5f164)	0	34,693	71.8%
4efc0d9 (context counted)	144	31,858	74.1%
201b550 (exempt + capped)	201	34,920	71.7%

So the exemption buys back 57 change lines for 0.1 points of reduction against master, which showed zero anchorable change lines at almost exactly the same byte count. Well clear of the 60% floor. Two tests pin it: one builds the same 30-hunk diff with and without leading context and asserts the change-line count is identical, the other builds 200 single-change hunks and asserts the context stays within max_lines / 10.

docs/usage/FEATURES.md. Rewritten with real captured output rather than the illustration — the old block showed change lines indented under a src/main.rs (+5/-2) header that no version of rtk has emitted. It now also states both limits of the anchored audit: past 100 change lines per hunk grep -c "^-" plateaus at what's shown, with the per-sign note carrying the rest where the same grep can't see it, and the output is not an applyable patch because rtk drops the diff --git / --- / +++ headers. rtk proxy git diff for that.

cargo fmt --all --check and cargo clippy --all-targets --all-features -- -D warnings clean; cargo test --all at 2506 passed, with the same two pre-existing hooks::rewrite_cmd::tests::unattestable_passthrough failures I reproduced with the branch's changes stashed.

[KuSh]

Both items from my previous review are fixed, and I checked them against a build of 201b550 rather than by reading the diff:

• diff --cc header leak — my earlier repro now gives ^- = 0 (was 1) and ^+ = 8 (was 9), with the per-file tally back to +4 -0.
• Leading context displacing changes — 354 change lines on a real 5-commit diff of this repo, identical to master's 354, where the previous head gave 347. The exemption plus the diff-wide cap does exactly what it says.
• FEATURES.md — I reproduced the example verbatim; the block content is byte-accurate.

cargo test --all on this head: 2508 passed, 0 failed. I have resolved those threads.

A second pass turned up four things, one of which I would block on:

1. Blocking — compact_diff still misreads patch/mbox input, and it lands on this PR's headline use case (gh pr diff --patch | grep -c "^-"). Inline.
2. Leading context keeps the first lines of a hunk, so the emitted body is not contiguous with the change and nothing says so. Inline.
3. Combined diffs under-count by design — on exactly the format this PR just added support for. Inline, plus a docs note.
4. diff_header_path truncates any path containing a space. Inline.

Plus two comment-convention nits and a docs count, all inline.

One thing not worth an inline comment: exempting leading context from max_lines makes it a soft cap — output can now exceed it by up to 10% (git stash show -p passes 100, so up to 110). That is deliberate and the code comment explains it well; it just deserves a line in the PR description, since the two call sites chose those numbers as hard budgets.

Unrelated heads-up: src/cmds/git/git.rs is being renamed to src/cmds/git/git_cmd.rs on another branch. If that lands first this PR will need a rebase.

[iliaal]

All four addressed in dc44930, and the mbox one is a good catch — it is exactly the case this PR was written for.

Patch and mbox input. Took both of your routes, since they close different halves. --patch joins has_non_diff_format_flag, so gh pr diff --patch passes through whole: someone asking for a patch wants one that applies, and compact_diff has no business parsing an mbox. And the hunk now ends where it says it ends — @@ -a,b +c,d @@ declares its line counts, so the body stops when they are consumed and everything after it is outside every hunk. Your two-patch mbox with two real deletions now gives grep -c "^-" = 2, and Subject: / From no longer appear in any body. You were right that a heuristic could not work here: a deleted line whose content is - renders as -- , so only the length approach distinguishes them. test_compact_diff_mbox_signature_is_not_a_deletion covers it, and test_parse_hunk_header_counts pins the parsing, including the @@ -1 +1 @@ omitted-count form and the combined @@@.

The general benefit is bigger than the mbox: any trailing text after a diff, and any truncated input, now stops being read as hunk content.

Leading context adjacency. Fixed with the ring buffer you described — it keeps the last three lines, so with -U10 the body shows ctx8 ctx9 ctx10 rather than ctx1 ctx2 ctx3. Same line count, and the adjacency it implies is real. I also moved the diff-wide budget so it is charged when the buffer drains rather than when a line enters it, so a line the ring evicted costs nothing. test_compact_diff_leading_context_is_adjacent_to_the_change asserts the exact three.

Combined diff under-count. Widened rather than documented-as-is, at least for the tally: the marker check now reads the first prefix_width columns, which parse_hunk_header derives from the @ run, so +ours and -theirs are counted. Verified against git diff --numstat on three complete commit ranges — the tally is now exact on all three, where master under-counted one by +2 -1.

The grep half I cannot fix without rewriting the line, so it is documented instead: docs/usage/FEATURES.md now lists it as a third limit next to the 100-lines-per-hunk cap, naming merge, rebase, cherry-pick and stash pop. The test asserts the tally at +5 -0 and the anchored count at 4 per file deliberately, with a comment saying which is which.

diff_header_path and spaces. Fixed with splitn(3, ' ').nth(2), and the dead unwrap_or is gone from the reasoning — test_diff_header_path_keeps_spaces covers all three header shapes with my file.txt.

Comment convention. Both rewritten to state the invariant. Fair repeat: I fixed this on the @@ branch and then wrote the same shape two comments later.

FEATURES.md. The count is 24, and the stat's first line has no leading space. On the thin-illustration point you are right, and the honest version is that a two-file diff saves 11% because git's own -U3 output is already compact — the compression comes from the per-file headers and the per-hunk cap. So the block now states that 11% directly and adds a measured larger case: 4 files with a 300-line hunk, 335 lines and 13,262 bytes down to 140 and 4,880, 63% reduction, with the truncation note and the recovery hint both visible.

Soft cap. Added to the PR description, with the git stash show -p call site named: 100 can now yield up to 110.

Noted on the git.rs to git_cmd.rs rename; happy to rebase whenever it lands.

cargo fmt --all --check and cargo clippy --all-targets --all-features -- -D warnings clean; cargo test --all at 2511 passed, with the same two pre-existing hooks::rewrite_cmd::tests::unattestable_passthrough failures that reproduce with this branch's changes stashed.

[KuSh]

All seven comments from the last round are fixed, and I checked each against a build of dc44930 rather than by reading:

• A real git format-patch --stdout mbox now gives ^- = 2 for its 2 real deletions (was 5), with the --- / -- / From: / Subject: envelope gone entirely. Ending the hunk at its declared length is the right shape for this, and the --patch passthrough is a good belt-and-braces.
• -U10 now emits ctx8 ctx9 ctx10 — contiguous with the change.
• A conflicted my file.txt keeps its full name; the combined tally counts +main (+5 -0, was +4 -0).
• The docs example re-measures correctly (21 lines / 440 bytes raw against 24 / 392 filtered), and the third limit is accurate.

Change-line parity holds against master (390 = 390 on a real 5-commit diff of this repo) and cargo test --all gives 2508 -> 2513 passing. I have resolved all of those threads.

The rework introduces four new problems, one of which is a crash:

1. &line[..width] panics on non-ASCII. rtk git diff --word-diff exits 101 with empty stdout on this branch and works on master.
2. Combined-diff old accounting is wrong in both directions, so declared-length termination — the mechanism this commit is built around — silently does not apply to diff --cc, and the reverse case drops change lines with no marker.
3. Buffered leading context is dropped when a hunk ends without a change line.
4. diff_header_path mishandles git's quoted (non-ASCII) paths.

1 and 2 are related but independent: fixing 2 does not fix 1, since --word-diff hunks are ordinary @@ whose counts mismatch by construction. Leaving 2 unfixed means the hunk-length guard does not protect conflict diffs, which is where this PR started.

[iliaal]

One thing I looked at and left: max_lines can overshoot by two entries at a file boundary, because the previous file's tally and the next file's header are pushed before the budget check. At max_lines = 200 over 100 single-hunk files the output carries 201 non-context entries. It is byte-identical on master and on dc44930, so it is not this PR's, and it is a second source of overshoot alongside the context exemption the description already documents. Say the word if you want it in scope here rather than on its own.

Non-regression: ordinary unified diffs are byte-identical to dc44930, verified on a 26,561-line real diff of this repo and on a 390-line one, plus a 300-case generated corpus covering additions, deletions, replacements, empty rows, Unicode, leading +/-/\, multiple hunks and multiple files. The intentional divergence is the context-only hunk. cargo fmt --all --check and cargo clippy --all-targets clean, cmds::git::git 119/119.

[KuSh]

Approving. Everything raised across the four rounds is addressed, and I verified this head by building and running it rather than by reading the diff.

What I checked on this head

• --word-diff, --color-words, --word-diff=porcelain pass through untouched (exit 0); --word-diff=none compacts again — 25,307 bytes against 101,259 raw on a five-commit diff of this repo, so the saving that flag used to cost is back.
• A real git format-patch --stdout mbox gives ^- = 2 for its two deletions, envelope and -- signature gone.
• A real diff --cc conflict: ^- = 0, +5 -0 counting the column-2 marker, my file.txt keeping its space.
• core.quotepath path renders as été.txt, so rtk git diff | grep été finds it.
• --dst-prefix=w/ names git.rs; diff --cc dup dup names dup dup.
• -U10 leading context comes out adjacent to the change (ctx8 ctx9 ctx10).
• Change-line parity against develop: 453 = 453 on a real diff, so nothing is lost relative to the old behaviour.
• cargo fmt --all --check, cargo clippy --all-targets, cargo test --all — clean, 2527 passing.

The two commits I pushed (thanks for the maintainer-edit box): e224cf4 decodes git's quoting rather than only unwrapping it, gates the midpoint split on the header kind so a single-path diff --cc is not split, matches prefixes against each other so --dst-prefix reads like any other pair, and honours --word-diff=none last-one-wins. af01931 is a one-line plural fix — a hunk truncated at exactly one line over the cap said 1 deletions truncated.

Two shapes consciously accepted, both documented in the code:

• A --no-prefix rename between two directories of equal path length reads as the shared trailing path. Space-separated paths with no prefix are ambiguous by construction; there is no signal to disambiguate on.
• Lines between a hunk's declared end and the next header are dropped. That is the mbox fix working as intended, and it means git log -p | rtk pipe --filter git-diff no longer shows commit <sha> / Author: lines as indented context. The old rendering was accidental and rtk git log -p does not route through compact_diff, so this is not worth holding the PR for — but it is a real difference for that one pipe.

Actually blocked, waiting for #3793 to be merged