diff --git a/CLAUDE.md b/CLAUDE.md index 9c0d58f2..a04633fb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -93,7 +93,8 @@ TUI for reviewing diffs, files, and documents with inline annotations, built wit - Overlay popups managed by `overlay.Manager`. `Compose()` uses ANSI-aware compositing via `charmbracelet/x/ansi.Cut`. `HandleKey()` returns `Outcome` — Model switches on `OutcomeKind` for side effects (file jumps, theme apply/persist). Overlay kinds: help, annot-list, theme-select, commit-info. One overlay at a time — opening any overlay auto-closes whichever was previously open - Overlay mouse routing: `Manager.HandleMouse` mirrors `HandleKey` (wheel + left-click). `app/ui/mouse.go::handleOverlayMouse` delegates when any overlay is active — wheel scrolls the overlay's own state, clicks in annotlist/themeselect select, clicks in commitinfo/help are no-ops, clicks outside the popup are swallowed (no dismiss). `Close()` must reset `Manager.bounds = popupBounds{}` or a stale rectangle from the previous overlay will satisfy click hit-tests. `overlayCenter()` records the popup rectangle as a side effect of rendering — if a new Open* path bypasses Compose, clicks will be treated as outside. Row offsets are hardcoded per overlay (annotlist `contentTop=2`, themeselect `entriesTop=4`, both `horizChromeCols=2`); any change to lipgloss `Padding(...)` on the overlay box style or to the number of lines rendered before the first entry requires a matching update in that overlay's `handleLeftClick`. `overlay.WheelStep` (shared with `app/ui.wheelStep`) defines the wheel notch size across panes. - Reload (`R` key): `reloadState` on `Model` holds `pending bool` (waiting for y/cancel), `hint string` (transient status-bar message), and `applicable bool` (false in `--stdin` mode — stream consumed). `ReloadApplicable` is wired at the composition root in `main.go`, following the same pattern as `CommitsApplicable`. The reload method is named `triggerReload()` — not `reload()` — because Go forbids a method and a field with the same name on the same type (`Model.reload` is the state field). Reload resets the diff cursor to the top of the file; tree selection (which file) is restored by `SelectByPath` in `handleFilesLoaded`. -- Compact mode (`C` key, `--compact` / `--compact-context=N`): shrinks the VCS diff at generation time (not at render time) by passing a `contextLines int` parameter through every `Renderer.FileDiff()` callsite. VCS renderers (Git, Hg, Jj) translate it via per-renderer helpers — `gitContextArg` / `hgContextArg` produce `-U`, `jjContextArg` produces `--context=`. Sentinel for full-file: `contextLines <= 0` or `>= 1000000` → use the full-file arg (`-U1000000` / `--context=1000000`). Context-only sources (FileReader, DirectoryReader, StdinReader) accept the parameter but ignore it — there are no changes to contextualize. `CompactApplicable` is computed at the composition root (`compactApplicable()` in `main.go`) via type assertion on the renderer chain (same approach as `CommitLogger`, no new interface); `false` for `--stdin`, `--all-files`, and file-only modes without VCS. Toggle re-fetches only the current file via `reloadCurrentFile()` — a deliberately lightweight sibling of `triggerReload()` that bumps `file.loadSeq` and returns `loadFileDiff(m.file.name)` without re-fetching the files list or commit log. Cursor reset to first hunk happens naturally via `skipInitialDividers()` in `handleFileLoaded` after the reload completes. `m.currentContextLines()` is the helper every `FileDiff` callsite must use — returns `m.modes.compactContext` if `m.modes.compact && m.compact.applicable`, else `0`. Runtime state (applicability + transient hint) lives on `compactState` alongside `reloadState` / `commitsState`; user-toggled state (on/off, context size) stays on `modeState` with the other view toggles. Composes cleanly with `--collapsed` (different layers: compact shrinks diff before parsing, collapsed hides removed lines during rendering). +- Compact mode (`C` key, `--compact` / `--compact-context=N`): shrinks the VCS diff at generation time (not at render time) by passing a `contextLines int` parameter through every `Renderer.FileDiff()` callsite. VCS renderers (Git, Hg, Jj) translate it via per-renderer helpers — `gitContextArg` / `hgContextArg` produce `-U`, `jjContextArg` produces `--context=`. Sentinel for full-file: `contextLines <= 0` or `>= 1000000` → use the full-file arg (`-U1000000` / `--context=1000000`). Context-only sources (FileReader, DirectoryReader, StdinReader) accept the parameter but ignore it — there are no changes to contextualize. `CompactApplicable` is computed at the composition root (`compactApplicable()` in `main.go`) via type assertion on the renderer chain (same approach as `CommitLogger`, no new interface); `false` for `--stdin`, `--all-files`, and file-only modes without VCS. Toggle re-fetches only the current file via `reloadCurrentFile()` — a deliberately lightweight sibling of `triggerReload()` that bumps `file.loadSeq` and returns `loadFileDiff(m.file.name)` without re-fetching the files list or commit log. Cursor reset to first hunk happens naturally via `skipInitialDividers()` in `handleFileLoaded` after the reload completes. `m.currentContextLines()` is the helper every `FileDiff` callsite must use — returns `m.modes.compactContext` if `m.modes.compact && m.compact.applicable`, else `0`. Runtime state (applicability + transient hint) lives on `compactState` alongside `reloadState` / `commitsState`; user-toggled state (on/off, context size) stays on `modeState` with the other view toggles. Composes cleanly with `--collapsed` (different layers: compact shrinks diff before parsing, collapsed hides removed lines during rendering). Divider lines now carry line-count labels: `parseUnifiedDiff` emits `⋯ N line[s] ⋯` dividers at three positions — leading (first hunk does not start at line 1), between non-adjacent hunks (computed from `prevOldEnd` tracked via hunk-header metadata, handles insertion-only `@@ -K,0 ...` hunks where `oldNum` does not advance on `+` lines), and trailing (last hunk does not reach EOF). Trailing requires `totalOldLines` passed by the VCS impl; Git/Hg/Jj `FileDiff` fetch it via `git show :` / `hg cat` / `jj file show` and only when `contextLines > 0 && contextLines < fullContextSentinel` — full-file mode always reaches EOF so the probe is skipped. +- `diff.DiffLine.Content` for `ChangeDivider` rows is a human-readable `⋯ N line[s] ⋯` label produced by `parseUnifiedDiff`. Never pattern-match the string — dispatch on `ChangeType == diff.ChangeDivider`. Test fixtures may construct `ChangeDivider` rows with arbitrary `Content` (e.g. `"..."`, `"@@..."`); that is a test-only shortcut and NOT the parser's contract. - Commit info overlay (`i` key) uses the `diff.CommitLogger` capability interface (additive to `diff.Renderer`). Model resolves a `commitLogSource` at construction: explicit `ModelConfig.CommitLog` wins, else type-asserts the renderer for `CommitLogger`, else the feature is unavailable and `i` is a no-op with a transient status-bar hint. `CommitsApplicable` is computed at the composition root by `commitsApplicable()` in `main.go` (using the `commitLogger` field populated by `setupVCSRenderer` in `renderer_setup.go`) — Model copies it, does not re-derive. Data is fetched eagerly at startup and on `R` reload: `Init()` and `triggerReload()` both return `tea.Batch(m.loadFiles(), m.loadCommits())`, running files and commits loads in parallel as independent goroutines. `loadCommits()` captures `m.commits.loadSeq` at invocation time and tags the resulting `commitsLoadedMsg` with it; `handleCommitsLoaded` drops any message whose seq no longer matches (stale-result guard, mirrors the files-load pattern). Eager parallel fetch shrinks the window where overlay and diff can disagree from "time until first `i` press" to "skew between two parallel goroutine starts" (milliseconds). Not a strict snapshot guarantee — the two subprocesses each resolve `HEAD` independently — but the practical race window is tens of ms instead of minutes. If the user presses `i` before `commitsLoadedMsg` arrives, `handleCommitInfo` sets a transient `loading commits…` hint instead of opening the overlay — a second press after load succeeds. Hg cannot use literal NUL in argv templates — use ASCII US/RS (`\x1f`/`\x1e`) as field/record separators for hg only - **ANSI nesting with lipgloss**: `lipgloss.Render()` emits `\033[0m` (full reset) which breaks outer style backgrounds. For styled substrings inside a lipgloss container (status bar separators, search highlights, diff cursor, annotation lines), use raw ANSI sequences via the style sub-package (`style.AnsiFg`, `resolver.Color`), or dedicated `Renderer` methods. Never use `lipgloss.NewStyle().Render()` for inline elements within a lipgloss-rendered parent. - **Background fill for themed panes**: lipgloss pane `Render()` and viewport internal padding emit plain spaces after reset, causing terminal default bg. Workarounds: (1) `extendLineBg()` pads lines to full width, (2) `padContentBg()` re-pads pane content, (3) `BorderBackground()` on border styles. **Ordering**: `extendLineBg()` must be called AFTER `applyHorizontalScroll()`. diff --git a/app/diff/diff.go b/app/diff/diff.go index 1c4a8856..3822e659 100644 --- a/app/diff/diff.go +++ b/app/diff/diff.go @@ -20,7 +20,7 @@ const ( ChangeAdd ChangeType = "+" ChangeRemove ChangeType = "-" ChangeContext ChangeType = " " - ChangeDivider ChangeType = "~" // separates non-adjacent hunks + ChangeDivider ChangeType = "~" // marks a skipped unchanged region (leading, between-hunk, or trailing) // fullContextSentinel is the numeric threshold that callers use to request // full-file diff context. contextLines <= 0 or >= fullContextSentinel causes @@ -45,7 +45,7 @@ const ( type DiffLine struct { OldNum int // line number in old version (0 for additions) NewNum int // line number in new version (0 for removals) - Content string // line content without the +/- prefix + Content string // line content without the +/- prefix; for ChangeDivider rows it is a human-readable "⋯ N line[s] ⋯" label — never pattern-match it, dispatch on ChangeType ChangeType ChangeType // changeAdd, ChangeRemove, ChangeContext, or ChangeDivider IsBinary bool // true when this line is a binary file placeholder IsPlaceholder bool // true for non-content placeholders (broken symlink, non-regular file, too-long lines) @@ -391,7 +391,13 @@ func (g *Git) FileDiff(ref, file string, staged bool, contextLines int) ([]DiffL return nil, fmt.Errorf("get file diff for %s: %w", file, err) } - lines, err := parseUnifiedDiff(out) + // trailing divider is only meaningful in compact mode — full-file mode always + // reaches EOF, so probing the old-file size would be a wasted subprocess. + total := 0 + if contextLines > 0 && contextLines < fullContextSentinel { + total = g.totalOldLines(ref, file, staged) + } + lines, err := parseUnifiedDiff(out, total) if err != nil { return nil, err } @@ -406,6 +412,39 @@ func (g *Git) FileDiff(ref, file string, staged bool, contextLines int) ([]DiffL return lines, nil } +// totalOldLines returns the line count of the pre-change version of file, used by +// parseUnifiedDiff to emit a trailing divider. Returns 0 when the old-side file is +// unavailable (new files, bad refs, etc.) — the parser treats 0 as "unknown" and +// skips the trailing divider. +// +// Old-side resolution: +// - ref empty + staged → HEAD (git diff --cached compares HEAD against index) +// - ref empty + not staged → index via `git show :path` +// - ref contains ".." or "..." → left operand (triple-dot checked first so A...B +// is not mis-split on the leading "..") +// - single ref → use as-is +// +// For triple-dot ranges the left operand is an approximation of the true old side +// (merge-base(A,B)); accurate enough for the informational trailing-divider count. +func (g *Git) totalOldLines(ref, file string, staged bool) int { + oldRef := ref + if left, _, ok := strings.Cut(ref, "..."); ok { + oldRef = left + } + if left, _, ok := strings.Cut(oldRef, ".."); ok { + oldRef = left + } + if oldRef == "" && staged { + oldRef = "HEAD" + } + // `git show :path` (empty oldRef) shows the index version of the file + out, err := g.runGit("show", oldRef+":"+file) + if err != nil { + return 0 + } + return countLines(out) +} + // gitContextArg returns the -U argument for git diff given the caller's requested // context size. A non-positive contextLines or one at or above fullContextSentinel // returns the full-file arg; any other value returns -U. @@ -557,8 +596,37 @@ func (g *Git) formatSize(bytes int64) string { } } -// hunkHeaderRe matches unified diff hunk headers like @@ -1,5 +1,7 @@ -var hunkHeaderRe = regexp.MustCompile(`^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@`) +// hunkHeaderRe matches unified diff hunk headers like @@ -1,5 +1,7 @@. +// Lengths are optional per git's spec (omitted means length 1) and are captured +// so the parser can compute the old-side end of each hunk. +var hunkHeaderRe = regexp.MustCompile(`^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@`) + +// countLines returns the number of lines in s, counting a final non-newline-terminated +// line as one additional line. Empty input returns 0. Used by the per-VCS totalOldLines +// methods to translate file contents into a line count for the trailing divider. +func countLines(s string) int { + if s == "" { + return 0 + } + n := strings.Count(s, "\n") + if !strings.HasSuffix(s, "\n") { + n++ + } + return n +} + +// appendGapDivider appends a "⋯ N lines ⋯" divider to lines when gap is positive. +// Used for leading, between-hunks, and trailing dividers — same format, different +// source of the gap count. Returns lines unchanged when gap <= 0 (nothing to show). +func appendGapDivider(lines []DiffLine, gap int) []DiffLine { + switch { + case gap == 1: + return append(lines, DiffLine{ChangeType: ChangeDivider, Content: "⋯ 1 line ⋯"}) + case gap > 1: + return append(lines, DiffLine{ChangeType: ChangeDivider, Content: fmt.Sprintf("⋯ %d lines ⋯", gap)}) + } + return lines +} // binaryFilesRe matches git's "Binary files ... differ" line for binary diffs. // Assumes English locale; non-English git may localize this message. @@ -568,7 +636,12 @@ var binaryFilesRe = regexp.MustCompile(`^Binary files .+ and .+ differ$`) // it handles the diff header, hunk headers, and content lines. // for binary diffs ("Binary files ... differ"), it returns a single placeholder DiffLine. // intended for single-file diffs; multi-file diffs are not fully supported. -func parseUnifiedDiff(raw string) ([]DiffLine, error) { +// +// totalOldLines is the total line count of the pre-change file, used to emit a +// trailing "⋯ N lines ⋯" divider after the last hunk when it does not reach EOF. +// Pass 0 when unknown (context-only sources, tests, or any case where the caller +// cannot cheaply determine the old file's size) — trailing divider is then skipped. +func parseUnifiedDiff(raw string, totalOldLines int) ([]DiffLine, error) { var lines []DiffLine scanner := bufio.NewScanner(strings.NewReader(raw)) scanner.Buffer(make([]byte, 0, bufio.MaxScanTokenSize), MaxLineLength) @@ -576,7 +649,13 @@ func parseUnifiedDiff(raw string) ([]DiffLine, error) { // skip diff header lines (---, +++, diff --git, index, etc.) inHeader := true var oldNum, newNum int - firstHunk := true + // prevOldEnd = next untouched old-side line. Initialized to 1 so the first hunk's + // leading divider uses the same `oldStart - prevOldEnd` formula as between-hunks gaps. + prevOldEnd := 1 + // sawHunk tracks whether any hunk header was parsed — used as the guard for the + // trailing divider so that insertion-at-start hunks (@@ -0,0 ...) don't collide + // with the prevOldEnd==1 initialization sentinel. + var sawHunk bool var isNewFile, isDeletedFile bool for scanner.Scan() { @@ -608,16 +687,25 @@ func parseUnifiedDiff(raw string) ([]DiffLine, error) { // parse hunk header if m := hunkHeaderRe.FindStringSubmatch(line); m != nil { oldStart, errOld := strconv.Atoi(m[1]) - newStart, errNew := strconv.Atoi(m[2]) + newStart, errNew := strconv.Atoi(m[3]) if errOld != nil || errNew != nil { return nil, fmt.Errorf("parse hunk header %q: old=%w new=%w", line, errOld, errNew) } - - // add divider between non-adjacent hunks (when using normal context, not -U1000000) - if !firstHunk { - lines = append(lines, DiffLine{ChangeType: ChangeDivider, Content: "..."}) - } - firstHunk = false + // Atoi("") returns 0 with error; regex guarantees m[2] is digits when non-empty. + // Both the omitted-length case (git spec: implicit 1) and the literal `,0` (insertion-only) + // end up at oldLen=0 here, and max(oldLen,1) below resolves both to the same advance. + oldLen, _ := strconv.Atoi(m[2]) + + // emit divider representing unchanged lines BEFORE this hunk. + // Leading divider (first hunk) uses prevOldEnd=1 initialization; between-hunks use + // prevOldEnd from prior iteration. Gap uses hunk-header metadata not oldNum, so + // insertion-only hunks (@@ -K,0 ...) compute correctly; oldNum stays put on `+` lines. + lines = appendGapDivider(lines, oldStart-prevOldEnd) + sawHunk = true + // prevOldEnd = line number AFTER the current hunk on the old side. Normal hunks + // (oldLen>0) cover [oldStart, oldStart+oldLen). Insertion-only hunks (oldLen==0, + // e.g. @@ -K,0 ...) insert between old lines K and K+1 — handled by max(oldLen,1). + prevOldEnd = oldStart + max(oldLen, 1) oldNum = oldStart newNum = newStart @@ -663,6 +751,14 @@ func parseUnifiedDiff(raw string) ([]DiffLine, error) { return nil, fmt.Errorf("scan diff: %w", err) } + // 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. sawHunk (not prevOldEnd > 1) is the correct "processed" flag + // — insertion-at-start hunks (@@ -0,0 ...) leave prevOldEnd at 1 but still count. + if totalOldLines > 0 && sawHunk { + lines = appendGapDivider(lines, totalOldLines-prevOldEnd+1) + } + return lines, nil } diff --git a/app/diff/diff_test.go b/app/diff/diff_test.go index d1613b86..469a4772 100644 --- a/app/diff/diff_test.go +++ b/app/diff/diff_test.go @@ -15,7 +15,7 @@ import ( func TestParseUnifiedDiff_SimpleAdd(t *testing.T) { raw := readFixture(t, "simple_add.diff") - lines, err := parseUnifiedDiff(raw) + lines, err := parseUnifiedDiff(raw, 0) require.NoError(t, err) // expected: context, blank, context, add, add, add, context, context @@ -57,7 +57,7 @@ func TestParseUnifiedDiff_SimpleAdd(t *testing.T) { func TestParseUnifiedDiff_SimpleRemove(t *testing.T) { raw := readFixture(t, "simple_remove.diff") - lines, err := parseUnifiedDiff(raw) + lines, err := parseUnifiedDiff(raw, 0) require.NoError(t, err) require.NotEmpty(t, lines, "expected non-empty result") @@ -78,17 +78,20 @@ func TestParseUnifiedDiff_SimpleRemove(t *testing.T) { func TestParseUnifiedDiff_MultiHunk(t *testing.T) { raw := readFixture(t, "multi_hunk.diff") - lines, err := parseUnifiedDiff(raw) + lines, err := parseUnifiedDiff(raw, 0) require.NoError(t, err) - // verify divider exists between hunks - var dividers int + // verify dividers carry line-count labels. + // fixture: hunk 1 at @@ -2,3, hunk 2 at @@ -10,3. + // - leading divider: line 1 precedes first hunk → 1 line. + // - between-hunks: prevOldEnd=5 after hunk 1, next at 10 → 5 lines skipped. + var dividers []string for _, l := range lines { if l.ChangeType == ChangeDivider { - dividers++ + dividers = append(dividers, l.Content) } } - assert.Equal(t, 1, dividers, "expected 1 divider between two hunks") + assert.Equal(t, []string{"⋯ 1 line ⋯", "⋯ 5 lines ⋯"}, dividers, "expected leading + between-hunks dividers") // verify additions in both hunks var additions []string @@ -100,9 +103,222 @@ func TestParseUnifiedDiff_MultiHunk(t *testing.T) { assert.Equal(t, []string{`import "os"`, " os.Exit(0)"}, additions) } +// TestParseUnifiedDiff_GapLabels exercises every gap-label branch through the +// real parser (not a helper in isolation). Covers: plural gaps, singular gap, +// omitted-length hunk headers (@@ -N +N @@), insertion-only hunks (@@ -K,0 ...), +// start-of-file insertion (@@ -0,0 ...), and three-hunk chains. +func TestParseUnifiedDiff_GapLabels(t *testing.T) { + tests := []struct { + name string + raw string + dividers []string + }{ + { + name: "plural gap, standard two-hunk", + raw: "--- a\n+++ b\n" + + "@@ -1,3 +1,3 @@\n a\n-b\n+B\n" + + "@@ -10,3 +10,3 @@\n x\n-y\n+Y\n", + dividers: []string{"⋯ 6 lines ⋯"}, + }, + { + name: "singular gap", + raw: "--- a\n+++ b\n" + + "@@ -1,3 +1,3 @@\n a\n-b\n+B\n" + + "@@ -5,3 +5,3 @@\n x\n-y\n+Y\n", + dividers: []string{"⋯ 1 line ⋯"}, + }, + { + name: "omitted length (implicit 1) on both sides", + raw: "--- a\n+++ b\n" + + "@@ -1 +1 @@\n-old\n+new\n" + + "@@ -5 +5 @@\n-old2\n+new2\n", + dividers: []string{"⋯ 3 lines ⋯"}, + }, + { + name: "insertion-only prior hunk (oldLen==0, mid-file)", + raw: "--- a\n+++ b\n" + + "@@ -5,0 +6,2 @@\n+x\n+y\n" + + "@@ -10,3 +12,3 @@\n p\n-q\n+Q\n", + // leading: lines 1..4 before first hunk = 4 lines. + // between: prior hunk inserts between old lines 5 and 6 (prevOldEnd=6), next at 10 → gap=4. + dividers: []string{"⋯ 4 lines ⋯", "⋯ 4 lines ⋯"}, + }, + { + name: "insertion-only prior hunk at start (@@ -0,0)", + raw: "--- a\n+++ b\n" + + "@@ -0,0 +1,2 @@\n+x\n+y\n" + + "@@ -5,3 +7,3 @@\n p\n-q\n+Q\n", + // prior hunk prepends; next untouched old line is 1. gap = 5 - 1 = 4. + dividers: []string{"⋯ 4 lines ⋯"}, + }, + { + name: "three-hunk chain, multiple dividers", + raw: "--- a\n+++ b\n" + + "@@ -1,2 +1,2 @@\n a\n-b\n+B\n" + + "@@ -5,2 +5,2 @@\n p\n-q\n+Q\n" + + "@@ -20,2 +20,2 @@\n x\n-y\n+Y\n", + dividers: []string{"⋯ 2 lines ⋯", "⋯ 13 lines ⋯"}, + }, + { + name: "leading divider when first hunk starts after line 1", + raw: "--- a\n+++ b\n" + + "@@ -48,3 +48,3 @@\n ctx\n-old\n+new\n", + // 47 unchanged lines precede the first hunk. + dividers: []string{"⋯ 47 lines ⋯"}, + }, + { + name: "no leading divider when first hunk starts at line 1", + raw: "--- a\n+++ b\n" + + "@@ -1,3 +1,3 @@\n ctx\n-old\n+new\n", + dividers: nil, + }, + { + name: "leading + between-hunks dividers", + raw: "--- a\n+++ b\n" + + "@@ -10,2 +10,2 @@\n a\n+b\n" + + "@@ -20,2 +20,2 @@\n c\n+d\n", + // 9 lines before first hunk; 8 lines between (prevOldEnd=12, next oldStart=20). + dividers: []string{"⋯ 9 lines ⋯", "⋯ 8 lines ⋯"}, + }, + { + name: "new file (first hunk @@ -0,0 ...) suppresses leading divider", + raw: "--- /dev/null\n+++ b\n" + + "@@ -0,0 +1,2 @@\n+x\n+y\n", + // oldStart=0, gap = 0-1 = -1, no divider. + dividers: nil, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + lines, err := parseUnifiedDiff(tc.raw, 0) + require.NoError(t, err) + + var got []string + for _, l := range lines { + if l.ChangeType == ChangeDivider { + got = append(got, l.Content) + } + } + assert.Equal(t, tc.dividers, got) + }) + } +} + +// TestParseUnifiedDiff_TrailingDivider covers trailing divider emission driven +// by the totalOldLines parameter — the caller-supplied total line count of the +// pre-change file. 0 (unknown) skips the trailing divider; positive values emit +// a divider only when the last hunk does not reach EOF. +func TestParseUnifiedDiff_TrailingDivider(t *testing.T) { + tests := []struct { + name string + raw string + totalOldLines int + wantDividers []string + }{ + { + name: "trailing plural — hunk ends at line 10, total 300", + raw: "--- a\n+++ b\n" + + "@@ -8,3 +8,3 @@\n a\n-b\n+B\n", + // prevOldEnd=11, trailing = 300-11+1 = 290. + totalOldLines: 300, + wantDividers: []string{"⋯ 7 lines ⋯", "⋯ 290 lines ⋯"}, + }, + { + name: "trailing singular — exactly one line after last hunk", + raw: "--- a\n+++ b\n" + + "@@ -1,3 +1,3 @@\n a\n-b\n+B\n", + // prevOldEnd=4, totalOldLines=4, gap = 4-4+1 = 1 → singular label + totalOldLines: 4, + wantDividers: []string{"⋯ 1 line ⋯"}, + }, + { + name: "no trailing — last hunk covers to EOF", + raw: "--- a\n+++ b\n" + + "@@ -1,3 +1,3 @@\n a\n-b\n+B\n", + // prevOldEnd=4, total=3, 4>3 → no trailing. + totalOldLines: 3, + wantDividers: nil, + }, + { + name: "totalOldLines=0 → no trailing (unknown)", + raw: "--- a\n+++ b\n" + + "@@ -1,3 +1,3 @@\n a\n-b\n+B\n", + totalOldLines: 0, + wantDividers: nil, + }, + { + name: "leading + between + trailing", + raw: "--- a\n+++ b\n" + + "@@ -5,2 +5,2 @@\n a\n+b\n" + + "@@ -15,2 +15,2 @@\n c\n+d\n", + // leading: 4 lines. between: prevOldEnd=7, next=15 → 8. trailing: prevOldEnd=17, total=50 → 34. + totalOldLines: 50, + wantDividers: []string{"⋯ 4 lines ⋯", "⋯ 8 lines ⋯", "⋯ 34 lines ⋯"}, + }, + { + name: "empty diff with totalOldLines set emits nothing (no hunks)", + raw: "", + // no hunks processed → prevOldEnd stays 1 → trailing skipped by guard. + totalOldLines: 100, + wantDividers: nil, + }, + { + name: "insertion-only LAST hunk computes trailing from prevOldEnd=K+1", + raw: "--- a\n+++ b\n" + + "@@ -1,1 +1,1 @@\n-a\n+A\n" + + "@@ -10,0 +11,2 @@\n+x\n+y\n", + // between hunks: prevOldEnd after hunk 1 = 2, next oldStart = 10 → gap = 8. + // last hunk is insertion-only: prevOldEnd = 10 + max(0,1) = 11. + // trailing: totalOldLines=20, 20 - 11 + 1 = 10. + totalOldLines: 20, + wantDividers: []string{"⋯ 8 lines ⋯", "⋯ 10 lines ⋯"}, + }, + { + name: "deleted file (@@ -1,N +0,0 @@) — last hunk covers EOF, no trailing", + raw: "--- a\n+++ /dev/null\n" + + "@@ -1,3 +0,0 @@\n-a\n-b\n-c\n", + // prevOldEnd = 1 + 3 = 4 = totalOldLines + 1, so gap = 3-4+1 = 0 → no divider. + totalOldLines: 3, + wantDividers: nil, + }, + { + name: "exact boundary prevOldEnd == totalOldLines + 1 emits no trailing", + raw: "--- a\n+++ b\n" + + "@@ -5,3 +5,3 @@\n a\n-b\n+B\n", + // leading: 4 lines (5-1). prevOldEnd = 8. totalOldLines = 7 → 7-8+1 = 0 → no trailing. + totalOldLines: 7, + wantDividers: []string{"⋯ 4 lines ⋯"}, + }, + { + name: "insertion-at-start hunk (@@ -0,0) with totalOldLines>0 emits trailing", + raw: "--- a\n+++ b\n" + + "@@ -0,0 +1,2 @@\n+x\n+y\n", + // hypothetical/malformed: prevOldEnd stays at 1 after the hunk, + // but sawHunk is true, so the trailing guard still fires. + // gap = totalOldLines - prevOldEnd + 1 = 10 - 1 + 1 = 10. + totalOldLines: 10, + wantDividers: []string{"⋯ 10 lines ⋯"}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + lines, err := parseUnifiedDiff(tc.raw, tc.totalOldLines) + require.NoError(t, err) + + var got []string + for _, l := range lines { + if l.ChangeType == ChangeDivider { + got = append(got, l.Content) + } + } + assert.Equal(t, tc.wantDividers, got) + }) + } +} + func TestParseUnifiedDiff_MixedChanges(t *testing.T) { raw := readFixture(t, "mixed_changes.diff") - lines, err := parseUnifiedDiff(raw) + lines, err := parseUnifiedDiff(raw, 0) require.NoError(t, err) types := make([]ChangeType, 0, len(lines)) @@ -126,14 +342,14 @@ func TestParseUnifiedDiff_MixedChanges(t *testing.T) { } func TestParseUnifiedDiff_Empty(t *testing.T) { - lines, err := parseUnifiedDiff("") + lines, err := parseUnifiedDiff("", 0) require.NoError(t, err) assert.Empty(t, lines) } func TestParseUnifiedDiff_Binary(t *testing.T) { raw := readFixture(t, "binary.diff") - lines, err := parseUnifiedDiff(raw) + lines, err := parseUnifiedDiff(raw, 0) require.NoError(t, err) require.Len(t, lines, 1) assert.Equal(t, BinaryPlaceholder, lines[0].Content) @@ -145,7 +361,7 @@ func TestParseUnifiedDiff_Binary(t *testing.T) { func TestParseUnifiedDiff_BinaryNewFile(t *testing.T) { raw := "diff --git a/new.bin b/new.bin\nnew file mode 100644\nindex 0000000..dd12d3a\nBinary files /dev/null and b/new.bin differ\n" - lines, err := parseUnifiedDiff(raw) + lines, err := parseUnifiedDiff(raw, 0) require.NoError(t, err) require.Len(t, lines, 1) assert.Equal(t, "(new binary file)", lines[0].Content) @@ -154,7 +370,7 @@ func TestParseUnifiedDiff_BinaryNewFile(t *testing.T) { func TestParseUnifiedDiff_BinaryDeleted(t *testing.T) { raw := "diff --git a/old.bin b/old.bin\ndeleted file mode 100644\nindex 2dfe7e4..0000000\nBinary files a/old.bin and /dev/null differ\n" - lines, err := parseUnifiedDiff(raw) + lines, err := parseUnifiedDiff(raw, 0) require.NoError(t, err) require.Len(t, lines, 1) assert.Equal(t, "(deleted binary file)", lines[0].Content) @@ -163,7 +379,7 @@ func TestParseUnifiedDiff_BinaryDeleted(t *testing.T) { func TestParseUnifiedDiff_LineNumbers(t *testing.T) { raw := readFixture(t, "simple_add.diff") - lines, err := parseUnifiedDiff(raw) + lines, err := parseUnifiedDiff(raw, 0) require.NoError(t, err) // additions should have OldNum=0 @@ -185,7 +401,7 @@ func TestParseUnifiedDiff_LineNumbers(t *testing.T) { func TestParseUnifiedDiff_RemoveLineNumbers(t *testing.T) { raw := readFixture(t, "simple_remove.diff") - lines, err := parseUnifiedDiff(raw) + lines, err := parseUnifiedDiff(raw, 0) require.NoError(t, err) for _, l := range lines { @@ -306,7 +522,7 @@ func TestParseUnifiedDiff_LongLine(t *testing.T) { longContent := strings.Repeat("x", 100_000) raw := "diff --git a/big.js b/big.js\n--- a/big.js\n+++ b/big.js\n@@ -1,1 +1,2 @@\n context\n+" + longContent + "\n" - lines, err := parseUnifiedDiff(raw) + lines, err := parseUnifiedDiff(raw, 0) require.NoError(t, err, "should handle lines up to 1MB without error") var hasAdd bool @@ -897,6 +1113,58 @@ func gitCmd(t *testing.T, dir string, args ...string) { require.NoError(t, err, "git %v failed: %s", args, string(out)) } +func TestGit_TotalOldLines(t *testing.T) { + dir := setupTestRepo(t) + // commit A: 5 lines + writeFile(t, dir, "f.txt", "a\nb\nc\nd\ne\n") + gitCmd(t, dir, "add", "f.txt") + gitCmd(t, dir, "commit", "-m", "A") + commitA, err := exec.Command("git", "-C", dir, "rev-parse", "HEAD").Output() //nolint:gosec // test-controlled args + require.NoError(t, err) + refA := strings.TrimSpace(string(commitA)) + + // commit B: 3 lines (same file, modified) + writeFile(t, dir, "f.txt", "a\nB\nC\n") + gitCmd(t, dir, "add", "f.txt") + gitCmd(t, dir, "commit", "-m", "B") + + // working tree: 7 lines + writeFile(t, dir, "f.txt", "a\nB\nC\nd\ne\nf\ng\n") + + // staged version (10 lines) + writeFile(t, dir, "staged.txt", strings.Repeat("x\n", 10)) + gitCmd(t, dir, "add", "staged.txt") + + // file without trailing newline + writeFile(t, dir, "notrail.txt", "line1\nline2") + gitCmd(t, dir, "add", "notrail.txt") + gitCmd(t, dir, "commit", "-m", "notrail") + + g := NewGit(dir) + + tests := []struct { + name string + ref string + file string + staged bool + want int + }{ + {"HEAD commit B — 3 lines", "HEAD", "f.txt", false, 3}, + {"commit A — 5 lines via single ref", refA, "f.txt", false, 5}, + {"range A..HEAD — left operand A, 5 lines", refA + "..HEAD", "f.txt", false, 5}, + {"triple-dot A...HEAD — takes left operand A, 5 lines", refA + "...HEAD", "f.txt", false, 5}, + {"empty ref + staged → HEAD, 3 lines", "", "f.txt", true, 3}, + {"non-existent ref → 0", "nonexistent-ref-xyz", "f.txt", false, 0}, + {"non-existent file → 0", "HEAD", "missing.txt", false, 0}, + {"file without trailing newline counts final line", "HEAD", "notrail.txt", false, 2}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, g.totalOldLines(tc.ref, tc.file, tc.staged)) + }) + } +} + func TestMatchesPrefix(t *testing.T) { prefixes := []string{"src", "pkg/util"} diff --git a/app/diff/hg.go b/app/diff/hg.go index 73265973..19ad6c7d 100644 --- a/app/diff/hg.go +++ b/app/diff/hg.go @@ -159,7 +159,43 @@ func (h *Hg) FileDiff(ref, file string, _ bool, contextLines int) ([]DiffLine, e return nil, fmt.Errorf("get file diff for %s: %w", file, err) } - return parseUnifiedDiff(out) + // trailing divider only matters in compact mode; skip the probe in full-file mode. + total := 0 + if contextLines > 0 && contextLines < fullContextSentinel { + total = h.totalOldLines(ref, file) + } + return parseUnifiedDiff(out, total) +} + +// totalOldLines returns the line count of the pre-change version of file, used by +// parseUnifiedDiff to emit a trailing divider. Returns 0 when the old-side file is +// unavailable — the parser treats 0 as "unknown" and skips the trailing divider. +// +// Old-side resolution (mirrors revFlag): +// - ref empty → "." (working-copy parent) +// - ref contains ".." or "..." → left operand (triple-dot checked first so A...B +// is not mis-split on the leading "..") +// - single ref → use as-is +// +// For triple-dot ranges the left operand is an approximation of the true old side +// (ancestor(A,B)); accurate enough for the informational trailing-divider count. +func (h *Hg) totalOldLines(ref, file string) int { + oldRef := ref + if left, _, ok := strings.Cut(ref, "..."); ok { + oldRef = left + } + if left, _, ok := strings.Cut(oldRef, ".."); ok { + oldRef = left + } + oldRef = translateRef(oldRef) + if oldRef == "" { + oldRef = "." + } + out, err := h.runHg("cat", "-r", oldRef, "--", file) + if err != nil { + return 0 + } + return countLines(out) } // hgContextArg returns the -U argument for hg diff given the caller's requested diff --git a/app/diff/jj.go b/app/diff/jj.go index f4ad11b8..6d19540f 100644 --- a/app/diff/jj.go +++ b/app/diff/jj.go @@ -156,7 +156,45 @@ func (j *Jj) FileDiff(ref, file string, _ bool, contextLines int) ([]DiffLine, e // jj emits raw bytes for binary files instead of git's "Binary files … differ" // marker. Detect and rewrite so parseUnifiedDiff produces the binary placeholder. normalized := j.synthesizeBinaryDiff(out) - return parseUnifiedDiff(normalized) + + // trailing divider only matters in compact mode; skip the probe in full-file mode. + total := 0 + if contextLines > 0 && contextLines < fullContextSentinel { + total = j.totalOldLines(ref, file) + } + return parseUnifiedDiff(normalized, total) +} + +// totalOldLines returns the line count of the pre-change version of file, used by +// parseUnifiedDiff to emit a trailing divider. Returns 0 when the old-side file is +// unavailable — the parser treats 0 as "unknown" and skips the trailing divider. +// +// Old-side resolution (mirrors diffRangeFlags): +// - ref empty → "@-" (parent of the working-copy commit) +// - ref contains ".." or "..." → left operand (triple-dot checked first so A...B +// is not mis-split on the leading "..") +// - single ref → use as-is +// +// For triple-dot ranges the left operand is an approximation of the true old side +// (the jj revset ancestors(A) & ancestors(B)); accurate enough for the informational +// trailing-divider count. +func (j *Jj) totalOldLines(ref, file string) int { + oldRef := ref + if left, _, ok := strings.Cut(ref, "..."); ok { + oldRef = left + } + if left, _, ok := strings.Cut(oldRef, ".."); ok { + oldRef = left + } + oldRef = j.translateRef(oldRef) + if oldRef == "" { + oldRef = "@-" + } + out, err := j.runJj("file", "show", "-r", oldRef, "--", file) + if err != nil { + return 0 + } + return countLines(out) } // jjContextArg returns the --context argument for jj diff given the caller's