From 791c0cc7e0182ba5be66652d005651d6ffadf720 Mon Sep 17 00:00:00 2001 From: Umputun Date: Thu, 2 Apr 2026 16:37:10 -0500 Subject: [PATCH 1/8] feat: compute and cache file diff stats (adds/removes) --- ui/model.go | 17 +++++++++++++ ui/model_test.go | 63 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/ui/model.go b/ui/model.go index 40eafe07..30df8906 100644 --- a/ui/model.go +++ b/ui/model.go @@ -70,6 +70,9 @@ type Model struct { collapsed collapsedState // collapsed diff view state + fileAdds int // cached count of added lines in current file + fileRemoves int // cached count of removed lines in current file + discarded bool // true when user chose to discard annotations and quit inConfirmDiscard bool // true when showing discard confirmation prompt noConfirmDiscard bool // skip confirmation prompt on discard quit @@ -413,6 +416,7 @@ func (m Model) handleFileLoaded(msg fileLoadedMsg) (tea.Model, tea.Cmd) { } m.currFile = msg.file m.diffLines = msg.lines + m.computeFileStats() m.highlightedLines = m.highlighter.HighlightLines(msg.file, msg.lines) m.cursorOnAnnotation = false m.scrollX = 0 @@ -423,6 +427,19 @@ func (m Model) handleFileLoaded(msg fileLoadedMsg) (tea.Model, tea.Cmd) { return m, nil } +// computeFileStats counts added and removed lines in the current diffLines. +func (m *Model) computeFileStats() { + m.fileAdds, m.fileRemoves = 0, 0 + for _, dl := range m.diffLines { + switch dl.ChangeType { + case diff.ChangeAdd: + m.fileAdds++ + case diff.ChangeRemove: + m.fileRemoves++ + } + } +} + // skipInitialDividers positions diffCursor on the first visible line. // skips divider lines, and in collapsed mode also skips removed lines // unless their hunk is expanded. diff --git a/ui/model_test.go b/ui/model_test.go index f7d2122d..f169cdc9 100644 --- a/ui/model_test.go +++ b/ui/model_test.go @@ -91,6 +91,69 @@ func TestModel_FileLoaded(t *testing.T) { assert.Len(t, model.diffLines, 2) } +func TestModel_ComputeFileStats(t *testing.T) { + tests := []struct { + name string + lines []diff.DiffLine + adds int + removes int + }{ + {name: "empty diff", lines: nil, adds: 0, removes: 0}, + {name: "context only", lines: []diff.DiffLine{ + {NewNum: 1, Content: "package main", ChangeType: diff.ChangeContext}, + {NewNum: 2, Content: "// comment", ChangeType: diff.ChangeContext}, + }, adds: 0, removes: 0}, + {name: "adds only", lines: []diff.DiffLine{ + {NewNum: 1, Content: "line1", ChangeType: diff.ChangeAdd}, + {NewNum: 2, Content: "line2", ChangeType: diff.ChangeAdd}, + {NewNum: 3, Content: "line3", ChangeType: diff.ChangeAdd}, + }, adds: 3, removes: 0}, + {name: "removes only", lines: []diff.DiffLine{ + {OldNum: 1, Content: "old1", ChangeType: diff.ChangeRemove}, + {OldNum: 2, Content: "old2", ChangeType: diff.ChangeRemove}, + }, adds: 0, removes: 2}, + {name: "mixed changes", lines: []diff.DiffLine{ + {NewNum: 1, Content: "package main", ChangeType: diff.ChangeContext}, + {OldNum: 2, Content: "old func", ChangeType: diff.ChangeRemove}, + {NewNum: 2, Content: "new func", ChangeType: diff.ChangeAdd}, + {NewNum: 3, Content: "// ok", ChangeType: diff.ChangeContext}, + {Content: "", ChangeType: diff.ChangeDivider}, + {NewNum: 10, Content: "added line", ChangeType: diff.ChangeAdd}, + }, adds: 2, removes: 1}, + {name: "dividers ignored", lines: []diff.DiffLine{ + {Content: "", ChangeType: diff.ChangeDivider}, + {Content: "", ChangeType: diff.ChangeDivider}, + }, adds: 0, removes: 0}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := testModel(nil, nil) + m.diffLines = tt.lines + m.computeFileStats() + assert.Equal(t, tt.adds, m.fileAdds, "fileAdds") + assert.Equal(t, tt.removes, m.fileRemoves, "fileRemoves") + }) + } +} + +func TestModel_FileLoadedComputesStats(t *testing.T) { + lines := []diff.DiffLine{ + {NewNum: 1, Content: "package main", ChangeType: diff.ChangeContext}, + {OldNum: 2, Content: "removed", ChangeType: diff.ChangeRemove}, + {NewNum: 2, Content: "added1", ChangeType: diff.ChangeAdd}, + {NewNum: 3, Content: "added2", ChangeType: diff.ChangeAdd}, + } + m := testModel([]string{"a.go"}, map[string][]diff.DiffLine{"a.go": lines}) + m.tree = newFileTree([]string{"a.go"}) + m.loadSeq = 1 + + result, _ := m.Update(fileLoadedMsg{file: "a.go", seq: 1, lines: lines}) + model := result.(Model) + assert.Equal(t, 2, model.fileAdds) + assert.Equal(t, 1, model.fileRemoves) +} + func TestModel_QuitKey(t *testing.T) { m := testModel([]string{"a.go"}, nil) From 1400b5b0408157338b879d21b8c62f7c94fff856 Mon Sep 17 00:00:00 2001 From: Umputun Date: Thu, 2 Apr 2026 16:44:40 -0500 Subject: [PATCH 2/8] feat: rewrite status line with filename, stats, mode icons, and help hint --- ui/collapsed_test.go | 63 +++-------- ui/model.go | 136 +++++++++++++++-------- ui/model_test.go | 256 +++++++++++++++++++++++++++---------------- 3 files changed, 270 insertions(+), 185 deletions(-) diff --git a/ui/collapsed_test.go b/ui/collapsed_test.go index 5acf8b64..1f6220c9 100644 --- a/ui/collapsed_test.go +++ b/ui/collapsed_test.go @@ -1076,7 +1076,7 @@ func TestModel_CollapsedPageUpSkipsRemovedLines(t *testing.T) { assert.NotEqual(t, diff.ChangeRemove, dl.ChangeType, "cursor should not land on hidden removed line") } -func TestModel_StatusBarViewModeHint(t *testing.T) { +func TestModel_StatusBarCollapsedIndicator(t *testing.T) { lines := []diff.DiffLine{ {NewNum: 1, Content: "ctx", ChangeType: diff.ChangeContext}, {NewNum: 2, Content: "add", ChangeType: diff.ChangeAdd}, @@ -1087,29 +1087,21 @@ func TestModel_StatusBarViewModeHint(t *testing.T) { m.focus = paneDiff m.width = 200 - t.Run("expanded mode shows collapse hint", func(t *testing.T) { + t.Run("expanded mode has no collapsed indicator", func(t *testing.T) { m.collapsed.enabled = false - status := m.statusBarText(m.annotatedFiles()) - assert.Contains(t, status, "[v] collapse") - assert.NotContains(t, status, "[v] expand") + status := m.statusBarText() + assert.NotContains(t, status, "▼") }) - t.Run("collapsed mode shows expand hint", func(t *testing.T) { + t.Run("collapsed mode shows indicator", func(t *testing.T) { m.collapsed.enabled = true m.collapsed.expandedHunks = make(map[int]bool) - status := m.statusBarText(m.annotatedFiles()) - assert.Contains(t, status, "[v] expand") - assert.NotContains(t, status, "[v] collapse") - }) - - t.Run("tree pane does not show view mode hint", func(t *testing.T) { - m.focus = paneTree - status := m.statusBarText(m.annotatedFiles()) - assert.NotContains(t, status, "[v]") + status := m.statusBarText() + assert.Contains(t, status, "▼") }) } -func TestModel_StatusBarDotHint(t *testing.T) { +func TestModel_StatusBarNoShortcutHints(t *testing.T) { lines := []diff.DiffLine{ {NewNum: 1, Content: "ctx", ChangeType: diff.ChangeContext}, {OldNum: 2, Content: "removed", ChangeType: diff.ChangeRemove}, @@ -1121,39 +1113,14 @@ func TestModel_StatusBarDotHint(t *testing.T) { m.currFile = "a.go" m.focus = paneDiff m.width = 200 + m.collapsed.enabled = true + m.collapsed.expandedHunks = make(map[int]bool) + m.diffCursor = 2 - t.Run("collapsed mode on hunk shows expand hunk hint", func(t *testing.T) { - m.collapsed.enabled = true - m.collapsed.expandedHunks = make(map[int]bool) - m.diffCursor = 2 // on add line in hunk - status := m.statusBarText(m.annotatedFiles()) - assert.Contains(t, status, "[.] expand hunk") - assert.NotContains(t, status, "[.] collapse hunk") - }) - - t.Run("collapsed mode on expanded hunk shows collapse hunk hint", func(t *testing.T) { - m.collapsed.enabled = true - m.collapsed.expandedHunks = map[int]bool{1: true} // hunk starts at index 1 - m.diffCursor = 2 // on add line in expanded hunk - status := m.statusBarText(m.annotatedFiles()) - assert.Contains(t, status, "[.] collapse hunk") - assert.NotContains(t, status, "[.] expand hunk") - }) - - t.Run("collapsed mode on context line hides dot hint", func(t *testing.T) { - m.collapsed.enabled = true - m.collapsed.expandedHunks = make(map[int]bool) - m.diffCursor = 0 // on context line - status := m.statusBarText(m.annotatedFiles()) - assert.NotContains(t, status, "[.]") - }) - - t.Run("expanded mode hides dot hint", func(t *testing.T) { - m.collapsed.enabled = false - m.diffCursor = 2 // on changed line, but not collapsed - status := m.statusBarText(m.annotatedFiles()) - assert.NotContains(t, status, "[.]") - }) + status := m.statusBarText() + // shortcut hints are moved to help overlay, not in status line + assert.NotContains(t, status, "[.]") + assert.NotContains(t, status, "[v]") } func TestModel_CollapsedCursorToEndSkipsRemovedLines(t *testing.T) { diff --git a/ui/model.go b/ui/model.go index 30df8906..69240e52 100644 --- a/ui/model.go +++ b/ui/model.go @@ -436,6 +436,8 @@ func (m *Model) computeFileStats() { m.fileAdds++ case diff.ChangeRemove: m.fileRemoves++ + case diff.ChangeContext, diff.ChangeDivider: + // not counted in stats } } } @@ -498,12 +500,13 @@ func (m Model) View() string { return mainView } - status := m.styles.StatusBar.Width(m.width).Render(m.statusBarText(annotated)) + status := m.styles.StatusBar.Width(m.width).Render(m.statusBarText()) return lipgloss.JoinVertical(lipgloss.Left, mainView, status) } -// statusBarText returns context-sensitive status bar hints. -func (m Model) statusBarText(annotated map[string]bool) string { +// statusBarText returns context-sensitive status line content. +// shows filename, diff stats, hunk position, mode indicators, and right-aligned annotation count + help hint. +func (m Model) statusBarText() string { if m.inConfirmDiscard { return fmt.Sprintf("discard %d annotations? [y/n]", m.store.Count()) } @@ -512,59 +515,102 @@ func (m Model) statusBarText(annotated map[string]bool) string { return "[enter] save [esc] cancel" } - filterHint := "" - if len(annotated) > 0 { - filterHint = " [f] filter" - } - fileNoteHint := "" + // build left-side segments + var segments []string + + // filename segment if m.currFile != "" { - fileNoteHint = " [A] file note" + segments = append(segments, m.currFile) } - annotationCount := m.store.Count() - countHint := "" - if annotationCount > 0 { - countHint = fmt.Sprintf(" %d annotations", annotationCount) + // diff stats segment + if m.currFile != "" { + segments = append(segments, fmt.Sprintf("+%d/-%d", m.fileAdds, m.fileRemoves)) } - var hints string - switch m.focus { - case paneTree: - hints = "[j/k] navigate [enter] select [l/tab] diff" + filterHint + " [n/p] next/prev [Q] discard [q] quit" - case paneDiff: - deleteHint := "" - if m.cursorLineHasAnnotation() { - deleteHint = " [d] delete" - } - hunkHint := "" - if cur, total := m.currentHunk(); total > 0 { - hunkHint = fmt.Sprintf(" [ ] hunk %d/%d", cur, total) - } - viewModeHint := " [v] collapse" - if m.collapsed.enabled { - viewModeHint = " [v] expand" + // hunk position (only when cursor is on a changed line in diff pane) + if m.focus == paneDiff { + if cur, total := m.currentHunk(); total > 0 && cur > 0 { + segments = append(segments, fmt.Sprintf("hunk %d/%d", cur, total)) } - dotHint := "" - if m.collapsed.enabled { - if hs, ok := m.cursorHunkStart(); ok && m.collapsed.expandedHunks[hs] { - dotHint = " [.] collapse hunk" - } else if ok { - dotHint = " [.] expand hunk" - } + } + + // mode indicators + if m.collapsed.enabled { + segments = append(segments, "▼") + } + if m.tree.filter { + segments = append(segments, "◉") + } + + // build right-side segments + var rightParts []string + if cnt := m.store.Count(); cnt > 0 { + rightParts = append(rightParts, fmt.Sprintf("%d annotations", cnt)) + } + rightParts = append(rightParts, "? help") + + left := strings.Join(segments, " ") + right := strings.Join(rightParts, " ") + + // truncate filename from left with … if status line is too wide + minRight := len(right) + 4 // 2 for status bar padding + 2 for separator + available := max(m.width-minRight, 0) + + // graceful degradation: drop segments from right to left when too narrow + if len(left) > available { + // rebuild without mode icons first + segments = m.statusSegmentsNoIcons() + left = strings.Join(segments, " ") + } + if len(left) > available { + // rebuild without hunk info + segments = m.statusSegmentsMinimal() + left = strings.Join(segments, " ") + } + if len(left) > available && m.currFile != "" { + // truncate filename + statsStr := fmt.Sprintf("+%d/-%d", m.fileAdds, m.fileRemoves) + nameMax := max(available-len(statsStr)-2, 4) // 2 for separator between name and stats + name := m.currFile + if len(name) > nameMax { + name = "…" + name[len(name)-nameMax+1:] } - hints = "[j/k] scroll [h/tab] files [enter/a] annotate" + deleteHint + hunkHint + viewModeHint + dotHint + filterHint + fileNoteHint + " [n/p] next/prev [Q] discard [q] quit" + left = name + " " + statsStr } - if countHint != "" { - // pad hints to push annotation count to the right - padding := m.width - len(hints) - len(countHint) - 2 // 2 for status bar padding - if padding > 0 { - hints += strings.Repeat(" ", padding) + countHint - } else { - hints += countHint + // pad left to push right section to the end + padding := m.width - len(left) - len(right) - 2 // 2 for status bar padding + if padding > 0 { + return left + strings.Repeat(" ", padding) + right + } + if left != "" { + return left + " " + right + } + return right +} + +// statusSegmentsNoIcons returns left segments without mode indicators (▼ ◉). +func (m Model) statusSegmentsNoIcons() []string { + var segments []string + if m.currFile != "" { + segments = append(segments, m.currFile, fmt.Sprintf("+%d/-%d", m.fileAdds, m.fileRemoves)) + } + if m.focus == paneDiff { + if cur, total := m.currentHunk(); total > 0 && cur > 0 { + segments = append(segments, fmt.Sprintf("hunk %d/%d", cur, total)) } } - return hints + return segments +} + +// statusSegmentsMinimal returns left segments with only filename and stats. +func (m Model) statusSegmentsMinimal() []string { + var segments []string + if m.currFile != "" { + segments = append(segments, m.currFile, fmt.Sprintf("+%d/-%d", m.fileAdds, m.fileRemoves)) + } + return segments } // handleDiscardQuit handles the Q key press for discard-and-quit. diff --git a/ui/model_test.go b/ui/model_test.go index f169cdc9..c0144737 100644 --- a/ui/model_test.go +++ b/ui/model_test.go @@ -283,40 +283,32 @@ func TestModel_FKeyFilterToggle(t *testing.T) { }) } -func TestModel_StatusBarFilterHint(t *testing.T) { +func TestModel_StatusBarFilterIndicator(t *testing.T) { lines := []diff.DiffLine{{NewNum: 1, Content: "line1", ChangeType: diff.ChangeContext}} - t.Run("filter hint shown when annotations exist", func(t *testing.T) { + t.Run("filter icon shown when filter active", func(t *testing.T) { m := testModel([]string{"a.go"}, map[string][]diff.DiffLine{"a.go": lines}) m.tree = newFileTree([]string{"a.go"}) + m.tree.filter = true m.ready = true m.currFile = "a.go" m.diffLines = lines - m.store.Add(annotation.Annotation{File: "a.go", Line: 1, Type: " ", Comment: "note"}) + m.width = 200 - m.focus = paneTree - view := m.View() - assert.Contains(t, view, "[f] filter", "tree pane should show filter hint when annotations exist") - - m.focus = paneDiff - view = m.View() - assert.Contains(t, view, "[f] filter", "diff pane should show filter hint when annotations exist") + status := m.statusBarText() + assert.Contains(t, status, "◉", "should show filter icon when filter active") }) - t.Run("filter hint hidden when no annotations", func(t *testing.T) { + t.Run("filter icon hidden when filter inactive", func(t *testing.T) { m := testModel([]string{"a.go"}, map[string][]diff.DiffLine{"a.go": lines}) m.tree = newFileTree([]string{"a.go"}) m.ready = true m.currFile = "a.go" m.diffLines = lines + m.width = 200 - m.focus = paneTree - view := m.View() - assert.NotContains(t, view, "[f] filter", "tree pane should not show filter hint without annotations") - - m.focus = paneDiff - view = m.View() - assert.NotContains(t, view, "[f] filter", "diff pane should not show filter hint without annotations") + status := m.statusBarText() + assert.NotContains(t, status, "◉", "should not show filter icon when filter inactive") }) } @@ -510,19 +502,17 @@ func TestModel_ViewOutput(t *testing.T) { m.tree = newFileTree([]string{"internal/a.go", "internal/b.go"}) m.ready = true - // tree pane focused - should show tree navigation hints + // tree pane focused - should show file tree and help hint m.focus = paneTree view := m.View() assert.Contains(t, view, "a.go") assert.Contains(t, view, "b.go") - assert.Contains(t, view, "quit") - assert.Contains(t, view, "navigate") + assert.Contains(t, view, "? help") - // diff pane focused - should show diff hints + // diff pane focused - should show help hint m.focus = paneDiff view = m.View() - assert.Contains(t, view, "annotate") - assert.Contains(t, view, "scroll") + assert.Contains(t, view, "? help") } func TestModel_ViewNotReady(t *testing.T) { @@ -665,19 +655,23 @@ func TestModel_EnterInDiffPaneOnDividerIgnored(t *testing.T) { assert.False(t, model.annotating, "enter on divider should not start annotation") } -func TestModel_StatusBarShowsEnterAnnotateHint(t *testing.T) { +func TestModel_StatusBarShowsFilenameAndStats(t *testing.T) { lines := []diff.DiffLine{ {NewNum: 1, Content: "line1", ChangeType: diff.ChangeContext}, + {NewNum: 2, Content: "added", ChangeType: diff.ChangeAdd}, } m := testModel([]string{"a.go"}, map[string][]diff.DiffLine{"a.go": lines}) m.tree = newFileTree([]string{"a.go"}) m.ready = true m.currFile = "a.go" m.diffLines = lines + m.fileAdds = 1 m.focus = paneDiff - view := m.View() - assert.Contains(t, view, "[enter/a] annotate", "diff pane status bar should show enter/a annotate hint") + status := m.statusBarText() + assert.Contains(t, status, "a.go", "status bar should show filename") + assert.Contains(t, status, "+1/-0", "status bar should show diff stats") + assert.Contains(t, status, "? help", "status bar should show help hint") } func TestModel_AnnotateEnterSaves(t *testing.T) { @@ -906,19 +900,111 @@ func TestModel_AnnotationCountInStatusBar(t *testing.T) { m.width = 120 m.store.Add(annotation.Annotation{File: "a.go", Line: 1, Type: "+", Comment: "note"}) m.store.Add(annotation.Annotation{File: "b.go", Line: 5, Type: " ", Comment: "other"}) - annotated := m.annotatedFiles() - status := m.statusBarText(annotated) + status := m.statusBarText() assert.Contains(t, status, "2 annotations") } func TestModel_NoAnnotationCountWhenEmpty(t *testing.T) { m := testModel([]string{"a.go"}, nil) m.width = 120 - annotated := m.annotatedFiles() - status := m.statusBarText(annotated) + status := m.statusBarText() assert.NotContains(t, status, "annotations") } +func TestModel_StatusBarFilenameTruncation(t *testing.T) { + longFile := "very/long/path/to/some/deeply/nested/file/in/the/project/structure.go" + m := testModel(nil, nil) + m.currFile = longFile + m.fileAdds = 3 + m.fileRemoves = 1 + m.focus = paneDiff + m.width = 40 // narrow terminal forces truncation + + status := m.statusBarText() + assert.Contains(t, status, "…", "should truncate filename with ellipsis") + assert.Contains(t, status, "+3/-1", "should still show stats after truncation") + assert.Contains(t, status, "? help", "should still show help hint") +} + +func TestModel_StatusBarModeIndicators(t *testing.T) { + m := testModel(nil, nil) + m.currFile = "a.go" + m.diffLines = []diff.DiffLine{{NewNum: 1, Content: "add", ChangeType: diff.ChangeAdd}} + m.diffCursor = 0 + m.focus = paneDiff + m.width = 200 + + t.Run("both indicators when collapsed and filtered", func(t *testing.T) { + m.collapsed.enabled = true + m.collapsed.expandedHunks = make(map[int]bool) + m.tree.filter = true + status := m.statusBarText() + assert.Contains(t, status, "▼") + assert.Contains(t, status, "◉") + }) + + t.Run("no indicators in default mode", func(t *testing.T) { + m.collapsed.enabled = false + m.tree.filter = false + status := m.statusBarText() + assert.NotContains(t, status, "▼") + assert.NotContains(t, status, "◉") + }) +} + +func TestModel_StatusBarNarrowTerminalDegradation(t *testing.T) { + lines := []diff.DiffLine{ + {NewNum: 1, Content: "ctx", ChangeType: diff.ChangeContext}, + {NewNum: 2, Content: "add", ChangeType: diff.ChangeAdd}, + } + m := testModel(nil, nil) + m.currFile = "a.go" + m.diffLines = lines + m.diffCursor = 1 + m.fileAdds = 1 + m.focus = paneDiff + m.collapsed.enabled = true + m.collapsed.expandedHunks = make(map[int]bool) + m.tree.filter = true + + t.Run("wide terminal shows all segments", func(t *testing.T) { + m.width = 200 + status := m.statusBarText() + assert.Contains(t, status, "a.go") + assert.Contains(t, status, "+1/-0") + assert.Contains(t, status, "hunk 1/1") + assert.Contains(t, status, "▼") + assert.Contains(t, status, "◉") + assert.Contains(t, status, "? help") + }) + + t.Run("narrow terminal drops icons first", func(t *testing.T) { + m.width = 40 + status := m.statusBarText() + assert.Contains(t, status, "? help") + assert.NotContains(t, status, "◉", "icons should be dropped on narrow terminal") + }) + + t.Run("very narrow terminal drops hunk info", func(t *testing.T) { + m.width = 30 + status := m.statusBarText() + assert.Contains(t, status, "? help") + assert.NotContains(t, status, "hunk", "hunk should be dropped on very narrow terminal") + }) +} + +func TestModel_StatusBarStatsDisplay(t *testing.T) { + m := testModel(nil, nil) + m.currFile = "main.go" + m.fileAdds = 10 + m.fileRemoves = 5 + m.width = 120 + + status := m.statusBarText() + assert.Contains(t, status, "main.go") + assert.Contains(t, status, "+10/-5") +} + func TestModel_AnnotateStatusBar(t *testing.T) { m := testModel([]string{"a.go"}, nil) m.tree = newFileTree([]string{"a.go"}) @@ -1397,7 +1483,7 @@ func TestModel_AnnotateRenderWithDividers(t *testing.T) { assert.Contains(t, rendered, "...") } -func TestModel_StatusBarShowsDeleteOnAnnotatedLine(t *testing.T) { +func TestModel_StatusBarNoShortcutHintsInDiffPane(t *testing.T) { lines := []diff.DiffLine{ {NewNum: 1, Content: "line1", ChangeType: diff.ChangeContext}, {NewNum: 2, Content: "added", ChangeType: diff.ChangeAdd}, @@ -1408,37 +1494,21 @@ func TestModel_StatusBarShowsDeleteOnAnnotatedLine(t *testing.T) { m.currFile = "a.go" m.diffLines = lines m.diffCursor = 0 - m.cursorOnAnnotation = true // cursor on the annotation sub-line + m.cursorOnAnnotation = true m.focus = paneDiff m.store.Add(annotation.Annotation{File: "a.go", Line: 1, Type: " ", Comment: "review this"}) - view := m.View() - assert.Contains(t, view, "[d] delete", "status bar should show delete hint on annotation sub-line") - assert.Contains(t, view, "annotate") -} - -func TestModel_StatusBarHidesDeleteOnNonAnnotatedLine(t *testing.T) { - lines := []diff.DiffLine{ - {NewNum: 1, Content: "line1", ChangeType: diff.ChangeContext}, - {NewNum: 2, Content: "added", ChangeType: diff.ChangeAdd}, - } - m := testModel([]string{"a.go"}, map[string][]diff.DiffLine{"a.go": lines}) - m.tree = newFileTree([]string{"a.go"}) - m.ready = true - m.currFile = "a.go" - m.diffLines = lines - m.diffCursor = 0 - m.focus = paneDiff - - // no annotations exist - delete hint should not appear - view := m.View() - assert.NotContains(t, view, "[d] delete", "status bar should not show delete hint on non-annotated line") - assert.Contains(t, view, "annotate") - - // add annotation on line 2 (index 1), but cursor is on line 1 (index 0) - still no delete - m.store.Add(annotation.Annotation{File: "a.go", Line: 2, Type: "+", Comment: "some comment"}) - view = m.View() - assert.NotContains(t, view, "[d] delete", "status bar should not show delete hint when cursor is on a different line") + status := m.statusBarText() + // shortcut hints moved to help overlay + assert.NotContains(t, status, "[d]") + assert.NotContains(t, status, "[enter/a]") + assert.NotContains(t, status, "[A]") + assert.NotContains(t, status, "[Q]") + assert.NotContains(t, status, "[q]") + // should show filename, stats, annotation count, help hint + assert.Contains(t, status, "a.go") + assert.Contains(t, status, "1 annotations") + assert.Contains(t, status, "? help") } func TestModel_PgDownMovesCursorByPageHeight(t *testing.T) { @@ -2164,7 +2234,7 @@ func TestModel_CursorOnFileAnnotationLineReportsAnnotation(t *testing.T) { assert.True(t, m.cursorLineHasAnnotation(), "cursor on file annotation line should report annotation") } -func TestModel_StatusBarShowsFileNoteHint(t *testing.T) { +func TestModel_StatusBarShowsHelpHint(t *testing.T) { lines := []diff.DiffLine{ {NewNum: 1, Content: "line1", ChangeType: diff.ChangeContext}, } @@ -2174,26 +2244,25 @@ func TestModel_StatusBarShowsFileNoteHint(t *testing.T) { m.currFile = "a.go" m.diffLines = lines - // tree pane should not show file note hint (A only works from diff pane) m.focus = paneTree - view := m.View() - assert.NotContains(t, view, "[A] file note", "tree pane should not show file note hint") + status := m.statusBarText() + assert.Contains(t, status, "? help", "tree pane should show help hint") - // diff pane should show file note hint m.focus = paneDiff - view = m.View() - assert.Contains(t, view, "[A] file note", "diff pane should show file note hint when file is loaded") + status = m.statusBarText() + assert.Contains(t, status, "? help", "diff pane should show help hint") } -func TestModel_StatusBarHidesFileNoteHintWithoutFile(t *testing.T) { +func TestModel_StatusBarNoFilenameWithoutFile(t *testing.T) { m := testModel([]string{"a.go"}, nil) m.tree = newFileTree([]string{"a.go"}) m.ready = true m.currFile = "" m.focus = paneTree - view := m.View() - assert.NotContains(t, view, "[A] file note", "should not show file note hint when no file is loaded") + status := m.statusBarText() + assert.NotContains(t, status, "+") + assert.Contains(t, status, "? help") } func TestModel_CursorNavigatesToFileAnnotation(t *testing.T) { @@ -2499,11 +2568,11 @@ func TestModel_StatusBarShowsHunkIndicator(t *testing.T) { m.currFile = "a.go" m.focus = paneDiff - status := m.statusBarText(m.annotatedFiles()) - assert.Contains(t, status, "[ ] hunk 1/2") + status := m.statusBarText() + assert.Contains(t, status, "hunk 1/2") m.diffCursor = 3 - status = m.statusBarText(m.annotatedFiles()) + status = m.statusBarText() assert.Contains(t, status, "hunk 2/2") } @@ -2519,24 +2588,24 @@ func TestModel_StatusBarNoHunkIndicatorWithoutChanges(t *testing.T) { m.currFile = "a.go" m.focus = paneDiff - status := m.statusBarText(m.annotatedFiles()) - assert.NotContains(t, status, "[ ] hunk", "should not show hunk hint when no hunks") + status := m.statusBarText() + assert.NotContains(t, status, "hunk", "should not show hunk when cursor on context line") } -func TestModel_StatusBarHunksHintInDiffPane(t *testing.T) { +func TestModel_StatusBarHunkOnlyInDiffPane(t *testing.T) { m := testModel(nil, nil) m.currFile = "a.go" m.diffLines = []diff.DiffLine{{NewNum: 1, Content: "add", ChangeType: diff.ChangeAdd}} m.diffCursor = 0 m.focus = paneDiff - status := m.statusBarText(m.annotatedFiles()) - assert.Contains(t, status, "[ ] hunk 1/1") + status := m.statusBarText() + assert.Contains(t, status, "hunk 1/1") - // tree pane should not show hunk hint + // tree pane should not show hunk m.focus = paneTree - status = m.statusBarText(m.annotatedFiles()) - assert.NotContains(t, status, "[ ] hunk") + status = m.statusBarText() + assert.NotContains(t, status, "hunk") } func TestModel_EditExistingFileAnnotationShowsInput(t *testing.T) { @@ -2933,26 +3002,29 @@ func TestModel_StatusBarDiscardConfirmation(t *testing.T) { m.store.Add(annotation.Annotation{File: "b.go", Line: 5, Type: " ", Comment: "other"}) m.inConfirmDiscard = true - annotated := m.annotatedFiles() - status := m.statusBarText(annotated) + status := m.statusBarText() assert.Equal(t, "discard 2 annotations? [y/n]", status) } -func TestModel_StatusBarShowsDiscardHint(t *testing.T) { +func TestModel_StatusBarNoKeyHints(t *testing.T) { m := testModel([]string{"a.go"}, nil) m.width = 120 - t.Run("tree pane", func(t *testing.T) { + t.Run("tree pane has no shortcut hints", func(t *testing.T) { m.focus = paneTree - status := m.statusBarText(m.annotatedFiles()) - assert.Contains(t, status, "[Q] discard") - assert.Contains(t, status, "[q] quit") + status := m.statusBarText() + assert.NotContains(t, status, "[Q]") + assert.NotContains(t, status, "[q]") + assert.NotContains(t, status, "[j/k]") + assert.Contains(t, status, "? help") }) - t.Run("diff pane", func(t *testing.T) { + t.Run("diff pane has no shortcut hints", func(t *testing.T) { m.focus = paneDiff - status := m.statusBarText(m.annotatedFiles()) - assert.Contains(t, status, "[Q] discard") - assert.Contains(t, status, "[q] quit") + status := m.statusBarText() + assert.NotContains(t, status, "[Q]") + assert.NotContains(t, status, "[q]") + assert.NotContains(t, status, "[enter/a]") + assert.Contains(t, status, "? help") }) } From 6862e3b6235ae04bc332779ada3eef28cc518b83 Mon Sep 17 00:00:00 2001 From: Umputun Date: Thu, 2 Apr 2026 16:47:47 -0500 Subject: [PATCH 3/8] feat: add help overlay rendering with bordered popup and section headers --- ui/model.go | 44 +++++++++++++++++++++++++++++++++++++++++ ui/model_test.go | 51 ++++++++++++++++++++++++++++++++++++++++++++++++ ui/styles.go | 4 ++++ 3 files changed, 99 insertions(+) diff --git a/ui/model.go b/ui/model.go index 69240e52..ee7937bb 100644 --- a/ui/model.go +++ b/ui/model.go @@ -73,6 +73,8 @@ type Model struct { fileAdds int // cached count of added lines in current file fileRemoves int // cached count of removed lines in current file + showHelp bool // true when help overlay is visible + discarded bool // true when user chose to discard annotations and quit inConfirmDiscard bool // true when showing discard confirmation prompt noConfirmDiscard bool // skip confirmation prompt on discard quit @@ -496,6 +498,12 @@ func (m Model) View() string { mainView := lipgloss.JoinHorizontal(lipgloss.Top, treePane, diffPane) + if m.showHelp { + // replace main content with centered help overlay + helpBox := m.helpOverlay() + mainView = lipgloss.Place(m.width, ph+2, lipgloss.Center, lipgloss.Center, helpBox) + } + if m.noStatusBar { return mainView } @@ -613,6 +621,42 @@ func (m Model) statusSegmentsMinimal() []string { return segments } +// helpOverlay returns a bordered help popup with keybinding sections. +func (m Model) helpOverlay() string { + help := "" + + "Navigation\n" + + " tab switch pane\n" + + " n / p next / prev file\n" + + " j / k scroll down / up\n" + + " g / G top / bottom\n" + + " h / l scroll left / right\n" + + " { / } prev / next hunk\n" + + " enter focus diff pane\n" + + "\n" + + "Annotations\n" + + " enter annotate line (diff pane)\n" + + " A annotate file\n" + + " f filter annotated files\n" + + "\n" + + "View\n" + + " v toggle collapsed mode\n" + + " . expand/collapse hunk\n" + + " [ / ] narrow / widen tree\n" + + "\n" + + "Quit\n" + + " q quit\n" + + " Q discard annotations & quit\n" + + " ? / esc close help" + + border := lipgloss.NormalBorder() + boxStyle := lipgloss.NewStyle(). + Border(border). + BorderForeground(lipgloss.Color(m.styles.colors.Accent)). + Padding(1, 2) + + return boxStyle.Render(help) +} + // handleDiscardQuit handles the Q key press for discard-and-quit. func (m Model) handleDiscardQuit() (tea.Model, tea.Cmd) { if m.store.Count() == 0 || m.noConfirmDiscard || m.noStatusBar { diff --git a/ui/model_test.go b/ui/model_test.go index c0144737..d99cfacd 100644 --- a/ui/model_test.go +++ b/ui/model_test.go @@ -3028,3 +3028,54 @@ func TestModel_StatusBarNoKeyHints(t *testing.T) { assert.Contains(t, status, "? help") }) } + +func TestModel_HelpOverlaySections(t *testing.T) { + m := testModel([]string{"a.go"}, nil) + m.styles = plainStyles() + help := m.helpOverlay() + + // verify section headers are present + assert.Contains(t, help, "Navigation") + assert.Contains(t, help, "Annotations") + assert.Contains(t, help, "View") + assert.Contains(t, help, "Quit") +} + +func TestModel_HelpOverlayKeyListings(t *testing.T) { + m := testModel([]string{"a.go"}, nil) + m.styles = plainStyles() + help := m.helpOverlay() + + // verify key listings are present + keys := []string{ + "tab", "n / p", "j / k", "g / G", "h / l", "{ / }", + "enter", "A", "f", "v", ".", "[ / ]", + "q", "Q", "? / esc", + } + for _, k := range keys { + assert.Contains(t, help, k, "help overlay should contain key: %s", k) + } +} + +func TestModel_HelpOverlayInView(t *testing.T) { + m := testModel([]string{"a.go"}, nil) + m.styles = plainStyles() + m.tree = newFileTree([]string{"a.go"}) + m.ready = true + m.width = 80 + m.height = 30 + + // without help, view should not contain help sections + m.showHelp = false + view := m.View() + assert.NotContains(t, view, "Navigation") + assert.NotContains(t, view, "Annotations") + + // with help, view should contain help sections + m.showHelp = true + view = m.View() + assert.Contains(t, view, "Navigation") + assert.Contains(t, view, "Annotations") + assert.Contains(t, view, "View") + assert.Contains(t, view, "Quit") +} diff --git a/ui/styles.go b/ui/styles.go index cab0197e..0025cd3c 100644 --- a/ui/styles.go +++ b/ui/styles.go @@ -56,6 +56,8 @@ type styles struct { DiffCursorLine lipgloss.Style // annotation AnnotationLine lipgloss.Style + + colors Colors // original color values for dynamic style construction } // normalizeColor ensures hex color values have a # prefix. @@ -171,6 +173,8 @@ func newStyles(c Colors) styles { AnnotationLine: lipgloss.NewStyle(). Foreground(lipgloss.Color(c.Annotation)). Italic(true), + + colors: c, } } From e043f7dc1b89228f22bd3c87a5b1374de5f206c7 Mon Sep 17 00:00:00 2001 From: Umputun Date: Thu, 2 Apr 2026 16:51:11 -0500 Subject: [PATCH 4/8] feat: wire up ? key handling for help overlay toggle --- ui/model.go | 47 +++++++++++++++++++++++++++++---------- ui/model_test.go | 58 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 12 deletions(-) diff --git a/ui/model.go b/ui/model.go index ee7937bb..017ff6c8 100644 --- a/ui/model.go +++ b/ui/model.go @@ -196,6 +196,11 @@ func (m Model) handleKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { return m.handleAnnotateKey(msg) } + // help overlay: toggle with ?, dismiss with esc, block everything else + if msg.String() == "?" || m.showHelp { + return m.handleHelpKey(msg) + } + switch { case msg.String() == "Q": return m.handleDiscardQuit() @@ -225,18 +230,7 @@ func (m Model) handleKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { return m.loadSelectedIfChanged() case msg.String() == "enter": - switch m.focus { - case paneTree: - if m.currFile != "" { - m.focus = paneDiff - } - return m, nil - case paneDiff: - cmd := m.startAnnotation() - m.viewport.SetContent(m.renderDiff()) - return m, cmd - } - return m, nil + return m.handleEnterKey() case msg.String() == "A": // file-level annotation only from diff pane to avoid annotating the wrong file @@ -667,6 +661,35 @@ func (m Model) handleDiscardQuit() (tea.Model, tea.Cmd) { return m, nil } +// handleEnterKey handles enter key based on current pane focus. +func (m Model) handleEnterKey() (tea.Model, tea.Cmd) { + switch m.focus { + case paneTree: + if m.currFile != "" { + m.focus = paneDiff + } + return m, nil + case paneDiff: + cmd := m.startAnnotation() + m.viewport.SetContent(m.renderDiff()) + return m, cmd + } + return m, nil +} + +// handleHelpKey handles help overlay keys. +// ? toggles the overlay, esc closes it, all other keys are blocked while showing. +func (m Model) handleHelpKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { + if msg.String() == "?" { + m.showHelp = !m.showHelp + return m, nil + } + if msg.Type == tea.KeyEsc { + m.showHelp = false + } + return m, nil +} + // handleConfirmDiscardKey handles keys during discard confirmation prompt. func (m Model) handleConfirmDiscardKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { switch msg.String() { diff --git a/ui/model_test.go b/ui/model_test.go index d99cfacd..d97d2fdf 100644 --- a/ui/model_test.go +++ b/ui/model_test.go @@ -3079,3 +3079,61 @@ func TestModel_HelpOverlayInView(t *testing.T) { assert.Contains(t, view, "View") assert.Contains(t, view, "Quit") } + +func TestModel_HelpToggle(t *testing.T) { + m := testModel([]string{"a.go"}, map[string][]diff.DiffLine{"a.go": {{ChangeType: diff.ChangeContext, Content: "x"}}}) + m.currFile = "a.go" + m.focus = paneDiff + assert.False(t, m.showHelp) + + // press ? to open help + result, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'?'}}) + model := result.(Model) + assert.True(t, model.showHelp) + + // press ? again to close help + result, _ = model.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'?'}}) + model = result.(Model) + assert.False(t, model.showHelp) +} + +func TestModel_HelpCloseWithEsc(t *testing.T) { + m := testModel([]string{"a.go"}, map[string][]diff.DiffLine{"a.go": {{ChangeType: diff.ChangeContext, Content: "x"}}}) + m.currFile = "a.go" + m.showHelp = true + + // press esc to close help + result, _ := m.Update(tea.KeyMsg{Type: tea.KeyEsc}) + model := result.(Model) + assert.False(t, model.showHelp) +} + +func TestModel_HelpBlocksOtherKeys(t *testing.T) { + m := testModel([]string{"a.go", "b.go"}, map[string][]diff.DiffLine{ + "a.go": {{ChangeType: diff.ChangeContext, Content: "x"}}, + "b.go": {{ChangeType: diff.ChangeContext, Content: "y"}}, + }) + m.currFile = "a.go" + m.focus = paneDiff + m.showHelp = true + + // navigation keys should be blocked + for _, key := range []rune{'n', 'p', 'v', 'f', 'q', 'j', 'k'} { + result, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{key}}) + model := result.(Model) + assert.True(t, model.showHelp, "key %q should not close help", string(key)) + assert.Nil(t, cmd, "key %q should produce no command", string(key)) + } + + // tab should also be blocked + result, cmd := m.Update(tea.KeyMsg{Type: tea.KeyTab}) + model := result.(Model) + assert.True(t, model.showHelp, "tab should not close help") + assert.Nil(t, cmd, "tab should produce no command") + + // enter should be blocked + result, cmd = model.Update(tea.KeyMsg{Type: tea.KeyEnter}) + model = result.(Model) + assert.True(t, model.showHelp, "enter should not close help") + assert.Nil(t, cmd, "enter should produce no command") +} From b09dc5d7f7a7fe4310e02090a67f7cc7974d1de7 Mon Sep 17 00:00:00 2001 From: Umputun Date: Thu, 2 Apr 2026 16:53:20 -0500 Subject: [PATCH 5/8] feat: update documentation for status line and help overlay - add ? help keybinding to README and plugin usage reference - add status line and help overlay to features list - move plan to completed --- .../skills/revdiff/references/usage.md | 1 + .../skills/revdiff/scripts/launch-revdiff.sh | 12 +- README.md | 3 + .../20260402-status-line-help-overlay.md | 139 ++++++++++++++++++ ui/model.go | 81 +++++----- ui/model_test.go | 35 ++++- 6 files changed, 228 insertions(+), 43 deletions(-) create mode 100644 docs/plans/completed/20260402-status-line-help-overlay.md diff --git a/.claude-plugin/skills/revdiff/references/usage.md b/.claude-plugin/skills/revdiff/references/usage.md index 0a388b50..4ed5bf18 100644 --- a/.claude-plugin/skills/revdiff/references/usage.md +++ b/.claude-plugin/skills/revdiff/references/usage.md @@ -46,6 +46,7 @@ revdiff HEAD~1 # review last commit | `v` | Toggle collapsed diff mode (shows final text with change markers) | | `.` | Expand/collapse individual hunk under cursor (collapsed mode only) | | `f` | Toggle filter: all files / annotated only | +| `?` | Toggle help overlay showing all keybindings | | `q` | Quit, output annotations to stdout | | `Q` | Discard all annotations and quit (confirms if annotations exist) | diff --git a/.claude-plugin/skills/revdiff/scripts/launch-revdiff.sh b/.claude-plugin/skills/revdiff/scripts/launch-revdiff.sh index fd68d62d..d1fd8de1 100755 --- a/.claude-plugin/skills/revdiff/scripts/launch-revdiff.sh +++ b/.claude-plugin/skills/revdiff/scripts/launch-revdiff.sh @@ -20,9 +20,17 @@ trap 'rm -f "$OUTPUT_FILE"' EXIT REVDIFF_CMD="$REVDIFF_BIN --output=$OUTPUT_FILE $*" CWD="$(pwd)" +# build descriptive title: "revdiff: dirname [ref]" +DIR_NAME=$(basename "$CWD") +TITLE_REF="" +for arg in "$@"; do + case "$arg" in --*) ;; *) TITLE_REF="$arg"; break ;; esac +done +OVERLAY_TITLE="rd: ${DIR_NAME}${TITLE_REF:+ [$TITLE_REF]}" + # tmux: display-popup -E blocks until command exits if [ -n "${TMUX:-}" ] && command -v tmux >/dev/null 2>&1; then - tmux display-popup -E -w 90% -h 90% -T " revdiff " -d "$CWD" -- sh -c "$REVDIFF_CMD" + tmux display-popup -E -w 90% -h 90% -T " $OVERLAY_TITLE " -d "$CWD" -- sh -c "$REVDIFF_CMD" cat "$OUTPUT_FILE" exit 0 fi @@ -33,7 +41,7 @@ if [ -n "$KITTY_SOCK" ] && command -v kitty >/dev/null 2>&1; then SENTINEL=$(mktemp /tmp/revdiff-done-XXXXXX) rm -f "$SENTINEL" - KITTY_ARGS=(kitty @ --to "$KITTY_SOCK" launch --type=overlay --title="revdiff" --cwd="$CWD") + KITTY_ARGS=(kitty @ --to "$KITTY_SOCK" launch --type=overlay --title="$OVERLAY_TITLE" --cwd="$CWD") if [ -n "${KITTY_WINDOW_ID:-}" ]; then KITTY_ARGS+=(--match "id:${KITTY_WINDOW_ID}") fi diff --git a/README.md b/README.md index 05943ee2..d1fa7805 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,8 @@ Built for a specific use case: reviewing code changes without leaving a terminal - Two-pane TUI: file tree (left) + colorized diff viewport (right) - Hunk navigation to jump between change groups - Filter file tree to show only annotated files +- Status line with filename, diff stats, hunk position, and mode indicators +- Help overlay (`?`) showing all keybindings organized by section - Fully customizable colors via environment variables, CLI flags, or config file ![revdiff screenshot](screenshot.png) @@ -233,6 +235,7 @@ revdiff HEAD~1 | `v` | Toggle collapsed diff mode (shows final text with change markers) | | `.` | Expand/collapse individual hunk under cursor (collapsed mode only) | | `f` | Toggle filter: all files / annotated only (shown when annotations exist) | +| `?` | Toggle help overlay showing all keybindings | | `q` | Quit, output annotations to stdout | | `Q` | Discard all annotations and quit (confirms if annotations exist) | diff --git a/docs/plans/completed/20260402-status-line-help-overlay.md b/docs/plans/completed/20260402-status-line-help-overlay.md new file mode 100644 index 00000000..dba23eee --- /dev/null +++ b/docs/plans/completed/20260402-status-line-help-overlay.md @@ -0,0 +1,139 @@ +# Split Status Bar into Status Line + Help Overlay + +## Overview +Replace the current single status bar (which mixes status info and shortcut hints) with two separate concerns: +1. **Status line** — shows file info, diff stats, hunk position, mode indicators, annotation count +2. **Help overlay** — a modal popup triggered by `?` showing all keybindings organized by section + +The current `statusBarText()` in `ui/model.go:488-551` is crowded and hard to scan. Splitting status from help makes both more useful: the status line becomes a clean info strip, and the help overlay provides comprehensive reference without cluttering the screen. + +## Context +- **Primary file:** `ui/model.go` — `statusBarText()` (lines 488-551), `View()` (lines 442-486), `handleKey()` (lines 188-259) +- **Styles:** `ui/styles.go` — `StatusBar` style, `Colors` struct +- **Diff navigation:** `ui/diffview.go` — `currentHunk()`, `findHunks()` +- **Collapsed mode:** `ui/collapsed.go` — `collapsedState` +- **Model fields:** `ui/model.go` — `currFile`, `diffLines`, `store`, `collapsed`, etc. + +## Solution Overview + +### Status line layout (left to right) +``` +filename +N/-N hunk X/Y ▼ ◉ 3 annotations ? help +``` +- **filename** — current file path (truncated from left with `…` if too long) +- **+N/-N** — additions/deletions count for the current file +- **hunk X/Y** — current hunk position (only when cursor is on a changed line) +- **▼** — collapsed mode indicator (only when active) +- **◉** — filter active indicator (only when active) +- **right-aligned:** annotation count + `? help` hint + +### Help overlay +- Triggered by `?` key, dismissed by `?` or `esc` +- Centered bordered box rendered on top of the main view +- Sections: Navigation, Annotations, View, Quit +- Uses lipgloss border styling consistent with existing pane borders + +## Technical Details + +### New fields in Model +- `showHelp bool` — true when help overlay is visible +- No new files needed — help rendering goes in a new `helpOverlay()` method in `model.go` + +### File stats computation +- Count adds/removes from `m.diffLines` on file load (in `handleFileLoaded`) +- Cache as `fileAdds int`, `fileRemoves int` fields on Model +- Reset on file change + +### Status line segments +Each segment is a small string. Segments are joined with double-space separators. Right-aligned section uses padding like current implementation. + +### Help overlay rendering +- Build help text as a lipgloss-bordered box +- When `m.showHelp` is true, `View()` replaces the main content area with the centered help popup (standard bubbletea modal pattern — no true compositing, the help box replaces tree+diff content) +- Use `lipgloss.Place(m.width, paneHeight, lipgloss.Center, lipgloss.Center, helpBox)` + status bar below +- Note: bubbletea reports `?` key correctly via `msg.String()` (shifted `/` key) + +### Narrow terminal handling +- Truncate filename from left with `…` prefix when space is tight +- Drop lower-priority segments (hunk, mode icons) if width is insufficient + +## Development Approach +- **Testing approach:** regular (code first, then tests) +- Complete each task fully before moving to the next +- Run tests after each change +- Maintain backward compatibility (existing CLI flags, config, styles all still work) + +## Implementation Steps + +### Task 1: Compute and cache file diff stats + +**Files:** +- Modify: `ui/model.go` +- Modify: `ui/model_test.go` + +- [x] add `fileAdds` and `fileRemoves` int fields to `Model` struct +- [x] add `computeFileStats()` method that counts add/remove lines from `m.diffLines` +- [x] call `computeFileStats()` in `handleFileLoaded` after setting `m.diffLines` +- [x] write tests for `computeFileStats()` with various diff line combinations +- [x] run `make test` — must pass before task 2 + +### Task 2: Rewrite status line and update tests + +**Files:** +- Modify: `ui/model.go` +- Modify: `ui/model_test.go` + +- [x] rewrite `statusBarText()` to show: filename, +N/-N stats, hunk X/Y, mode icons (▼ ◉), right-aligned annotation count + `? help` +- [x] keep special cases for `inConfirmDiscard` and `annotating` modes unchanged +- [x] implement filename truncation with `…` prefix for narrow terminals +- [x] drop hunk and mode icons gracefully when terminal is too narrow +- [x] update existing `statusBarText` tests to match new format (no shortcut hints, has filename/stats) +- [x] add test cases for: filename truncation, mode indicators present/absent, stats display +- [x] add test cases for narrow terminal width graceful degradation +- [x] run `make test` — must pass before task 3 + +### Task 3: Add help overlay rendering + +**Files:** +- Modify: `ui/model.go` + +- [x] add `showHelp bool` field to Model +- [x] add `helpOverlay()` method returning the bordered help text with sections (Navigation, Annotations, View, Quit) +- [x] modify `View()` to overlay help popup using `lipgloss.Place()` when `m.showHelp` is true +- [x] write tests for `helpOverlay()` verifying section headers and key listings are present +- [x] run `make test` — must pass before task 4 + +### Task 4: Wire up `?` key handling + +**Files:** +- Modify: `ui/model.go` +- Modify: `ui/model_test.go` + +- [x] handle `?` key in `handleKey()` to toggle `m.showHelp` +- [x] handle `esc` key to close help when `m.showHelp` is true +- [x] block all other key handling when help overlay is showing (except `?` and `esc`) +- [x] write tests for help toggle behavior (open, close with ?, close with esc) +- [x] write test that other keys are blocked when help is showing +- [x] run `make test` — must pass before task 5 + +### Task 5: Verify acceptance criteria +- [x] verify status line shows filename, stats, hunk, mode icons, annotations, help hint +- [x] verify help overlay opens with `?` and closes with `?` or `esc` +- [x] verify no shortcut hints in status bar anymore (all moved to help overlay) +- [x] verify special modes (annotation input, discard confirm) still work in status bar +- [x] run full test suite: `make test` +- [x] run linter: `make lint` + +### Task 6: [Final] Update documentation +- [x] update README.md with new `?` help shortcut and status line description +- [x] update `.claude-plugin/skills/revdiff/references/usage.md` with `?` help keybinding +- [x] update CLAUDE.md if any new patterns discovered +- [x] move this plan to `docs/plans/completed/` + +## Post-Completion + +**Manual verification:** +- test with narrow terminal widths (< 80 cols) to verify truncation +- test with large diffs (many hunks) to verify hunk counter +- test collapsed mode + filter active to verify both icons show +- verify help overlay looks correct with different color themes diff --git a/ui/model.go b/ui/model.go index 017ff6c8..44e17106 100644 --- a/ui/model.go +++ b/ui/model.go @@ -11,6 +11,7 @@ import ( "github.com/charmbracelet/bubbles/viewport" tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/lipgloss" + "github.com/mattn/go-runewidth" "github.com/umputun/revdiff/annotation" "github.com/umputun/revdiff/diff" @@ -520,14 +521,9 @@ func (m Model) statusBarText() string { // build left-side segments var segments []string - // filename segment + // filename and diff stats segments if m.currFile != "" { - segments = append(segments, m.currFile) - } - - // diff stats segment - if m.currFile != "" { - segments = append(segments, fmt.Sprintf("+%d/-%d", m.fileAdds, m.fileRemoves)) + segments = append(segments, m.currFile, fmt.Sprintf("+%d/-%d", m.fileAdds, m.fileRemoves)) } // hunk position (only when cursor is on a changed line in diff pane) @@ -548,7 +544,11 @@ func (m Model) statusBarText() string { // build right-side segments var rightParts []string if cnt := m.store.Count(); cnt > 0 { - rightParts = append(rightParts, fmt.Sprintf("%d annotations", cnt)) + suffix := "annotations" + if cnt == 1 { + suffix = "annotation" + } + rightParts = append(rightParts, fmt.Sprintf("%d %s", cnt, suffix)) } rightParts = append(rightParts, "? help") @@ -556,33 +556,45 @@ func (m Model) statusBarText() string { right := strings.Join(rightParts, " ") // truncate filename from left with … if status line is too wide - minRight := len(right) + 4 // 2 for status bar padding + 2 for separator + minRight := lipgloss.Width(right) + 4 // 2 for status bar padding + 2 for separator available := max(m.width-minRight, 0) // graceful degradation: drop segments from right to left when too narrow - if len(left) > available { + if lipgloss.Width(left) > available { // rebuild without mode icons first segments = m.statusSegmentsNoIcons() left = strings.Join(segments, " ") } - if len(left) > available { + if lipgloss.Width(left) > available { // rebuild without hunk info segments = m.statusSegmentsMinimal() left = strings.Join(segments, " ") } - if len(left) > available && m.currFile != "" { - // truncate filename + if lipgloss.Width(left) > available && m.currFile != "" { + // truncate filename from left, keeping end of path. + // uses display-width measurement to handle wide characters (CJK, emoji) statsStr := fmt.Sprintf("+%d/-%d", m.fileAdds, m.fileRemoves) - nameMax := max(available-len(statsStr)-2, 4) // 2 for separator between name and stats + nameMax := max(available-lipgloss.Width(statsStr)-2, 4) // 2 for separator between name and stats name := m.currFile - if len(name) > nameMax { - name = "…" + name[len(name)-nameMax+1:] + if lipgloss.Width(name) > nameMax { + budget := nameMax - 1 // reserve 1 cell for "…" + runes := []rune(name) + w, cutIdx := 0, len(runes) + for i := len(runes) - 1; i >= 0; i-- { + rw := runewidth.RuneWidth(runes[i]) + if w+rw > budget { + break + } + w += rw + cutIdx = i + } + name = "…" + string(runes[cutIdx:]) } left = name + " " + statsStr } // pad left to push right section to the end - padding := m.width - len(left) - len(right) - 2 // 2 for status bar padding + padding := m.width - lipgloss.Width(left) - lipgloss.Width(right) - 2 // 2 for status bar padding if padding > 0 { return left + strings.Repeat(" ", padding) + right } @@ -619,28 +631,31 @@ func (m Model) statusSegmentsMinimal() []string { func (m Model) helpOverlay() string { help := "" + "Navigation\n" + - " tab switch pane\n" + - " n / p next / prev file\n" + - " j / k scroll down / up\n" + - " g / G top / bottom\n" + - " h / l scroll left / right\n" + - " { / } prev / next hunk\n" + - " enter focus diff pane\n" + + " tab switch pane\n" + + " n / p next / prev file\n" + + " j / k scroll down / up\n" + + " PgDn/PgUp page down / up\n" + + " Ctrl+d/u half-page down / up\n" + + " Home/End top / bottom\n" + + " h / l focus tree / diff pane\n" + + " \u2190 / \u2192 scroll left / right (diff)\n" + + " [ / ] prev / next hunk\n" + + " enter focus diff pane\n" + "\n" + "Annotations\n" + - " enter annotate line (diff pane)\n" + - " A annotate file\n" + - " f filter annotated files\n" + + " a / enter annotate line (diff pane)\n" + + " A annotate file\n" + + " d delete annotation\n" + "\n" + "View\n" + - " v toggle collapsed mode\n" + - " . expand/collapse hunk\n" + - " [ / ] narrow / widen tree\n" + + " v toggle collapsed mode\n" + + " . expand/collapse hunk\n" + + " f filter annotated files\n" + "\n" + "Quit\n" + - " q quit\n" + - " Q discard annotations & quit\n" + - " ? / esc close help" + " q quit\n" + + " Q discard annotations & quit\n" + + " ? / esc close help" border := lipgloss.NormalBorder() boxStyle := lipgloss.NewStyle(). diff --git a/ui/model_test.go b/ui/model_test.go index d97d2fdf..3137b358 100644 --- a/ui/model_test.go +++ b/ui/model_test.go @@ -7,6 +7,7 @@ import ( "testing" tea "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/lipgloss" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -926,6 +927,23 @@ func TestModel_StatusBarFilenameTruncation(t *testing.T) { assert.Contains(t, status, "? help", "should still show help hint") } +func TestModel_StatusBarFilenameTruncationWideChars(t *testing.T) { + // CJK characters are 2 display cells wide per rune, the truncation must use + // display-width measurement, not rune count, to avoid overflowing the status line + wideFile := "path/to/日本語のファイル名/テスト.go" + m := testModel(nil, nil) + m.currFile = wideFile + m.fileAdds = 1 + m.fileRemoves = 0 + m.focus = paneDiff + m.width = 40 + + status := m.statusBarText() + assert.Contains(t, status, "…", "should truncate wide-char filename with ellipsis") + assert.Contains(t, status, "+1/-0", "should still show stats after truncation") + assert.LessOrEqual(t, lipgloss.Width(status), m.width-2, "status text must fit within terminal width minus padding") +} + func TestModel_StatusBarModeIndicators(t *testing.T) { m := testModel(nil, nil) m.currFile = "a.go" @@ -979,14 +997,15 @@ func TestModel_StatusBarNarrowTerminalDegradation(t *testing.T) { }) t.Run("narrow terminal drops icons first", func(t *testing.T) { - m.width = 40 + m.width = 35 status := m.statusBarText() assert.Contains(t, status, "? help") - assert.NotContains(t, status, "◉", "icons should be dropped on narrow terminal") + assert.NotContains(t, status, "◉", "filter icon should be dropped on narrow terminal") + assert.NotContains(t, status, "▼", "collapsed icon should be dropped on narrow terminal") }) t.Run("very narrow terminal drops hunk info", func(t *testing.T) { - m.width = 30 + m.width = 28 status := m.statusBarText() assert.Contains(t, status, "? help") assert.NotContains(t, status, "hunk", "hunk should be dropped on very narrow terminal") @@ -1507,7 +1526,7 @@ func TestModel_StatusBarNoShortcutHintsInDiffPane(t *testing.T) { assert.NotContains(t, status, "[q]") // should show filename, stats, annotation count, help hint assert.Contains(t, status, "a.go") - assert.Contains(t, status, "1 annotations") + assert.Contains(t, status, "1 annotation") assert.Contains(t, status, "? help") } @@ -2261,7 +2280,7 @@ func TestModel_StatusBarNoFilenameWithoutFile(t *testing.T) { m.focus = paneTree status := m.statusBarText() - assert.NotContains(t, status, "+") + assert.NotContains(t, status, "/-", "no diff stats should be shown without a file") assert.Contains(t, status, "? help") } @@ -3048,8 +3067,8 @@ func TestModel_HelpOverlayKeyListings(t *testing.T) { // verify key listings are present keys := []string{ - "tab", "n / p", "j / k", "g / G", "h / l", "{ / }", - "enter", "A", "f", "v", ".", "[ / ]", + "tab", "n / p", "j / k", "PgDn/PgUp", "Ctrl+d/u", "Home/End", "h / l", "← / →", "[ / ]", + "a / enter", "A", "d", "f", "v", ".", "q", "Q", "? / esc", } for _, k := range keys { @@ -3118,7 +3137,7 @@ func TestModel_HelpBlocksOtherKeys(t *testing.T) { m.showHelp = true // navigation keys should be blocked - for _, key := range []rune{'n', 'p', 'v', 'f', 'q', 'j', 'k'} { + for _, key := range []rune{'n', 'p', 'v', 'f', 'q', 'Q', 'j', 'k'} { result, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{key}}) model := result.(Model) assert.True(t, model.showHelp, "key %q should not close help", string(key)) From 885baab6fc2ef6fb23be50786a5314997be168b2 Mon Sep 17 00:00:00 2001 From: Umputun Date: Thu, 2 Apr 2026 17:56:54 -0500 Subject: [PATCH 6/8] fix: improve status line and help overlay rendering Show hunk count always in diff pane (not just when cursor is on a changed line), render help overlay on top of content instead of replacing it, add pipe separators between status line sections, group mode icons into a single section, and fix "1 hunks" singular/plural grammar. --- CLAUDE.md | 1 + ui/model.go | 109 ++++++++++++++++++++++++++++++++++++----------- ui/model_test.go | 53 ++++++++++++++++++++++- 3 files changed, 137 insertions(+), 26 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index bc418d17..06b38a81 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -65,3 +65,4 @@ git diff → diff.ParseUnifiedDiff() → []DiffLine - `DiffLine.Content` has no `+`/`-` prefix - prefix is re-added at render time - Tab replacement happens at render time in `renderDiffLine`, not in diff parsing - `run()` resolves git repo root via `git rev-parse --show-toplevel` so revdiff works from any subdirectory +- Help overlay uses `overlayCenter()` (ANSI-aware compositing via `charmbracelet/x/ansi.Cut`) to render on top of existing content; background (tree pane) remains visible at the edges diff --git a/ui/model.go b/ui/model.go index 44e17106..d2681889 100644 --- a/ui/model.go +++ b/ui/model.go @@ -11,6 +11,7 @@ import ( "github.com/charmbracelet/bubbles/viewport" tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/x/ansi" "github.com/mattn/go-runewidth" "github.com/umputun/revdiff/annotation" @@ -494,9 +495,9 @@ func (m Model) View() string { mainView := lipgloss.JoinHorizontal(lipgloss.Top, treePane, diffPane) if m.showHelp { - // replace main content with centered help overlay + // overlay help popup on top of current content helpBox := m.helpOverlay() - mainView = lipgloss.Place(m.width, ph+2, lipgloss.Center, lipgloss.Center, helpBox) + mainView = m.overlayCenter(mainView, helpBox) } if m.noStatusBar { @@ -526,19 +527,14 @@ func (m Model) statusBarText() string { segments = append(segments, m.currFile, fmt.Sprintf("+%d/-%d", m.fileAdds, m.fileRemoves)) } - // hunk position (only when cursor is on a changed line in diff pane) - if m.focus == paneDiff { - if cur, total := m.currentHunk(); total > 0 && cur > 0 { - segments = append(segments, fmt.Sprintf("hunk %d/%d", cur, total)) - } + // hunk position (always shown in diff pane when there are hunks) + if hs := m.hunkSegment(); hs != "" { + segments = append(segments, hs) } - // mode indicators - if m.collapsed.enabled { - segments = append(segments, "▼") - } - if m.tree.filter { - segments = append(segments, "◉") + // mode indicators (combined into one segment) + if modeIcons := m.statusModeIcons(); modeIcons != "" { + segments = append(segments, modeIcons) } // build right-side segments @@ -552,29 +548,30 @@ func (m Model) statusBarText() string { } rightParts = append(rightParts, "? help") - left := strings.Join(segments, " ") - right := strings.Join(rightParts, " ") + const sep = " | " + left := strings.Join(segments, sep) + right := strings.Join(rightParts, sep) // truncate filename from left with … if status line is too wide - minRight := lipgloss.Width(right) + 4 // 2 for status bar padding + 2 for separator + minRight := lipgloss.Width(right) + 5 // 2 for status bar padding + 3 for separator available := max(m.width-minRight, 0) // graceful degradation: drop segments from right to left when too narrow if lipgloss.Width(left) > available { // rebuild without mode icons first segments = m.statusSegmentsNoIcons() - left = strings.Join(segments, " ") + left = strings.Join(segments, sep) } if lipgloss.Width(left) > available { // rebuild without hunk info segments = m.statusSegmentsMinimal() - left = strings.Join(segments, " ") + left = strings.Join(segments, sep) } if lipgloss.Width(left) > available && m.currFile != "" { // truncate filename from left, keeping end of path. // uses display-width measurement to handle wide characters (CJK, emoji) statsStr := fmt.Sprintf("+%d/-%d", m.fileAdds, m.fileRemoves) - nameMax := max(available-lipgloss.Width(statsStr)-2, 4) // 2 for separator between name and stats + nameMax := max(available-lipgloss.Width(statsStr)-len(sep), 4) // reserve separator between name and stats name := m.currFile if lipgloss.Width(name) > nameMax { budget := nameMax - 1 // reserve 1 cell for "…" @@ -590,7 +587,7 @@ func (m Model) statusBarText() string { } name = "…" + string(runes[cutIdx:]) } - left = name + " " + statsStr + left = name + sep + statsStr } // pad left to push right section to the end @@ -599,21 +596,50 @@ func (m Model) statusBarText() string { return left + strings.Repeat(" ", padding) + right } if left != "" { - return left + " " + right + return left + sep + right } return right } +// hunkSegment returns a formatted hunk position string for the status line. +// returns "hunk X/Y" when cursor is on a changed line, "N hunks"/"1 hunk" otherwise, or empty if not in diff pane. +func (m Model) hunkSegment() string { + if m.focus != paneDiff { + return "" + } + cur, total := m.currentHunk() + if total == 0 { + return "" + } + if cur > 0 { + return fmt.Sprintf("hunk %d/%d", cur, total) + } + if total == 1 { + return "1 hunk" + } + return fmt.Sprintf("%d hunks", total) +} + +// statusModeIcons returns combined mode indicator icons (▼ for collapsed, ◉ for filter). +func (m Model) statusModeIcons() string { + var icons []string + if m.collapsed.enabled { + icons = append(icons, "▼") + } + if m.tree.filter { + icons = append(icons, "◉") + } + return strings.Join(icons, " ") +} + // statusSegmentsNoIcons returns left segments without mode indicators (▼ ◉). func (m Model) statusSegmentsNoIcons() []string { var segments []string if m.currFile != "" { segments = append(segments, m.currFile, fmt.Sprintf("+%d/-%d", m.fileAdds, m.fileRemoves)) } - if m.focus == paneDiff { - if cur, total := m.currentHunk(); total > 0 && cur > 0 { - segments = append(segments, fmt.Sprintf("hunk %d/%d", cur, total)) - } + if hs := m.hunkSegment(); hs != "" { + segments = append(segments, hs) } return segments } @@ -666,6 +692,39 @@ func (m Model) helpOverlay() string { return boxStyle.Render(help) } +// overlayCenter composites fg on top of bg, centered horizontally and vertically. +// uses ANSI-aware string cutting to preserve styling in both layers. +func (m Model) overlayCenter(bg, fg string) string { + bgLines := strings.Split(bg, "\n") + fgLines := strings.Split(fg, "\n") + + fgWidth := lipgloss.Width(fg) + fgHeight := len(fgLines) + bgHeight := len(bgLines) + + startY := (bgHeight - fgHeight) / 2 + startX := max((m.width-fgWidth)/2, 0) + + for i, fgLine := range fgLines { + bgIdx := startY + i + if bgIdx < 0 || bgIdx >= bgHeight { + continue + } + bgLine := bgLines[bgIdx] + // pad bg line to full width so right part is always available + bgW := lipgloss.Width(bgLine) + if bgW < m.width { + bgLine += strings.Repeat(" ", m.width-bgW) + } + + left := ansi.Cut(bgLine, 0, startX) + right := ansi.Cut(bgLine, startX+fgWidth, m.width) + bgLines[bgIdx] = left + fgLine + right + } + + return strings.Join(bgLines, "\n") +} + // handleDiscardQuit handles the Q key press for discard-and-quit. func (m Model) handleDiscardQuit() (tea.Model, tea.Cmd) { if m.store.Count() == 0 || m.noConfirmDiscard || m.noStatusBar { diff --git a/ui/model_test.go b/ui/model_test.go index 3137b358..453b384e 100644 --- a/ui/model_test.go +++ b/ui/model_test.go @@ -2627,6 +2627,55 @@ func TestModel_StatusBarHunkOnlyInDiffPane(t *testing.T) { assert.NotContains(t, status, "hunk") } +func TestModel_StatusBarHunkCountOnContextLine(t *testing.T) { + t.Run("plural hunks", func(t *testing.T) { + lines := []diff.DiffLine{ + {NewNum: 1, Content: "ctx", ChangeType: diff.ChangeContext}, + {NewNum: 2, Content: "add1", ChangeType: diff.ChangeAdd}, + {NewNum: 3, Content: "ctx2", ChangeType: diff.ChangeContext}, + {NewNum: 4, Content: "add2", ChangeType: diff.ChangeAdd}, + } + m := testModel(nil, nil) + m.diffLines = lines + m.diffCursor = 0 + m.currFile = "a.go" + m.focus = paneDiff + + status := m.statusBarText() + assert.Contains(t, status, "2 hunks") + assert.NotContains(t, status, "hunk 0/") + }) + + t.Run("singular hunk", func(t *testing.T) { + lines := []diff.DiffLine{ + {NewNum: 1, Content: "ctx", ChangeType: diff.ChangeContext}, + {NewNum: 2, Content: "add1", ChangeType: diff.ChangeAdd}, + } + m := testModel(nil, nil) + m.diffLines = lines + m.diffCursor = 0 + m.currFile = "a.go" + m.focus = paneDiff + + status := m.statusBarText() + assert.Contains(t, status, "1 hunk") + assert.NotContains(t, status, "1 hunks", "should use singular form for one hunk") + }) +} + +func TestModel_StatusBarPipeSeparators(t *testing.T) { + m := testModel(nil, nil) + m.currFile = "a.go" + m.diffLines = []diff.DiffLine{{NewNum: 1, Content: "add", ChangeType: diff.ChangeAdd}} + m.fileAdds = 1 + m.diffCursor = 0 + m.focus = paneDiff + + status := m.statusBarText() + assert.Contains(t, status, "a.go | +1/-0", "pipe separator between filename and stats") + assert.Contains(t, status, "+1/-0 | hunk", "pipe separator between stats and hunk") +} + func TestModel_EditExistingFileAnnotationShowsInput(t *testing.T) { m := testModel(nil, nil) m.currFile = "a.go" @@ -3090,13 +3139,15 @@ func TestModel_HelpOverlayInView(t *testing.T) { assert.NotContains(t, view, "Navigation") assert.NotContains(t, view, "Annotations") - // with help, view should contain help sections + // with help, view should contain help sections overlaid on top of content m.showHelp = true view = m.View() assert.Contains(t, view, "Navigation") assert.Contains(t, view, "Annotations") assert.Contains(t, view, "View") assert.Contains(t, view, "Quit") + // overlay should preserve background content (tree pane visible on edges) + assert.Contains(t, view, "a.go", "tree pane should be visible behind help overlay") } func TestModel_HelpToggle(t *testing.T) { From 557e666a8119b00dfb8d66bba087f339a4a56e32 Mon Sep 17 00:00:00 2001 From: Umputun Date: Thu, 2 Apr 2026 17:59:43 -0500 Subject: [PATCH 7/8] fix: address Copilot review findings for launch script Fix comment to match actual title format ("rd:" not "revdiff:") and harden flag parsing to skip short flags and handle -o/--output consuming the next argument. Bump plugin version to 0.2.1. --- .claude-plugin/marketplace.json | 2 +- .claude-plugin/plugin.json | 2 +- .../skills/revdiff/scripts/launch-revdiff.sh | 11 +++++++++-- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 236a4d71..0d37d6bd 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -9,7 +9,7 @@ "name": "revdiff", "source": "./", "description": "Review git diffs with inline annotations in a TUI overlay", - "version": "0.2.0", + "version": "0.2.1", "author": { "name": "umputun" } diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index a5bc7683..7ce913c5 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "revdiff", - "version": "0.2.0", + "version": "0.2.1", "description": "Review git diffs with inline annotations in a TUI overlay", "author": { "name": "umputun", diff --git a/.claude-plugin/skills/revdiff/scripts/launch-revdiff.sh b/.claude-plugin/skills/revdiff/scripts/launch-revdiff.sh index d1fd8de1..b603d755 100755 --- a/.claude-plugin/skills/revdiff/scripts/launch-revdiff.sh +++ b/.claude-plugin/skills/revdiff/scripts/launch-revdiff.sh @@ -20,11 +20,18 @@ trap 'rm -f "$OUTPUT_FILE"' EXIT REVDIFF_CMD="$REVDIFF_BIN --output=$OUTPUT_FILE $*" CWD="$(pwd)" -# build descriptive title: "revdiff: dirname [ref]" +# build descriptive title: "rd: dirname [ref]" DIR_NAME=$(basename "$CWD") TITLE_REF="" +SKIP_NEXT=0 for arg in "$@"; do - case "$arg" in --*) ;; *) TITLE_REF="$arg"; break ;; esac + if [ "$SKIP_NEXT" -eq 1 ]; then SKIP_NEXT=0; continue; fi + case "$arg" in + -o|--output) SKIP_NEXT=1 ;; + --output=*) ;; + -*) ;; + *) TITLE_REF="$arg"; break ;; + esac done OVERLAY_TITLE="rd: ${DIR_NAME}${TITLE_REF:+ [$TITLE_REF]}" From a45eedbdd5b971f2011b716078f3fb65995c59bc Mon Sep 17 00:00:00 2001 From: Umputun Date: Thu, 2 Apr 2026 18:06:48 -0500 Subject: [PATCH 8/8] fix: truncate long filenames in tree pane to prevent selection wrapping Long filenames that exceed the tree pane width caused the selection highlight to wrap onto multiple lines, making it look like two entries were selected. Truncate from the left with ellipsis to fit within the pane width. --- ui/filetree.go | 38 +++++++++++++++++++++++++++----------- ui/filetree_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/ui/filetree.go b/ui/filetree.go index f7ea4101..9a2d8933 100644 --- a/ui/filetree.go +++ b/ui/filetree.go @@ -4,6 +4,8 @@ import ( "path/filepath" "sort" "strings" + + "github.com/charmbracelet/lipgloss" ) // fileTree manages the list of changed files grouped by directory. @@ -235,17 +237,7 @@ func (ft *fileTree) render(width, height int, annotatedFiles map[string]bool, s if e.isDir { line = s.DirEntry.Render(" " + ft.truncateDirName(e.name, width-3)) } else { - marker := " " - if annotatedFiles[e.path] { - marker = s.AnnotationMark.Render(" *") - } - name := indent + e.name + marker - - if idx == ft.cursor { - line = s.FileSelected.Width(width - 2).Render(name) - } else { - line = s.FileEntry.Render(name) - } + line = ft.renderFileEntry(e, idx, indent, width, annotatedFiles, s) } b.WriteString(line) @@ -256,6 +248,30 @@ func (ft *fileTree) render(width, height int, annotatedFiles map[string]bool, s return b.String() } +// renderFileEntry renders a single file entry in the tree, truncating long names to prevent wrapping. +func (ft *fileTree) renderFileEntry(e treeEntry, idx int, indent string, width int, annotatedFiles map[string]bool, s styles) string { + marker := " " + if annotatedFiles[e.path] { + marker = s.AnnotationMark.Render(" *") + } + name := indent + e.name + marker + maxWidth := width - 2 + + // truncate from the left of the filename when it exceeds pane width + if lipgloss.Width(name) > maxWidth && maxWidth > 4 { + budget := maxWidth - lipgloss.Width(indent) - lipgloss.Width(marker) - 1 // 1 for "…" + if budget > 0 && lipgloss.Width(e.name) > budget { + runes := []rune(e.name) + name = indent + "…" + string(runes[len(runes)-budget+1:]) + marker + } + } + + if idx == ft.cursor { + return s.FileSelected.Width(maxWidth).Render(name) + } + return s.FileEntry.Render(name) +} + // filterFiles returns the subset of allFiles that have annotations. func (ft *fileTree) filterFiles(annotatedFiles map[string]bool) []string { var filtered []string diff --git a/ui/filetree_test.go b/ui/filetree_test.go index c3112fea..165c2824 100644 --- a/ui/filetree_test.go +++ b/ui/filetree_test.go @@ -4,7 +4,9 @@ import ( "strings" "testing" + "github.com/charmbracelet/lipgloss" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestFileTree_BuildEntries(t *testing.T) { @@ -441,6 +443,28 @@ func TestFileTree_RenderTruncatesLongDirNames(t *testing.T) { assert.NotContains(t, result, ".claude-plugin/skills/revdiff/references/", "full dir name should not appear") } +func TestFileTree_RenderTruncatesLongFileNames(t *testing.T) { + files := []string{"docs/plans/completed/20260402-status-line-help-overlay.md"} + ft := newFileTree(files) + s := plainStyles() + + // narrow tree pane should truncate long filename with ellipsis + result := ft.render(30, 10, nil, s) + lines := strings.Split(result, "\n") + + // find the file entry line (not the dir entry) + var fileLine string + for _, l := range lines { + if strings.Contains(l, ".md") { + fileLine = l + break + } + } + require.NotEmpty(t, fileLine, "should find the .md file entry") + assert.Contains(t, fileLine, "…", "long filename should be truncated with ellipsis") + assert.LessOrEqual(t, lipgloss.Width(fileLine), 30, "file entry should not exceed pane width") +} + func TestFileTree_RenderViewportCursorAlwaysVisible(t *testing.T) { files := []string{ "cmd/main.go", "cmd/flags.go",