Skip to content

diff: scroll wrapped display rows, not source lines - #667

Closed
andyrewlee wants to merge 1 commit into
advisor/053-replace-executable-without-path-gapfrom
advisor/054-diff-visual-rows
Closed

andyrewlee wants to merge 1 commit into
advisor/053-replace-executable-without-path-gapfrom
advisor/054-diff-visual-rows

Conversation

@andyrewlee

Copy link
Copy Markdown
Owner

Summary

  • Replaces source-line scroll math with a cached visual-row layout (internal/ui/diff/visual_rows.go): every wrapped segment is a scrollable row with a source-line→first-row lookup for hunk navigation
  • Fixes the unreachable-tail bug: a single wrapped line could fill several screens while maxScroll reported zero — wheel/keys now reach the end, and the footer counts display rows
  • Wrap toggles and width resizes re-anchor the top of the viewport to the same source line (segment clamped); height-only resizes preserve the offset
  • Chrome tiers bound the frame exactly: height ≥3 = header+stats+footer, 2 = header+footer, 1 = header, ≤0 = empty; every emitted row is ANSI-clipped to the viewport width
  • copyFile-style pre-commit seams n/a; scroll never rebuilds the wrap layout (proven by rowsBuilt counter in TestVisualRowsCache)

Test plan

  • go test ./internal/ui/diff ./internal/ui/center incl. new TestVisualRows* groups + real-git-diff center integration (TestDiffTabWrappedTailReachable)
  • go test -race on both packages, make harness-presets, verify-loop, lint-strict-new, devcheck (one ambient TempDir RemoveAll teardown flake in e2e — assertions green)
  • [!] PERF_STRICT=1 make perf-check: not evaluable on this host — sustained load ~25 inflated sidebar/monitor p95 tails on the baseline commit too; those presets never instantiate the diff viewer (p50s flat)

Stacked on #666 (plan 053).

The diff viewer computed scroll bounds from logical lines but rendered
wrapped display rows, so a long wrapped line's tail was unreachable.
The model now keeps a cached visual-row layout keyed on (diff, width,
wrap, styles): scrolling, wheel gating, footer position, and hunk jumps
all operate in that space, wrap toggles and width changes re-anchor to
the same source line, and chrome tiers bound the emitted frame. A
scroll limit can no longer strand wrapped tails.

PERF_STRICT=1 make perf-check could not be evaluated on this host:
sustained load (~25) inflates the tail percentiles for the sidebar and
monitor presets, which never instantiate the diff viewer; the baseline
commit passed all three presets on the same host minutes earlier.
@andyrewlee

Copy link
Copy Markdown
Owner Author

Superseded — content landed on main via merge 8580442 (plans/ scaffolding intentionally excluded).

@andyrewlee andyrewlee closed this Sep 28, 2026
@andyrewlee
andyrewlee deleted the advisor/054-diff-visual-rows branch September 28, 2026 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant