feat(diff): line-count labels on compact-mode dividers (⋯ N lines ⋯)#146
Merged
feat(diff): line-count labels on compact-mode dividers (⋯ N lines ⋯)#146
Conversation
parser computes the gap from hunk-header metadata and renders it as "⋯ N lines ⋯" (midline horizontal ellipsis). previously dividers were a static "...". uses prevOldEnd tracked via parsed hunk lengths rather than oldNum, so insertion-only hunks (@@ -K,0 ...) compute correctly — oldNum does not advance on `+` body lines. max(oldLen, 1) handles both omitted-length (git implicit 1) and literal-zero cases. leading divider appears when the first hunk starts after line 1; trailing divider is out of scope (needs total file-line count threaded through Renderer). Related to #145
parseUnifiedDiff now accepts totalOldLines and emits a "⋯ N lines ⋯" divider at the end when the last hunk does not reach EOF. each VCS implementation (git/hg/jj) computes the pre-change file's line count via git show / hg cat / jj file show against the resolved old-ref (range ref left operand, single ref, or sensible default). extracts appendGapDivider helper to unify leading / between-hunks / trailing emission — same "⋯ N lines ⋯" format, different gap source. test callers updated to pass 0 (unknown total, skip trailing). 6 new parser-level tests cover trailing plural / singular / no-trailing / unknown-total / leading+between+trailing / empty-diff cases. Related to #145
- git totalOldLines: check "..." before ".." in ref splitting so A...B takes left operand A (not "A" + ".B"). triple-dot ranges still show the left operand of the range rather than merge-base, which is approximate but matches hg/jj behavior. - gate totalOldLines subprocess behind compact mode in all VCS impls; full-file mode always reaches EOF so the trailing-divider probe was a wasted subprocess (~2× process launches per FileDiff call). - extract countLines helper (called from 3 VCS totalOldLines methods). - update ChangeDivider constant + DiffLine.Content godoc with new "⋯ N line[s] ⋯" format and pattern-match warning. - unify hg/jj totalOldLines godoc with git's bullet format. - clean up stale contradictory comments in TestParseUnifiedDiff_TrailingDivider. - add TestGit_TotalOldLines (8 ref-resolution cases, locks in triple-dot fix). - 3 new TestParseUnifiedDiff_TrailingDivider cases: insertion-only last hunk, deleted file, exact boundary. - CLAUDE.md: extend Compact mode gotcha with divider-position details and add gotcha warning against pattern-matching DiffLine.Content on dividers. Related to #145
There was a problem hiding this comment.
Pull request overview
Adds informative skipped-line dividers to compact-mode unified diff rendering by labeling unchanged gaps as ⋯ N line[s] ⋯, including leading (pre-first-hunk), between-hunk, and optional trailing (post-last-hunk) gaps.
Changes:
- Extend
parseUnifiedDiffto compute and emit labeled dividers based on hunk-header metadata, including singular/plural formatting. - Add per-VCS “old file line count” probes (Git/Hg/Jj) in compact mode to enable trailing divider emission.
- Expand parser/unit tests and update CLAUDE.md to document the new
ChangeDividercontent contract.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| app/diff/diff.go | Updates hunk header parsing + divider emission logic; adds totalOldLines plumbing and helpers. |
| app/diff/hg.go | Computes totalOldLines in compact mode via hg cat and passes it into the parser. |
| app/diff/jj.go | Computes totalOldLines in compact mode via jj file show and passes it into the parser. |
| app/diff/diff_test.go | Updates existing tests for new parser signature; adds extensive gap/trailing-divider coverage + Git line-count tests. |
| CLAUDE.md | Documents new divider labeling behavior and warns against pattern-matching divider strings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+749
to
+754
| // trailing divider: unchanged lines after the last hunk on the old side. | ||
| // Emitted only when the caller supplied totalOldLines AND at least one hunk | ||
| // was processed (prevOldEnd > 1 means a hunk advanced it past the initial value). | ||
| if totalOldLines > 0 && prevOldEnd > 1 { | ||
| lines = appendGapDivider(lines, totalOldLines-prevOldEnd+1) | ||
| } |
There was a problem hiding this comment.
Trailing divider emission is currently gated by prevOldEnd > 1, but prevOldEnd stays at 1 for an insertion-only hunk at the start of an existing file (@@ -0,0 ...@@). In that case at least one hunk was processed and, when totalOldLines > 0, a trailing divider should still be emitted for the unchanged old-side lines after the insertion. Consider tracking an explicit sawHunk boolean (set when a hunk header is parsed) and gating on that instead of prevOldEnd > 1 (or adjust the sentinel/initialization so insertion-at-start counts as processed).
Replaces the prevOldEnd > 1 guard with an explicit sawHunk boolean. The previous guard conflated "did we process any hunk" with "did the last hunk end past line 1", so a hypothetical `@@ -0,0 +1,N @@` hunk alongside totalOldLines > 0 would suppress the trailing divider even though the hunk was processed (prevOldEnd stays at 1 for that case: 0 + max(0,1) = 1, the same as the initial value). Adds a test locking in the behavior. The practical impact is nil today (git/hg/jj only emit @@ -0,0 on genuinely new files where totalOldLines=0 and the outer guard already skips), but the cleaner guard documents intent and avoids future surprises. Addresses Copilot inline review on #146.
Deploying revdiff with
|
| Latest commit: |
bd745af
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ebc82060.revdiff.pages.dev |
| Branch Preview URL: | https://compact-improve.revdiff.pages.dev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Enhances compact-mode diff rendering with informative
⋯ N line[s] ⋯dividers at three positions — leading (before first hunk when it doesn't start at line 1), between non-adjacent hunks, and trailing (after last hunk when it doesn't reach EOF). Previously the parser only emitted a static"..."divider between hunks with no count.Behavior
Between-hunk dividers now show the number of unchanged lines skipped. Computed from hunk-header metadata (
prevOldEndtracked across hunks) rather thanoldNum, so insertion-only hunks (@@ -K,0 ...) count correctly —oldNumdoesn't advance on+body lines.Leading divider: emitted when the first hunk starts after line 1. Gap =
oldStart - 1.Trailing divider: emitted when
totalOldLines > 0and last hunk doesn't reach EOF. Each VCS impl (Git/Hg/Jj) fetches the pre-change file's line count viagit show <old-ref>:<file>/hg cat/jj file show, gated behind compact mode (full-file mode always reaches EOF, probe skipped).Notable details
A...B) take the left operand as the old side in all three VCS impls. Approximate (true old side is merge-base) but consistent across git/hg/jj and accurate enough for an informational line count.DiffLine.ContentforChangeDividerrows is now a human-readable label (⋯ 47 lines ⋯) — dispatch onChangeType == ChangeDivider, never pattern-match the string. Documented in CLAUDE.md.Borrows an idea from #145 (visual divider labels shown in the reporter's screenshots).