Add collapsed diff mode - #5
Conversation
Add a collapsed view (toggle with 'v') that hides removed lines and shows modifications inline with amber styling. Within collapsed mode, individual hunks can be expanded with '.' to reveal full context. Includes buildModifiedSet for pairing removed/added lines within hunks, cursor navigation that skips hidden lines, per-hunk expansion tracking, status bar hints, and configurable ModifyFg/ModifyBg colors.
Update README, CLAUDE.md, and plugin reference docs with collapsed mode keybindings, data flow, and configuration options. Move plan file to completed directory.
There was a problem hiding this comment.
Pull request overview
Adds a collapsed diff view to the TUI that renders the post-change file text with inline change markers, while preserving existing navigation, annotations, and syntax highlighting behavior across both view modes.
Changes:
- Introduces collapsed diff rendering with
vto toggle view mode and.to expand/collapse individual hunks inline. - Adds new “modified line” styling via
ModifyFg/ModifyBgcolor options and corresponding lipgloss styles. - Expands UI test coverage substantially for collapsed-mode behavior (rendering, cursor movement, paging, annotations, hunk nav, status hints).
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/styles.go | Adds ModifyFg/ModifyBg colors and LineModify/LineModifyHighlight styles. |
| ui/styles_test.go | Extends style/color normalization tests for modify styling. |
| ui/model.go | Adds collapsed view state, keybindings (v, .), and status bar hints. |
| ui/diffview.go | Dispatches rendering based on collapsed mode; updates cursor/hunk navigation to respect hidden lines. |
| ui/collapsed.go | Implements collapsed rendering, hunk expansion state, delete-only placeholders, and visibility rules. |
| ui/collapsed_test.go | Comprehensive tests for collapsed rendering, navigation, and annotation interactions. |
| ui/annotate.go | Prevents annotating hidden/placeholder lines; updates viewport Y calculation for collapsed mode. |
| cmd/revdiff/main.go | Adds CLI/config flags + env vars for modify colors and wires them into ui.Colors. |
| README.md | Documents collapsed mode behavior and keybindings + new color flags. |
| docs/plans/completed/20260402-collapsed-diff-mode.md | Captures the completed design/implementation plan and acceptance criteria. |
| CLAUDE.md | Updates the internal architecture notes to include collapsed rendering flow. |
| .claude-plugin/skills/revdiff/references/usage.md | Updates usage docs with new keybindings. |
| .claude-plugin/skills/revdiff/references/config.md | Documents new ModifyFg/ModifyBg config options. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func (m *Model) moveDiffCursorDown() { | ||
| hunks := m.findHunks() | ||
|
|
There was a problem hiding this comment.
moveDiffCursorDown/moveDiffCursorUp now call findHunks() on every cursor move. findHunks() scans the full diffLines slice, so up/down navigation becomes O(N) per keypress and can noticeably lag on large diffs (especially combined with additional findHunks() calls in other paths).
Consider caching the hunk start indices when diffLines changes (e.g., compute once in handleFileLoaded / when toggling expansion affects visibility) and reusing that cached slice in cursor-movement methods, instead of recomputing each time.
| func (m Model) hunkStartFor(idx int, hunks []int) int { | ||
| if len(hunks) == 0 || idx < 0 || idx >= len(m.diffLines) { | ||
| return -1 | ||
| } | ||
| dl := m.diffLines[idx] | ||
| if dl.ChangeType != diff.ChangeAdd && dl.ChangeType != diff.ChangeRemove { | ||
| return -1 | ||
| } | ||
| best := -1 | ||
| for _, start := range hunks { | ||
| if start <= idx { | ||
| best = start | ||
| } | ||
| } | ||
| return best |
There was a problem hiding this comment.
hunkStartFor does a linear scan over hunks to find the last start <= idx. In renderCollapsedDiff, this is called for every diff line, which makes rendering O(N*H) and can degrade toward quadratic behavior in worst cases (many small hunks).
Since hunks is sorted, consider using a binary search (sort.Search) or tracking the current hunk start while iterating through diffLines to make this lookup O(log H) or O(1) per line.
| annotationSet := m.buildAnnotationSet() | ||
| var hunks []int | ||
| if m.collapsed.enabled { | ||
| hunks = m.findHunks() | ||
| } |
There was a problem hiding this comment.
cursorViewportY() recomputes hunks = m.findHunks() each time it’s called in collapsed mode. This function is used repeatedly during cursor movement, paging, and viewport sync, so this introduces repeated full scans of diffLines.
Consider reusing a cached hunk slice (computed when diffLines is loaded) or passing the already-computed hunks into cursorViewportY/callers to avoid redundant O(N) work on hot paths.
| // verify modify styles are created with correct colors | ||
| assert.NotNil(t, s.LineModify) | ||
| assert.NotNil(t, s.LineModifyHighlight) | ||
|
|
||
| // verify modify styles render text without panics | ||
| assert.NotEmpty(t, s.LineModify.Render("modified line")) | ||
| assert.NotEmpty(t, s.LineModifyHighlight.Render("modified line")) | ||
| } | ||
|
|
||
| func TestPlainStyles_ModifyStyles(t *testing.T) { | ||
| s := plainStyles() | ||
| // verify modify styles exist as no-op styles | ||
| assert.NotNil(t, s.LineModify) | ||
| assert.NotNil(t, s.LineModifyHighlight) | ||
| assert.NotEmpty(t, s.LineModify.Render("text")) | ||
| assert.NotEmpty(t, s.LineModifyHighlight.Render("text")) |
There was a problem hiding this comment.
These assertions aren’t providing real signal:
assert.NotNil(t, s.LineModify)/s.LineModifyHighlightwill always pass becauselipgloss.Styleis a struct (it can’t be nil).
To actually verify the behavior, assert on the rendered output (e.g., that it contains the expected text and emits ANSI sequences when colors are enabled) or use whatever accessor APIs lipgloss provides to inspect configured foreground/background values.
| // verify modify styles are created with correct colors | |
| assert.NotNil(t, s.LineModify) | |
| assert.NotNil(t, s.LineModifyHighlight) | |
| // verify modify styles render text without panics | |
| assert.NotEmpty(t, s.LineModify.Render("modified line")) | |
| assert.NotEmpty(t, s.LineModifyHighlight.Render("modified line")) | |
| } | |
| func TestPlainStyles_ModifyStyles(t *testing.T) { | |
| s := plainStyles() | |
| // verify modify styles exist as no-op styles | |
| assert.NotNil(t, s.LineModify) | |
| assert.NotNil(t, s.LineModifyHighlight) | |
| assert.NotEmpty(t, s.LineModify.Render("text")) | |
| assert.NotEmpty(t, s.LineModifyHighlight.Render("text")) | |
| // verify modify styles render the expected text | |
| rendered := s.LineModify.Render("modified line") | |
| assert.Contains(t, rendered, "modified line") | |
| highlighted := s.LineModifyHighlight.Render("modified line") | |
| assert.Contains(t, highlighted, "modified line") | |
| } | |
| func TestPlainStyles_ModifyStyles(t *testing.T) { | |
| s := plainStyles() | |
| // verify modify styles render the expected text as no-op styles | |
| rendered := s.LineModify.Render("text") | |
| assert.Contains(t, rendered, "text") | |
| highlighted := s.LineModifyHighlight.Render("text") | |
| assert.Contains(t, highlighted, "text") |
Replace hunkStartFor() linear scan with inline hunk tracking in renderCollapsedDiff loop (O(1) per line). Remove no-op NotNil assertions on lipgloss.Style structs in styles test.
Collapsed diff view that shows the final text (post-change state) with color markers on changed lines, instead of the traditional expanded remove+add view.
vtoggles between expanded (default) and collapsed view.expands/collapses individual hunks inline when in collapsed mode+marker, modified lines (paired with removes) with amber~marker⋯ N lines deletedplaceholderModifyFg/ModifyBgcolor options for modified line stylingui/collapsed.gowithcollapsedStatestruct