From 64c102fd1bd85296b259154bc520cd3d72c0a351 Mon Sep 17 00:00:00 2001 From: Queaxtra <60826916+Queaxtra@users.noreply.github.com> Date: Sun, 21 Jun 2026 22:44:16 +0300 Subject: [PATCH 01/10] feat: add git gutter colors to theme --- internal/theme/theme.go | 42 +++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/internal/theme/theme.go b/internal/theme/theme.go index bef1019..6c6d047 100644 --- a/internal/theme/theme.go +++ b/internal/theme/theme.go @@ -25,14 +25,19 @@ type Theme struct { LineHL tcell.Color // Active line highlight. // --- Foregrounds & accents --- - Text tcell.Color // Primary editor text. - Muted tcell.Color // Line numbers, inactive tabs, secondary UI text. - Subtle tcell.Color // Even more subtle (separators, hints). - Accent tcell.Color // Active tab accent, root label, important UI. - AccentSoft tcell.Color // Softer accent (active line number). - Selection tcell.Color // Selection background. - Modified tcell.Color // Dirty indicator (unsaved changes). - Error tcell.Color // Error messages. + Text tcell.Color // Primary editor text. + Muted tcell.Color // Line numbers, inactive tabs, secondary UI text. + Subtle tcell.Color // Even more subtle (separators, hints). + Accent tcell.Color // Active tab accent, root label, important UI. + AccentSoft tcell.Color // Softer accent (active line number). + Selection tcell.Color // Selection background. + Modified tcell.Color // Dirty indicator (unsaved changes). + Error tcell.Color // Error messages. + GitModified tcell.Color + GitAdded tcell.Color + GitDeleted tcell.Color + GitRenamed tcell.Color + GitMixed tcell.Color // FindMatch / FindCurrent paint search hits in the editor body. // FindMatch is a soft tint applied to every match in the viewport; @@ -72,14 +77,19 @@ func Default() Theme { LineHL: tcell.NewRGBColor(0x1f, 0x20, 0x2e), // Foregrounds & accents. - Text: tcell.NewRGBColor(0xc0, 0xca, 0xf5), - Muted: tcell.NewRGBColor(0x56, 0x5f, 0x89), - Subtle: tcell.NewRGBColor(0x32, 0x34, 0x4a), - Accent: tcell.NewRGBColor(0x7a, 0xa2, 0xf7), - AccentSoft: tcell.NewRGBColor(0xbb, 0x9a, 0xf7), - Selection: tcell.NewRGBColor(0x33, 0x46, 0x7c), - Modified: tcell.NewRGBColor(0xe0, 0xaf, 0x68), - Error: tcell.NewRGBColor(0xf7, 0x76, 0x8e), + Text: tcell.NewRGBColor(0xc0, 0xca, 0xf5), + Muted: tcell.NewRGBColor(0x56, 0x5f, 0x89), + Subtle: tcell.NewRGBColor(0x32, 0x34, 0x4a), + Accent: tcell.NewRGBColor(0x7a, 0xa2, 0xf7), + AccentSoft: tcell.NewRGBColor(0xbb, 0x9a, 0xf7), + Selection: tcell.NewRGBColor(0x33, 0x46, 0x7c), + Modified: tcell.NewRGBColor(0xe0, 0xaf, 0x68), + Error: tcell.NewRGBColor(0xf7, 0x76, 0x8e), + GitModified: tcell.NewRGBColor(0xff, 0x9e, 0x64), + GitAdded: tcell.NewRGBColor(0x9e, 0xce, 0x6a), + GitDeleted: tcell.NewRGBColor(0xf7, 0x76, 0x8e), + GitRenamed: tcell.NewRGBColor(0x7d, 0xcf, 0xf7), + GitMixed: tcell.NewRGBColor(0xbb, 0x9a, 0xf7), // Find. FindMatch is a desaturated amber so it reads as "all // hits" without competing with the syntax palette. FindCurrent From 9179567f68eb0bfb3c10d2b1ea2ab0b56739f55e Mon Sep 17 00:00:00 2001 From: Queaxtra <60826916+Queaxtra@users.noreply.github.com> Date: Sun, 21 Jun 2026 22:44:21 +0300 Subject: [PATCH 02/10] feat: add git line markers and highlight visible range --- internal/editor/highlight.go | 55 ++++++++++++++++++++++++++----- internal/editor/highlight_test.go | 22 +++++++++++++ 2 files changed, 68 insertions(+), 9 deletions(-) diff --git a/internal/editor/highlight.go b/internal/editor/highlight.go index 4082cff..0aa70ce 100644 --- a/internal/editor/highlight.go +++ b/internal/editor/highlight.go @@ -26,6 +26,37 @@ import ( // up the style for each cell it draws — at the cost of some memory. // For files small enough to comfortably review, that's a fine trade. func Highlight(filename, src string, t theme.Theme) [][]tcell.Style { + return highlightSource(filename, src, t) +} + +// HighlightVisible returns a style grid for the current viewport. Only visible +// rows are tokenised so keystroke cost follows terminal height, not file size. +func HighlightVisible(filename string, lines []string, startLine, height int, t theme.Theme) [][]tcell.Style { + styles := make([][]tcell.Style, len(lines)) + if height <= 0 || startLine >= len(lines) { + return styles + } + if startLine < 0 { + startLine = 0 + } + endLine := startLine + height + if endLine > len(lines) { + endLine = len(lines) + } + visible := strings.Join(lines[startLine:endLine], "\n") + visibleStyles := highlightSource(filename, visible, t) + for i, row := range visibleStyles { + lineIdx := startLine + i + if lineIdx >= endLine || lineIdx >= len(styles) { + break + } + styles[lineIdx] = row + } + return styles +} + +// highlightSource tokenises src and returns one style row per source line. +func highlightSource(filename, src string, t theme.Theme) [][]tcell.Style { lexer := lexers.Match(filename) if lexer == nil { lexer = lexers.Analyse(src) @@ -41,15 +72,7 @@ func Highlight(filename, src string, t theme.Theme) [][]tcell.Style { // Pre-allocate a styles grid sized to the source. We seed every cell // with the base style so untokenised runes still render readably. lines := strings.Split(src, "\n") - styles := make([][]tcell.Style, len(lines)) - for i, ln := range lines { - runes := []rune(ln) - row := make([]tcell.Style, len(runes)) - for j := range row { - row[j] = base - } - styles[i] = row - } + styles := baseStyleGrid(lines, base) iter, err := lexer.Tokenise(nil, src) if err != nil { @@ -74,6 +97,20 @@ func Highlight(filename, src string, t theme.Theme) [][]tcell.Style { return styles } +// baseStyleGrid returns a correctly shaped grid pre-filled with base. +func baseStyleGrid(lines []string, base tcell.Style) [][]tcell.Style { + styles := make([][]tcell.Style, len(lines)) + for i, ln := range lines { + runes := []rune(ln) + row := make([]tcell.Style, len(runes)) + for j := range row { + row[j] = base + } + styles[i] = row + } + return styles +} + // styleForToken maps a Chroma token type to a tcell.Style using the active // theme. We match by category first (Keyword, LiteralString, etc.) so the // mapping stays tight across the dozens of language-specific subtypes. diff --git a/internal/editor/highlight_test.go b/internal/editor/highlight_test.go index cf0938e..946e2e7 100644 --- a/internal/editor/highlight_test.go +++ b/internal/editor/highlight_test.go @@ -160,3 +160,25 @@ func (f *Foo) Bar() string { } } } + +// TestHighlightVisible_LimitsTokenisingToViewport pins the fast path: +// off-screen rows stay empty while visible rows are tokenised. +func TestHighlightVisible_LimitsTokenisingToViewport(t *testing.T) { + th := theme.Default() + lines := make([]string, 20) + for i := range lines { + lines[i] = "package main" + } + + got := HighlightVisible("main.go", lines, 10, 2, th) + if len(got) != len(lines) { + t.Fatalf("rows = %d, want %d", len(got), len(lines)) + } + if got[0] != nil { + t.Fatalf("off-screen row was highlighted, got %v", got[0]) + } + base := tcell.StyleDefault.Background(th.BG).Foreground(th.Text) + if got[10][0] == base { + t.Fatal("visible row was not highlighted") + } +} From e5cd8ce027ec53c7ecb4d43d19863bb20c3af830 Mon Sep 17 00:00:00 2001 From: Queaxtra <60826916+Queaxtra@users.noreply.github.com> Date: Sun, 21 Jun 2026 22:44:21 +0300 Subject: [PATCH 03/10] feat: add git line changes to tab state --- internal/editor/tab.go | 44 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/internal/editor/tab.go b/internal/editor/tab.go index 22e5518..f6940a4 100644 --- a/internal/editor/tab.go +++ b/internal/editor/tab.go @@ -24,11 +24,21 @@ import ( // pad on the right — comfortable for files of any realistic length. const gutterWidth = 6 +// GitLineChange describes the marker rendered in the editor gutter for a line. +type GitLineChange int + +const ( + GitLineNone GitLineChange = iota + GitLineModified + GitLineAdded + GitLineDeleted +) + // Tab is a single open file. It owns the on-disk path, the in-memory buffer, // the per-tab view state (scroll position, cursor, selection anchor), the // cached syntax-highlight styles, and a dirty flag. type Tab struct { - Path string // Empty for an unsaved/scratch tab. + Path string // Empty for an unsaved/scratch tab. Buffer *Buffer Cursor Position // Where new typed text appears. Anchor Position // Selection anchor; equals Cursor when nothing is selected. @@ -37,6 +47,7 @@ type Tab struct { Dirty bool Styles [][]tcell.Style StyleStale bool + GitLines map[int]GitLineChange // Mtime is the file's modification time as of the last successful // read or write. The app's periodic disk-reconcile loop compares it @@ -506,10 +517,6 @@ func (t *Tab) Render(scr tcell.Screen, th theme.Theme, x, y, w, h int) { t.renderImage(scr, th, x, y, w, h) return } - if t.StyleStale { - t.Styles = Highlight(t.Path, t.Buffer.String(), th) - t.StyleStale = false - } // Only re-center on the cursor if the cursor moved this tick. Doing it // every render fights the user when they scroll with the wheel. if t.cursorMoved { @@ -517,6 +524,8 @@ func (t *Tab) Render(scr tcell.Screen, th theme.Theme, x, y, w, h int) { t.cursorMoved = false } t.clampScroll(h) + t.Styles = HighlightVisible(t.Path, t.Buffer.Lines, t.ScrollY, h, th) + t.StyleStale = false bg := th.BG bgStyle := tcell.StyleDefault.Background(bg).Foreground(th.Text) @@ -565,7 +574,13 @@ func (t *Tab) Render(scr tcell.Screen, th theme.Theme, x, y, w, h int) { if isCursorLine { gutterStyle = gutterStyle.Foreground(th.AccentSoft) } + if marker, ok := t.GitLines[lineIdx]; ok && marker != GitLineNone { + scr.SetContent(x, cy, gitLineMarkerRune(marker), nil, gutterStyle.Foreground(gitLineMarkerColor(th, marker))) + } for i, r := range numStr { + if i == 0 && t.GitLines[lineIdx] != GitLineNone { + continue + } scr.SetContent(x+i, cy, r, nil, gutterStyle) } @@ -659,6 +674,25 @@ func (t *Tab) Render(scr tcell.Screen, th theme.Theme, x, y, w, h int) { } } +// gitLineMarkerRune returns the gutter glyph for a git line change. +func gitLineMarkerRune(change GitLineChange) rune { + if change == GitLineDeleted { + return '▁' + } + return '▌' +} + +// gitLineMarkerColor returns the gutter color for a git line change. +func gitLineMarkerColor(th theme.Theme, change GitLineChange) tcell.Color { + if change == GitLineAdded { + return th.GitAdded + } + if change == GitLineDeleted { + return th.GitDeleted + } + return th.GitModified +} + // HitTest converts screen coordinates within this tab's render area to a // buffer position. ok=false means the click was outside any line. func (t *Tab) HitTest(localX, localY, w, h int) (Position, bool) { From 8d57438dda67d0e224f2c1849f7b7aa2ff36e272 Mon Sep 17 00:00:00 2001 From: Queaxtra <60826916+Queaxtra@users.noreply.github.com> Date: Sun, 21 Jun 2026 22:44:24 +0300 Subject: [PATCH 04/10] feat: add git change kind tracking to filetree --- internal/filetree/filetree.go | 188 +++++++++++++++-- internal/filetree/filetree_test.go | 318 ++++++++++++++++++++++++++++- 2 files changed, 478 insertions(+), 28 deletions(-) diff --git a/internal/filetree/filetree.go b/internal/filetree/filetree.go index 92a14ed..6674597 100644 --- a/internal/filetree/filetree.go +++ b/internal/filetree/filetree.go @@ -35,6 +35,18 @@ type Node struct { Children []*Node } +// GitChangeKind describes the strongest git status a tree row should show. +type GitChangeKind int + +const ( + GitChangeNone GitChangeKind = iota + GitChangeModified + GitChangeAdded + GitChangeDeleted + GitChangeRenamed + GitChangeMixed +) + // Tree owns the root node and the most recently rendered flat list of // visible rows. Click hit-testing maps a screen row index back to the Node // drawn at that row. @@ -49,6 +61,7 @@ type Tree struct { // always visible. The app updates this whenever the user clicks a // tree node or opens a file. ActiveFolder string + ActiveFile string // DirtyFiles and DirtyFolders carry the project's git status — both // indexed by absolute path. Files in DirtyFiles render in the theme's @@ -56,8 +69,8 @@ type Tree struct { // branch still signals there's a change inside. Both maps are nil // when the project isn't a git repo or when git status hasn't been // loaded yet, and the renderer treats nil as "everything clean". - DirtyFiles map[string]bool - DirtyFolders map[string]bool + DirtyFiles map[string]GitChangeKind + DirtyFolders map[string]GitChangeKind // IconsEnabled toggles the Nerd Font glyph that prefixes each row. // Set by App.loadSpiceConfig at startup based on the user's @@ -224,6 +237,9 @@ func (t *Tree) Render(scr tcell.Screen, th theme.Theme, x, y, w, h int) { if rootActive { rootStyle = tcell.StyleDefault.Background(bg).Foreground(th.Accent).Bold(true) } + if rootChange := t.DirtyFolders[t.Root.Path]; rootChange != GitChangeNone { + rootStyle = rootStyle.Foreground(gitChangeColor(th, rootChange)) + } drawString(scr, x, y+1, w, " "+t.Root.Name, rootStyle) // Build the flat list of visible rows from the root's children. @@ -247,21 +263,18 @@ func (t *Tree) Render(scr tcell.Screen, th theme.Theme, x, y, w, h int) { continue } item := flat[idx] - active := item.Node.IsDir && item.Node.Path == t.ActiveFolder - dirty := t.isDirty(item.Node) - drawNodeRow(scr, th, x, listTop+row, w, item, active, dirty, t.IconsEnabled) + active := item.Node.Path == t.ActiveFile || (item.Node.IsDir && item.Node.Path == t.ActiveFolder) + change := t.changeKind(item.Node) + drawNodeRow(scr, th, x, listTop+row, w, item, active, change, t.IconsEnabled) visible = append(visible, item.Node) } t.visible = visible } -// isDirty reports whether a node should render in the Modified color — -// either because the file itself has uncommitted changes or because a -// folder somewhere below it does. Returns false for any node when git -// status hasn't been loaded. -func (t *Tree) isDirty(n *Node) bool { +// changeKind returns the git status color category for a tree node. +func (t *Tree) changeKind(n *Node) GitChangeKind { if n == nil { - return false + return GitChangeNone } if n.IsDir { return t.DirtyFolders[n.Path] @@ -270,12 +283,9 @@ func (t *Tree) isDirty(n *Node) bool { } // drawNodeRow renders one tree row with proper indent, chevron, and color. -// active=true marks this folder as the editor's current working folder -// (the New File default), and is drawn bold + accent-tinted so the user -// can see at a glance where the next "New file" will land. dirty=true -// marks the node as having uncommitted git changes (or, for folders, -// containing some) — it overrides the normal foreground with the -// theme's Modified color so changed files stand out at a glance. +// active=true marks the active file or current working folder. change marks +// uncommitted git status and overrides the normal foreground so changed names +// stand out in the tree like other modern editors. // withIcons=true prefixes the name with a Nerd Font glyph + space; off // renders the legacy chevron-only look for terminals that can't show // the private-use glyphs. @@ -286,7 +296,7 @@ func (t *Tree) isDirty(n *Node) bool { // styling. That's the visual cue you find in nvim-tree and friends: // a quick eye-scan picks out Go from Ruby from Markdown without // reading any text. -func drawNodeRow(scr tcell.Screen, th theme.Theme, x, y, w int, item flatNode, active, dirty, withIcons bool) { +func drawNodeRow(scr tcell.Screen, th theme.Theme, x, y, w int, item flatNode, active bool, change GitChangeKind, withIcons bool) { bg := th.SidebarBG indent := strings.Repeat(" ", item.Depth) @@ -314,8 +324,8 @@ func drawNodeRow(scr tcell.Screen, th theme.Theme, x, y, w int, item flatNode, a if active { fg = th.Accent } - if dirty { - fg = th.Modified + if change != GitChangeNone { + fg = gitChangeColor(th, change) } rowStyle := tcell.StyleDefault.Background(bg).Foreground(fg) if active { @@ -360,6 +370,23 @@ func drawNodeRow(scr tcell.Screen, th theme.Theme, x, y, w int, item flatNode, a drawString(scr, x+px+gx, y, w-px-gx, " "+suffix, rowStyle) } +// gitChangeColor maps git status kinds to the tree row foreground. +func gitChangeColor(th theme.Theme, change GitChangeKind) tcell.Color { + switch change { + case GitChangeAdded: + return th.GitAdded + case GitChangeDeleted: + return th.GitDeleted + case GitChangeRenamed: + return th.GitRenamed + case GitChangeMixed: + return th.GitMixed + case GitChangeModified: + return th.GitModified + } + return th.FileColor +} + // drawString writes s left-aligned within [x, x+w). Excess content is // truncated; short content is implicitly padded by the row's pre-painted bg. func drawString(scr tcell.Screen, x, y, w int, s string, st tcell.Style) { @@ -435,3 +462,124 @@ func (t *Tree) Scroll(delta int) { t.ScrollY = 0 } } + +// Reveal expands every directory from the tree root down to path's parent so +// the file becomes visible in the sidebar, then scrolls the viewport so the +// row lands on screen. Opening a file via the finder (Esc-p) or the command +// line lands on a path whose ancestors are still collapsed — without this, +// the active-file highlight is set but the row itself is invisible, leaving +// the sidebar out of sync with the editor like a tab with no tab bar entry. +// +// When the target row is already inside the current viewport the scroll +// position is left untouched, so clicking a visible row in the tree (which +// also routes through openFile) doesn't snap it to the top. +// +// No-op when path isn't under the root, escapes it, or lives inside a hidden +// directory the tree refuses to show (e.g. .git). viewH is the row count the +// renderer will hand Render's list area; pass 0 to expand ancestors without +// scrolling (used when the sidebar is hidden). +func (t *Tree) Reveal(path string, viewH int) { + if t.Root == nil { + return + } + abs, err := filepath.Abs(path) + if err != nil { + return + } + rel, err := filepath.Rel(t.Root.Path, abs) + if err != nil { + return + } + if rel == "." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) || rel == ".." { + return + } + parts := strings.Split(filepath.ToSlash(rel), "/") + + // Walk every directory component, lazily loading + expanding each so the + // next step can descend into it. The final component is the target row + // itself; it doesn't need expanding — revealing is about visibility, not + // auto-opening directories. + n := t.Root + for i := 0; i < len(parts)-1; i++ { + if !n.Loaded { + _ = loadChildren(n) + } + child := childByName(n, parts[i]) + if child == nil { + return // hidden or gone — can't descend further + } + if !child.Expanded { + child.Expanded = true + if !child.Loaded { + _ = loadChildren(child) + } + } + n = child + } + + // Find the target row among its parent's children so we can scroll to it. + if !n.Loaded { + _ = loadChildren(n) + } + target := childByName(n, parts[len(parts)-1]) + if target == nil { + return + } + + idx := t.flatIndexOf(target) + if idx < 0 { + return + } + if viewH <= 0 { + return + } + // Leave the viewport alone when the row is already on screen — a click on + // a visible row shouldn't snap it to the top. + if idx >= t.ScrollY && idx < t.ScrollY+viewH { + return + } + t.ScrollY = idx +} + +// flatIndexOf returns the row index of target in the renderer's flat list +// (the same pre-order walk Render builds via flattenInto), or -1 when target +// isn't currently visible. Mirrors the render order exactly so the index we +// scroll to is the row the user actually sees. +func (t *Tree) flatIndexOf(target *Node) int { + idx := 0 + var walk func(n *Node) bool + walk = func(n *Node) bool { + if n == target { + return true + } + idx++ + if n.IsDir && n.Expanded { + for _, c := range n.Children { + if walk(c) { + return true + } + } + } + return false + } + for _, c := range t.Root.Children { + if walk(c) { + return idx + } + } + return -1 +} + +// childByName returns the direct child of n named name, or nil when no such +// child exists. Reveal uses it to descend the path component by component. +func childByName(n *Node, name string) *Node { + if n == nil { + return nil + } + for _, c := range n.Children { + if c.Name == name { + return c + } + } + return nil +} diff --git a/internal/filetree/filetree_test.go b/internal/filetree/filetree_test.go index f601205..9caf16f 100644 --- a/internal/filetree/filetree_test.go +++ b/internal/filetree/filetree_test.go @@ -500,6 +500,31 @@ func TestRender_ActiveFolderIsBold(t *testing.T) { } } +// TestRender_ActiveFileIsBold verifies the open file itself is visible in the tree. +func TestRender_ActiveFileIsBold(t *testing.T) { + root := mkTree(t) + tr, err := New(root) + if err != nil { + t.Fatalf("New: %v", err) + } + alpha := findChild(tr.Root, "alpha") + if err := alpha.reload(); err != nil { + t.Fatalf("reload alpha: %v", err) + } + alpha.Expanded = true + inner := findChild(alpha, "inner.go") + tr.ActiveFile = inner.Path + + cells, w := renderAndCollect(t, tr, 40, 20) + rowY := findRowY(cells, w, 20, "inner.go") + if rowY < 0 { + t.Fatal("could not find active file row") + } + if !rowHasBold(cells, w, rowY) { + t.Fatal("active file row should be bold") + } +} + // TestRender_TinyHeightDoesNotPanic guards against an off-by-one when the // caller hands Render a height smaller than the 2-row header — listH goes // to zero and we shouldn't blow up dividing or indexing. @@ -541,14 +566,14 @@ func TestRender_DirtyFileUsesModifiedColor(t *testing.T) { if inner == nil { t.Fatal("alpha/inner.go missing from fixture") } - tr.DirtyFiles = map[string]bool{inner.Path: true} + tr.DirtyFiles = map[string]GitChangeKind{inner.Path: GitChangeModified} cells, w := renderAndCollect(t, tr, 40, 20) rowY := findRowY(cells, w, 20, "inner.go") if rowY < 0 { t.Fatal("could not find inner.go row in render output") } - if !rowHasColor(cells, w, rowY, theme.Default().Modified) { + if !rowHasColor(cells, w, rowY, theme.Default().GitModified) { t.Fatalf("expected inner.go row to be drawn in Modified color") } } @@ -564,18 +589,34 @@ func TestRender_DirtyFolderUsesModifiedColor(t *testing.T) { t.Fatalf("New: %v", err) } alpha := findChild(tr.Root, "alpha") - tr.DirtyFolders = map[string]bool{alpha.Path: true} + tr.DirtyFolders = map[string]GitChangeKind{alpha.Path: GitChangeModified} cells, w := renderAndCollect(t, tr, 40, 20) rowY := findRowY(cells, w, 20, "alpha") if rowY < 0 { t.Fatal("could not find alpha row in render output") } - if !rowHasColor(cells, w, rowY, theme.Default().Modified) { + if !rowHasColor(cells, w, rowY, theme.Default().GitModified) { t.Fatal("expected alpha folder row to be drawn in Modified color") } } +// TestRender_DirtyRootUsesModifiedColor ensures the project name itself +// reflects git changes when any descendant is dirty. +func TestRender_DirtyRootUsesModifiedColor(t *testing.T) { + root := mkTree(t) + tr, err := New(root) + if err != nil { + t.Fatalf("New: %v", err) + } + tr.DirtyFolders = map[string]GitChangeKind{tr.Root.Path: GitChangeModified} + + cells, w := renderAndCollect(t, tr, 40, 20) + if !rowHasColor(cells, w, 1, theme.Default().GitModified) { + t.Fatal("expected root project row to be drawn in Modified color") + } +} + // TestRender_DirtyAndActiveStaysBold confirms that the active-folder // styling (bold) and the dirty-folder styling (Modified colour) compose // cleanly — the user shouldn't lose the "current target" cue just @@ -588,14 +629,14 @@ func TestRender_DirtyAndActiveStaysBold(t *testing.T) { } alpha := findChild(tr.Root, "alpha") tr.ActiveFolder = alpha.Path - tr.DirtyFolders = map[string]bool{alpha.Path: true} + tr.DirtyFolders = map[string]GitChangeKind{alpha.Path: GitChangeModified} cells, w := renderAndCollect(t, tr, 40, 20) rowY := findRowY(cells, w, 20, "alpha") if rowY < 0 { t.Fatal("could not find alpha row") } - if !rowHasColor(cells, w, rowY, theme.Default().Modified) { + if !rowHasColor(cells, w, rowY, theme.Default().GitModified) { t.Error("expected alpha row to be Modified colour") } if !rowHasBold(cells, w, rowY) { @@ -791,14 +832,14 @@ func TestRender_DirtyOverridesDotMute(t *testing.T) { if err != nil { t.Fatalf("New: %v", err) } - tr.DirtyFiles = map[string]bool{envPath: true} + tr.DirtyFiles = map[string]GitChangeKind{envPath: GitChangeModified} cells, w := renderAndCollect(t, tr, 40, 20) rowY := findRowY(cells, w, 20, ".env") if rowY < 0 { t.Fatal("could not find .env row") } - if !rowHasColor(cells, w, rowY, theme.Default().Modified) { + if !rowHasColor(cells, w, rowY, theme.Default().GitModified) { t.Fatalf("dirty .env should override Muted with Modified, got %q", rowText(cells, w, rowY)) } @@ -886,3 +927,264 @@ func TestRender_IconsEnabledFolderOpenSwitches(t *testing.T) { t.Fatalf("expanded alpha row missing FolderOpen: %q", expanded) } } + +// mkNested builds a deeper layout than mkTree so Reveal has a real ancestor +// chain to walk. The shape: +// +// root/ +// a/ +// b/ +// deep.go +// other.go +// top.go +// zeta.txt +// ... +// +// The top-level files give the flat list enough rows for the scroll tests +// to have a target that genuinely sits below the viewport. +func mkNested(t *testing.T) string { + t.Helper() + root := t.TempDir() + mustMkdir(t, filepath.Join(root, "a")) + mustMkdir(t, filepath.Join(root, "a", "b")) + mustWrite(t, filepath.Join(root, "a", "b", "deep.go"), "x") + mustWrite(t, filepath.Join(root, "a", "b", "other.go"), "x") + mustWrite(t, filepath.Join(root, "top.go"), "x") + mustWrite(t, filepath.Join(root, "zeta.txt"), "x") + mustWrite(t, filepath.Join(root, "Apple.md"), "x") + return root +} + +// TestReveal_ExpandsAncestorsAndScrolls is the headline case: a file buried +// two directories deep is invisible until Reveal walks the chain, lazily +// loads + expands each ancestor, and brings the row into the viewport. +// Without this the finder would open the file and the sidebar would still +// show a collapsed "a/" — the bug the feature exists to fix. With a large +// viewport the row lands in view purely from the expansion, so the real +// contract being pinned here is "ancestors expanded AND row on screen". +func TestReveal_ExpandsAncestorsAndScrolls(t *testing.T) { + root := mkNested(t) + tr, err := New(root) + if err != nil { + t.Fatalf("New: %v", err) + } + deep := filepath.Join(root, "a", "b", "deep.go") + const viewH = 20 + + tr.Reveal(deep, viewH) + + a := findChild(tr.Root, "a") + if a == nil { + t.Fatal("a missing") + } + if !a.Expanded || !a.Loaded { + t.Fatalf("a should be expanded+loaded after reveal: %+v", a) + } + b := findChild(a, "b") + if b == nil { + t.Fatal("b missing") + } + if !b.Expanded || !b.Loaded { + t.Fatalf("b should be expanded+loaded after reveal: %+v", b) + } + deepNode := findChild(b, "deep.go") + if deepNode == nil { + t.Fatal("deep.go missing after reveal") + } + wantIdx := tr.flatIndexOf(deepNode) + if wantIdx < 0 { + t.Fatal("deep.go should be in the flat list after ancestors expanded") + } + if wantIdx < tr.ScrollY || wantIdx >= tr.ScrollY+viewH { + t.Fatalf("deep.go (idx %d) should be inside viewport [ScrollY=%d, +%d)", wantIdx, tr.ScrollY, viewH) + } +} + +// TestReveal_NoScrollWhenAlreadyVisible guards the click path: when the row +// is already on screen Reveal must leave ScrollY alone, otherwise clicking a +// visible row would snap it to the top — a surprising jump the user didn't +// ask for. +func TestReveal_NoScrollWhenAlreadyVisible(t *testing.T) { + root := mkNested(t) + tr, err := New(root) + if err != nil { + t.Fatalf("New: %v", err) + } + // Open a/b so deep.go is in the flat list, then park the viewport so + // deep.go is the second visible row. + a := findChild(tr.Root, "a") + tr.Toggle(a) + b := findChild(a, "b") + tr.Toggle(b) + deepNode := findChild(b, "deep.go") + idx := tr.flatIndexOf(deepNode) + tr.ScrollY = idx - 1 // deep.go one row into the viewport + + deep := filepath.Join(root, "a", "b", "deep.go") + tr.Reveal(deep, 10) + + if tr.ScrollY != idx-1 { + t.Fatalf("ScrollY should be unchanged when target is visible: got %d, want %d", tr.ScrollY, idx-1) + } +} + +// TestReveal_ScrollsWhenTargetBelowViewport checks the inverse: a target +// below the current viewport must move ScrollY so the row lands on screen. +// Without this the reveal would be a no-op for files the user scrolled past. +func TestReveal_ScrollsWhenTargetBelowViewport(t *testing.T) { + root := mkNested(t) + tr, err := New(root) + if err != nil { + t.Fatalf("New: %v", err) + } + // Open the nested chain so the flat list has the deep row at a + // non-zero index, then keep the viewport pinned at the top. + a := findChild(tr.Root, "a") + tr.Toggle(a) + b := findChild(a, "b") + tr.Toggle(b) + deepNode := findChild(b, "deep.go") + wantIdx := tr.flatIndexOf(deepNode) + tr.ScrollY = 0 + + deep := filepath.Join(root, "a", "b", "deep.go") + tr.Reveal(deep, 2) // tiny viewport so the target is well below it + + if tr.ScrollY != wantIdx { + t.Fatalf("ScrollY: got %d, want %d", tr.ScrollY, wantIdx) + } +} + +// TestReveal_DirectChildOfRoot covers the no-ancestor case: a file sitting +// directly under the root has no directories to expand, but Reveal should +// still scroll to it when it's off-screen. +func TestReveal_DirectChildOfRoot(t *testing.T) { + root := mkNested(t) + tr, err := New(root) + if err != nil { + t.Fatalf("New: %v", err) + } + zeta := findChild(tr.Root, "zeta.txt") + if zeta == nil { + t.Fatal("zeta.txt missing") + } + // Park the viewport below zeta.txt so it's off-screen, then reveal. + wantIdx := tr.flatIndexOf(zeta) + tr.ScrollY = wantIdx + 5 + + tr.Reveal(filepath.Join(root, "zeta.txt"), 2) + + if tr.ScrollY != wantIdx { + t.Fatalf("ScrollY: got %d, want %d", tr.ScrollY, wantIdx) + } +} + +// TestReveal_ViewHZeroExpandsButDoesNotScroll pins the sidebar-hidden +// contract: when viewH is 0 there's no viewport to scroll, but ancestors +// should still be expanded so the tree is correct the next time the sidebar +// is shown. +func TestReveal_ViewHZeroExpandsButDoesNotScroll(t *testing.T) { + root := mkNested(t) + tr, err := New(root) + if err != nil { + t.Fatalf("New: %v", err) + } + deep := filepath.Join(root, "a", "b", "deep.go") + + tr.Reveal(deep, 0) + + a := findChild(tr.Root, "a") + b := findChild(a, "b") + if !a.Expanded || !b.Expanded { + t.Fatalf("ancestors should still expand with viewH=0: a=%+v b=%+v", a, b) + } + if tr.ScrollY != 0 { + t.Fatalf("ScrollY should not change with viewH=0: got %d", tr.ScrollY) + } +} + +// TestReveal_HiddenDirIsNoop verifies Reveal gives up on paths that pass +// through a filtered directory. .git is in the hide list, so a file under it +// has no reachable ancestor — Reveal must bail without expanding anything +// and without touching ScrollY. +func TestReveal_HiddenDirIsNoop(t *testing.T) { + root := t.TempDir() + mustMkdir(t, filepath.Join(root, ".git")) + mustWrite(t, filepath.Join(root, ".git", "config"), "x") + mustWrite(t, filepath.Join(root, "visible.go"), "x") + tr, err := New(root) + if err != nil { + t.Fatalf("New: %v", err) + } + scrollBefore := tr.ScrollY + + tr.Reveal(filepath.Join(root, ".git", "config"), 10) + + if tr.ScrollY != scrollBefore { + t.Fatalf("ScrollY should not change for hidden path: was %d now %d", scrollBefore, tr.ScrollY) + } +} + +// TestReveal_OutsideRootIsNoop guards against a path that isn't under the +// tree at all. filepath.Rel yields a ".." prefix in that case; Reveal must +// return without mutating the tree. +func TestReveal_OutsideRootIsNoop(t *testing.T) { + root := mkNested(t) + tr, err := New(root) + if err != nil { + t.Fatalf("New: %v", err) + } + outside := filepath.Join(t.TempDir(), "unrelated.go") + mustWrite(t, outside, "x") + scrollBefore := tr.ScrollY + + tr.Reveal(outside, 10) + + if tr.ScrollY != scrollBefore { + t.Fatalf("ScrollY should not change for outside path: was %d now %d", scrollBefore, tr.ScrollY) + } +} + +// TestReveal_RootItselfIsNoop pins the degenerate "reveal the root" case: +// filepath.Rel(root, root) == ".", which Reveal treats as nothing to do. +func TestReveal_RootItselfIsNoop(t *testing.T) { + root := mkNested(t) + tr, err := New(root) + if err != nil { + t.Fatalf("New: %v", err) + } + scrollBefore := tr.ScrollY + + tr.Reveal(root, 10) + + if tr.ScrollY != scrollBefore { + t.Fatalf("ScrollY should not change for root path: was %d now %d", scrollBefore, tr.ScrollY) + } +} + +// TestFlatIndexOf_MatchesRenderOrder cross-checks the helper against the +// real flattenInto walk: every node's index from flatIndexOf must agree with +// its position in flattenInto's output. A drift here would scroll to the +// wrong row. +func TestFlatIndexOf_MatchesRenderOrder(t *testing.T) { + root := mkNested(t) + tr, err := New(root) + if err != nil { + t.Fatalf("New: %v", err) + } + // Expand the chain so the nested rows are in the flat list. + a := findChild(tr.Root, "a") + tr.Toggle(a) + b := findChild(a, "b") + tr.Toggle(b) + + var flat []flatNode + for _, c := range tr.Root.Children { + flattenInto(c, 0, &flat) + } + for i, fn := range flat { + if got := tr.flatIndexOf(fn.Node); got != i { + t.Fatalf("flatIndexOf(%s): got %d, want %d", fn.Node.Name, got, i) + } + } +} From b6aa50864db414f4ee132ffd9fa9b13ca30db449 Mon Sep 17 00:00:00 2001 From: Queaxtra <60826916+Queaxtra@users.noreply.github.com> Date: Sun, 21 Jun 2026 22:44:24 +0300 Subject: [PATCH 05/10] feat: add git gutter data loading and diff parsing --- internal/app/gitstatus.go | 215 +++++++++++++++++++++++++++++++-- internal/app/gitstatus_test.go | 156 +++++++++++++++++++----- 2 files changed, 327 insertions(+), 44 deletions(-) diff --git a/internal/app/gitstatus.go b/internal/app/gitstatus.go index 49366da..13a4e04 100644 --- a/internal/app/gitstatus.go +++ b/internal/app/gitstatus.go @@ -25,6 +25,9 @@ import ( "path/filepath" "strconv" "strings" + + "github.com/cloudmanic/spice-edit/internal/editor" + "github.com/cloudmanic/spice-edit/internal/filetree" ) // gitStatus is the snapshot of a single git status run. IsRepo distinguishes @@ -35,7 +38,8 @@ import ( // detached, or "" when we aren't in a repo. type gitStatus struct { IsRepo bool - DirtyFiles map[string]bool + Root string + DirtyFiles map[string]filetree.GitChangeKind Branch string } @@ -67,11 +71,45 @@ func loadGitStatus(rootDir string) gitStatus { // We *are* in a repo (rev-parse succeeded) but couldn't read // status. Mark the result as a repo with no known dirty files // so the caller at least knows we tried. - return gitStatus{IsRepo: true, DirtyFiles: map[string]bool{}, Branch: loadGitBranch(rootDir)} + return gitStatus{IsRepo: true, Root: toplevel, DirtyFiles: map[string]filetree.GitChangeKind{}, Branch: loadGitBranch(rootDir)} } dirty := parsePorcelain(out, toplevel) - return gitStatus{IsRepo: true, DirtyFiles: dirty, Branch: loadGitBranch(rootDir)} + return gitStatus{IsRepo: true, Root: toplevel, DirtyFiles: dirty, Branch: loadGitBranch(rootDir)} +} + +// rebaseGitPaths rewrites dirty paths to match the file tree root casing. +func rebaseGitPaths(paths map[string]filetree.GitChangeKind, treeRoot string) map[string]filetree.GitChangeKind { + if len(paths) == 0 || treeRoot == "" { + return paths + } + rebased := map[string]filetree.GitChangeKind{} + for path, kind := range paths { + rel, ok := relFromRoot(path, treeRoot) + if !ok { + rebased[path] = kind + continue + } + rebased[filepath.Join(treeRoot, rel)] = kind + } + return rebased +} + +// relFromRoot returns path relative to root, tolerating macOS path casing drift. +func relFromRoot(path, root string) (string, bool) { + if rel, err := filepath.Rel(root, path); err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return rel, true + } + root = filepath.Clean(root) + path = filepath.Clean(path) + if strings.EqualFold(path, root) { + return ".", true + } + prefix := root + string(filepath.Separator) + if len(path) > len(prefix) && strings.EqualFold(path[:len(prefix)], prefix) { + return path[len(prefix):], true + } + return "", false } // loadGitBranch returns the current branch name for rootDir, or a short @@ -109,13 +147,14 @@ func loadGitBranch(rootDir string) string { // // We treat any line as dirty regardless of the X/Y status codes; for renames // we mark both the old and new paths so the user sees both rows tinted. -func parsePorcelain(out []byte, toplevel string) map[string]bool { - dirty := map[string]bool{} +func parsePorcelain(out []byte, toplevel string) map[string]filetree.GitChangeKind { + dirty := map[string]filetree.GitChangeKind{} for _, raw := range bytes.Split(out, []byte{'\n'}) { line := string(raw) if len(line) < 4 { continue } + kind := porcelainKind(line[:2]) // Drop the two status chars + the separating space. body := line[3:] @@ -123,10 +162,10 @@ func parsePorcelain(out []byte, toplevel string) map[string]bool { oldPath := unquotePath(body[:idx]) newPath := unquotePath(body[idx+len(" -> "):]) if oldPath != "" { - dirty[filepath.Join(toplevel, oldPath)] = true + dirty[filepath.Join(toplevel, oldPath)] = filetree.GitChangeDeleted } if newPath != "" { - dirty[filepath.Join(toplevel, newPath)] = true + dirty[filepath.Join(toplevel, newPath)] = filetree.GitChangeRenamed } continue } @@ -135,11 +174,25 @@ func parsePorcelain(out []byte, toplevel string) map[string]bool { if path == "" { continue } - dirty[filepath.Join(toplevel, path)] = true + dirty[filepath.Join(toplevel, path)] = kind } return dirty } +// porcelainKind maps git porcelain's XY status pair to the tree status kind. +func porcelainKind(code string) filetree.GitChangeKind { + if strings.Contains(code, "?") || strings.Contains(code, "A") { + return filetree.GitChangeAdded + } + if strings.Contains(code, "D") { + return filetree.GitChangeDeleted + } + if strings.Contains(code, "R") || strings.Contains(code, "C") { + return filetree.GitChangeRenamed + } + return filetree.GitChangeModified +} + // unquotePath undoes git's C-style quoting (enabled by default via // core.quotePath) so paths with spaces, unicode, or control chars come // back as a normal Go string. Falls back to the raw input on any parse @@ -162,13 +215,13 @@ func unquotePath(s string) string { // folder under root. A folder is "dirty" if any of its descendants are // dirty, so collapsed branches still signal that there's something // changed inside. -func dirtyFolderSet(dirtyFiles map[string]bool, root string) map[string]bool { - folders := map[string]bool{} +func dirtyFolderSet(dirtyFiles map[string]filetree.GitChangeKind, root string) map[string]filetree.GitChangeKind { + folders := map[string]filetree.GitChangeKind{} if len(dirtyFiles) == 0 { return folders } root = filepath.Clean(root) - for path := range dirtyFiles { + for path, kind := range dirtyFiles { // Walk up from each dirty file's parent toward the root, // marking every ancestor inside the project. The walk halts // the moment we step outside root so a file outside the @@ -177,10 +230,14 @@ func dirtyFolderSet(dirtyFiles map[string]bool, root string) map[string]bool { if !pathInside(p, root) { break } - if folders[p] { + if folders[p] == kind || folders[p] == filetree.GitChangeMixed { break // already marked by a sibling — skip the rest. } - folders[p] = true + if folders[p] != filetree.GitChangeNone && folders[p] != kind { + folders[p] = filetree.GitChangeMixed + } else { + folders[p] = kind + } if p == root { break } @@ -189,6 +246,138 @@ func dirtyFolderSet(dirtyFiles map[string]bool, root string) map[string]bool { return folders } +// loadGitLineChanges returns line-level worktree changes for path. +func loadGitLineChanges(rootDir, path string) map[int]editor.GitLineChange { + if rootDir == "" || path == "" { + return nil + } + out, err := exec.Command("git", "-C", rootDir, "diff", "--unified=0", "--", path).Output() + if err != nil || len(out) == 0 { + return nil + } + return parseGitDiffLines(out) +} + +// loadGitHunkPreview returns the unified diff hunk covering zero-based line. +func loadGitHunkPreview(rootDir, path string, line int) []string { + if rootDir == "" || path == "" || line < 0 { + return nil + } + out, err := exec.Command("git", "-C", rootDir, "diff", "--unified=3", "--", path).Output() + if err != nil || len(out) == 0 { + return nil + } + return parseGitHunkPreview(out, line) +} + +// parseGitHunkPreview extracts the diff hunk covering zero-based line. +func parseGitHunkPreview(out []byte, line int) []string { + target := line + 1 + var current []string + match := false + flush := func() []string { + if match && len(current) > 0 { + return current + } + return nil + } + for _, raw := range bytes.Split(out, []byte{'\n'}) { + text := string(raw) + if strings.HasPrefix(text, "@@ ") { + if hunk := flush(); hunk != nil { + return hunk + } + _, _, newStart, newCount, ok := parseHunkHeader(text) + current = []string{text} + match = ok && lineInHunk(target, newStart, newCount) + continue + } + if len(current) == 0 { + continue + } + current = append(current, text) + } + return flush() +} + +// lineInHunk reports whether target one-based line belongs to a new-file range. +func lineInHunk(target, start, count int) bool { + if count == 0 { + return target == start + } + return target >= start && target < start+count +} + +// parseGitDiffLines converts unified diff hunks into editor gutter markers. +func parseGitDiffLines(out []byte) map[int]editor.GitLineChange { + changes := map[int]editor.GitLineChange{} + for _, raw := range bytes.Split(out, []byte{'\n'}) { + line := string(raw) + if !strings.HasPrefix(line, "@@ ") { + continue + } + oldStart, oldCount, newStart, newCount, ok := parseHunkHeader(line) + if !ok { + continue + } + if newCount == 0 { + mark := newStart + if mark < 0 { + mark = 0 + } + changes[mark] = editor.GitLineDeleted + _ = oldStart + _ = oldCount + continue + } + kind := editor.GitLineAdded + if oldCount > 0 { + kind = editor.GitLineModified + } + for lineNo := newStart; lineNo < newStart+newCount; lineNo++ { + changes[lineNo-1] = kind + } + } + return changes +} + +// parseHunkHeader extracts old/new ranges from a unified diff header. +func parseHunkHeader(line string) (int, int, int, int, bool) { + fields := strings.Fields(line) + if len(fields) < 3 { + return 0, 0, 0, 0, false + } + oldStart, oldCount, ok := parseDiffRange(fields[1]) + if !ok { + return 0, 0, 0, 0, false + } + newStart, newCount, ok := parseDiffRange(fields[2]) + if !ok { + return 0, 0, 0, 0, false + } + return oldStart, oldCount, newStart, newCount, true +} + +// parseDiffRange parses a hunk range such as -1,2 or +7. +func parseDiffRange(s string) (int, int, bool) { + if len(s) < 2 { + return 0, 0, false + } + parts := strings.SplitN(s[1:], ",", 2) + start, err := strconv.Atoi(parts[0]) + if err != nil { + return 0, 0, false + } + count := 1 + if len(parts) == 2 { + count, err = strconv.Atoi(parts[1]) + if err != nil { + return 0, 0, false + } + } + return start, count, true +} + // pathInside reports whether candidate is root or a descendant of root. // Uses filepath.Rel rather than string-prefix matching so '/foo/bar' // isn't considered inside '/foo/ba'. diff --git a/internal/app/gitstatus_test.go b/internal/app/gitstatus_test.go index f1c937a..98d2808 100644 --- a/internal/app/gitstatus_test.go +++ b/internal/app/gitstatus_test.go @@ -19,7 +19,11 @@ import ( "os/exec" "path/filepath" "sort" + "strings" "testing" + + "github.com/cloudmanic/spice-edit/internal/editor" + "github.com/cloudmanic/spice-edit/internal/filetree" ) // TestLoadGitStatus_NotARepo verifies that pointing the loader at a @@ -53,7 +57,7 @@ func TestLoadGitStatus_EmptyRoot(t *testing.T) { func TestLoadGitStatus_CleanRepo(t *testing.T) { requireGit(t) repo := initRepo(t) - writeFileT(t,filepath.Join(repo, "a.txt"), "hello") + writeFileT(t, filepath.Join(repo, "a.txt"), "hello") gitRun(t, repo, "add", "a.txt") gitRun(t, repo, "commit", "-m", "init") @@ -139,16 +143,16 @@ func TestLoadGitStatus_FindsModifiedAndUntracked(t *testing.T) { requireGit(t) repo := initRepo(t) - writeFileT(t,filepath.Join(repo, "tracked.txt"), "v1") + writeFileT(t, filepath.Join(repo, "tracked.txt"), "v1") gitRun(t, repo, "add", "tracked.txt") gitRun(t, repo, "commit", "-m", "init") // Modify the tracked file (worktree change). - writeFileT(t,filepath.Join(repo, "tracked.txt"), "v2") + writeFileT(t, filepath.Join(repo, "tracked.txt"), "v2") // Brand-new untracked file. - writeFileT(t,filepath.Join(repo, "untracked.txt"), "fresh") + writeFileT(t, filepath.Join(repo, "untracked.txt"), "fresh") // Staged-but-uncommitted. - writeFileT(t,filepath.Join(repo, "staged.txt"), "added") + writeFileT(t, filepath.Join(repo, "staged.txt"), "added") gitRun(t, repo, "add", "staged.txt") st := loadGitStatus(repo) @@ -157,7 +161,7 @@ func TestLoadGitStatus_FindsModifiedAndUntracked(t *testing.T) { } for _, want := range []string{"tracked.txt", "untracked.txt", "staged.txt"} { abs := filepath.Join(repo, want) - if !st.DirtyFiles[abs] { + if st.DirtyFiles[abs] == filetree.GitChangeNone { t.Errorf("expected %s to be dirty; got %v", want, sortedKeys(st.DirtyFiles)) } } @@ -175,14 +179,14 @@ func TestLoadGitStatus_FromSubdirectory(t *testing.T) { if err := os.MkdirAll(sub, 0o755); err != nil { t.Fatalf("mkdir: %v", err) } - writeFileT(t,filepath.Join(sub, "inside.txt"), "x") - writeFileT(t,filepath.Join(repo, "outside.txt"), "y") + writeFileT(t, filepath.Join(sub, "inside.txt"), "x") + writeFileT(t, filepath.Join(repo, "outside.txt"), "y") gitRun(t, repo, "add", ".") gitRun(t, repo, "commit", "-m", "init") // Mutate both files so they appear dirty. - writeFileT(t,filepath.Join(sub, "inside.txt"), "x2") - writeFileT(t,filepath.Join(repo, "outside.txt"), "y2") + writeFileT(t, filepath.Join(sub, "inside.txt"), "x2") + writeFileT(t, filepath.Join(repo, "outside.txt"), "y2") st := loadGitStatus(sub) if !st.IsRepo { @@ -192,7 +196,7 @@ func TestLoadGitStatus_FromSubdirectory(t *testing.T) { filepath.Join(sub, "inside.txt"), filepath.Join(repo, "outside.txt"), } { - if !st.DirtyFiles[want] { + if st.DirtyFiles[want] == filetree.GitChangeNone { t.Errorf("expected %s to be dirty; got %v", want, sortedKeys(st.DirtyFiles)) } } @@ -259,7 +263,7 @@ func TestParsePorcelain_BasicCases(t *testing.T) { len(got), sortedKeys(got)) } for _, k := range tc.wantKeys { - if !got[k] { + if got[k] == filetree.GitChangeNone { t.Errorf("missing %q in %v", k, sortedKeys(got)) } } @@ -267,20 +271,81 @@ func TestParsePorcelain_BasicCases(t *testing.T) { } } +// TestParsePorcelain_StatusKinds confirms the tree can color different git +// states distinctly instead of collapsing everything to one dirty color. +func TestParsePorcelain_StatusKinds(t *testing.T) { + top := "/tmp/repo" + got := parsePorcelain([]byte(" M mod.go\n?? new.go\n D gone.go\nR old.go -> moved.go\n"), top) + want := map[string]filetree.GitChangeKind{ + "/tmp/repo/mod.go": filetree.GitChangeModified, + "/tmp/repo/new.go": filetree.GitChangeAdded, + "/tmp/repo/gone.go": filetree.GitChangeDeleted, + "/tmp/repo/old.go": filetree.GitChangeDeleted, + "/tmp/repo/moved.go": filetree.GitChangeRenamed, + } + for path, kind := range want { + if got[path] != kind { + t.Fatalf("%s kind = %v, want %v; got %v", path, got[path], kind, got) + } + } +} + +// TestParseGitDiffLines maps unified hunk ranges to zero-based gutter rows. +func TestParseGitDiffLines(t *testing.T) { + diff := []byte("@@ -2,0 +3,2 @@\n+a\n+b\n@@ -8,2 +10,2 @@\n-old\n+new\n@@ -20,2 +21,0 @@\n-old\n") + got := parseGitDiffLines(diff) + if got[2] != editor.GitLineAdded || got[3] != editor.GitLineAdded { + t.Fatalf("added markers wrong: %v", got) + } + if got[9] != editor.GitLineModified || got[10] != editor.GitLineModified { + t.Fatalf("modified markers wrong: %v", got) + } + if got[21] != editor.GitLineDeleted { + t.Fatalf("deleted marker wrong: %v", got) + } +} + +// TestParseGitHunkPreview_ReturnsClickedHunk keeps gutter-click previews scoped +// to the hunk covering the clicked changed line. +func TestParseGitHunkPreview_ReturnsClickedHunk(t *testing.T) { + diff := []byte("diff --git a/a.go b/a.go\n@@ -1,2 +1,2 @@\n old context\n-old\n+new\n@@ -20,1 +20,2 @@\n keep\n+added\n") + got := parseGitHunkPreview(diff, 20) + if len(got) == 0 { + t.Fatal("expected hunk preview") + } + joined := strings.Join(got, "\n") + if !strings.Contains(joined, "+added") { + t.Fatalf("expected clicked hunk, got %q", joined) + } + if strings.Contains(joined, "-old") { + t.Fatalf("preview included wrong hunk: %q", joined) + } +} + +// TestLineInHunk_IncludesDeletionAnchor pins deleted-line marker matching. +func TestLineInHunk_IncludesDeletionAnchor(t *testing.T) { + if !lineInHunk(12, 12, 0) { + t.Fatal("deleted-only hunk should match its anchor line") + } + if lineInHunk(13, 12, 0) { + t.Fatal("deleted-only hunk should not match unrelated lines") + } +} + // TestUnquotePath_Variants verifies the C-style unquoter handles git's // default quoting — quoted paths come back clean, unquoted paths pass // through, and a malformed quoted string falls back to the raw input // rather than dropping the path entirely. func TestUnquotePath_Variants(t *testing.T) { cases := map[string]string{ - `plain.txt`: `plain.txt`, - `"quoted.txt"`: `quoted.txt`, - `"with space.txt"`: `with space.txt`, - `"escaped\nnewline"`: "escaped\nnewline", - `""`: ``, - ` spaced.txt `: `spaced.txt`, - ``: ``, - `"unterminated`: `"unterminated`, // malformed → raw fallback + `plain.txt`: `plain.txt`, + `"quoted.txt"`: `quoted.txt`, + `"with space.txt"`: `with space.txt`, + `"escaped\nnewline"`: "escaped\nnewline", + `""`: ``, + ` spaced.txt `: `spaced.txt`, + ``: ``, + `"unterminated`: `"unterminated`, // malformed → raw fallback } for in, want := range cases { if got := unquotePath(in); got != want { @@ -294,9 +359,9 @@ func TestUnquotePath_Variants(t *testing.T) { // collapsed branch still shows the user there's a change inside. func TestDirtyFolderSet_RollsUpToRoot(t *testing.T) { root := "/proj" - dirty := map[string]bool{ - "/proj/a/b/c/leaf.txt": true, - "/proj/x/y.txt": true, + dirty := map[string]filetree.GitChangeKind{ + "/proj/a/b/c/leaf.txt": filetree.GitChangeModified, + "/proj/x/y.txt": filetree.GitChangeModified, } got := dirtyFolderSet(dirty, root) @@ -308,12 +373,12 @@ func TestDirtyFolderSet_RollsUpToRoot(t *testing.T) { "/proj/x", } for _, w := range want { - if !got[w] { + if got[w] == filetree.GitChangeNone { t.Errorf("expected %q to be marked dirty; got %v", w, sortedKeys(got)) } } // The leaf file path itself isn't a folder, must not appear here. - if got["/proj/a/b/c/leaf.txt"] { + if got["/proj/a/b/c/leaf.txt"] != filetree.GitChangeNone { t.Error("dirtyFolderSet should not contain file paths") } } @@ -323,19 +388,19 @@ func TestDirtyFolderSet_RollsUpToRoot(t *testing.T) { // or the user's home directory can't be marked dirty by us. func TestDirtyFolderSet_StopsAtRoot(t *testing.T) { root := "/proj/inner" - dirty := map[string]bool{ - "/proj/inner/a/b.txt": true, + dirty := map[string]filetree.GitChangeKind{ + "/proj/inner/a/b.txt": filetree.GitChangeModified, } got := dirtyFolderSet(dirty, root) for _, ancestor := range []string{"/proj", "/", "/home"} { - if got[ancestor] { + if got[ancestor] != filetree.GitChangeNone { t.Errorf("walk escaped root: %q should not be marked", ancestor) } } - if !got["/proj/inner"] { + if got["/proj/inner"] == filetree.GitChangeNone { t.Error("root itself should be marked when something inside is dirty") } - if !got["/proj/inner/a"] { + if got["/proj/inner/a"] == filetree.GitChangeNone { t.Error("intermediate folder should be marked") } } @@ -352,6 +417,35 @@ func TestDirtyFolderSet_EmptyInput(t *testing.T) { } } +// TestRebaseGitPaths_NormalizesTreeRootCasing keeps git and filetree path keys +// aligned on case-insensitive filesystems where cwd casing may drift. +func TestRebaseGitPaths_NormalizesTreeRootCasing(t *testing.T) { + dirty := map[string]filetree.GitChangeKind{ + "/Users/fatih/Documents/Projeler/spice-edit/internal/app/app.go": filetree.GitChangeModified, + } + rebased := rebaseGitPaths(dirty, "/Users/fatih/documents/projeler/spice-edit") + want := "/Users/fatih/documents/projeler/spice-edit/internal/app/app.go" + if rebased[want] != filetree.GitChangeModified { + t.Fatalf("rebased path missing: got %v want key %q", rebased, want) + } +} + +// TestRebaseGitPaths_DoesNotMoveRepoPathsUnderSubdirRoot protects launches +// rooted at a subdirectory: only descendants of that tree root are rebased. +func TestRebaseGitPaths_DoesNotMoveRepoPathsUnderSubdirRoot(t *testing.T) { + dirty := map[string]filetree.GitChangeKind{ + "/repo/internal/app/app.go": filetree.GitChangeModified, + "/repo/internal/editor/tab.go": filetree.GitChangeModified, + } + rebased := rebaseGitPaths(dirty, "/repo/internal/app") + if rebased["/repo/internal/app/app.go"] != filetree.GitChangeModified { + t.Fatalf("descendant path should stay under subdir root, got %v", rebased) + } + if rebased["/repo/internal/editor/tab.go"] != filetree.GitChangeModified { + t.Fatalf("outside path should remain unchanged, got %v", rebased) + } +} + // TestPathInside covers the core ancestry check used by dirtyFolderSet. // Beyond the obvious matches, the prefix-trick trap ("/foo/bar" is NOT // inside "/foo/ba") is the regression we care most about. @@ -435,7 +529,7 @@ func writeFileT(t *testing.T, path, content string) { // sortedKeys returns the keys of m in lexicographic order — handy when // printing diff context inside test failures. -func sortedKeys(m map[string]bool) []string { +func sortedKeys[K comparable](m map[string]K) []string { out := make([]string, 0, len(m)) for k := range m { out = append(out, k) From 341e7f87c99e62e0052541162284eb7ae8f5b774 Mon Sep 17 00:00:00 2001 From: Queaxtra <60826916+Queaxtra@users.noreply.github.com> Date: Sun, 21 Jun 2026 22:44:29 +0300 Subject: [PATCH 06/10] feat: wire git gutter and tree sync into app --- internal/app/app.go | 77 ++++++++++++++++++++++++++++++++++++++-- internal/app/app_test.go | 32 +++++++++++++++++ 2 files changed, 107 insertions(+), 2 deletions(-) diff --git a/internal/app/app.go b/internal/app/app.go index b37900e..f82d1e1 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -539,11 +539,24 @@ func (a *App) refreshGitStatus() { a.tree.DirtyFiles = nil a.tree.DirtyFolders = nil a.gitBranch = "" + a.refreshGitLineChanges() return } - a.tree.DirtyFiles = st.DirtyFiles - a.tree.DirtyFolders = dirtyFolderSet(st.DirtyFiles, a.rootDir) + dirtyFiles := rebaseGitPaths(st.DirtyFiles, a.tree.Root.Path) + a.tree.DirtyFiles = dirtyFiles + a.tree.DirtyFolders = dirtyFolderSet(dirtyFiles, a.tree.Root.Path) a.gitBranch = st.Branch + a.refreshGitLineChanges() +} + +// refreshGitLineChanges refreshes gutter markers for every open text tab. +func (a *App) refreshGitLineChanges() { + for _, tab := range a.tabs { + if tab == nil || tab.Path == "" || tab.IsImage() { + continue + } + tab.GitLines = loadGitLineChanges(a.rootDir, tab.Path) + } } // startTreeRefresh launches a goroutine that posts a treeRefreshEvent every @@ -1257,6 +1270,9 @@ func (a *App) sidebarClick(x, y int) { // mirrors it onto the file tree so the matching row renders with the // "active" highlight. All writes to a.activeFolder go through here. func (a *App) setActiveFolder(path string) { + if abs, err := filepath.Abs(path); err == nil { + path = abs + } a.activeFolder = path if a.tree != nil { a.tree.ActiveFolder = path @@ -1279,11 +1295,25 @@ func (a *App) tabBarClick(x, _ int) { return } a.activeTab = r.Index + a.syncActiveTreeFile() return } } } +// syncActiveTreeFile mirrors the active tab path into the file tree. +func (a *App) syncActiveTreeFile() { + if a.tree == nil { + return + } + tab := a.activeTabPtr() + if tab == nil || tab.Path == "" { + a.tree.ActiveFile = "" + return + } + a.tree.ActiveFile = tab.Path +} + // editorPress handles the initial mouse press inside the editor — placing // the caret, optionally selecting a word on double-click. Image tabs // have no caret, so the press is dropped. @@ -1293,6 +1323,9 @@ func (a *App) editorPress(x, y int) { return } ex, ey, ew, eh := a.editorRect() + if a.openGitHunkAt(tab, x-ex, y-ey) { + return + } pos, ok := tab.HitTest(x-ex, y-ey, ew, eh) if !ok { return @@ -1308,6 +1341,24 @@ func (a *App) editorPress(x, y int) { tab.MoveCursorTo(pos, false) } +// openGitHunkAt opens a diff preview when the user clicks a gutter marker. +func (a *App) openGitHunkAt(tab *editor.Tab, localX, localY int) bool { + if localX != 0 || localY < 0 { + return false + } + line := tab.ScrollY + localY + if tab.GitLines[line] == editor.GitLineNone { + return false + } + lines := loadGitHunkPreview(a.rootDir, tab.Path, line) + if len(lines) == 0 { + a.openInfo("Git change", []string{"No git diff found for this line."}) + return true + } + a.openInfo("Git change · "+filepath.Base(tab.Path), lines) + return true +} + // editorDrag extends the selection during a click-drag inside the editor. // (x, y) is clamped to the editor rect so dragging into another pane still // extends the selection sensibly. When the mouse passes above or below the @@ -1498,10 +1549,30 @@ func (a *App) OpenFile(path string) { a.openFile(path) } // Whatever the path resolves to, its parent becomes the active folder so // the next New File from the main menu lands next to it. func (a *App) openFile(path string) { + if abs, err := filepath.Abs(path); err == nil { + path = abs + } a.setActiveFolder(filepath.Dir(path)) + if a.tree != nil { + a.tree.ActiveFile = path + // Reveal the file's location in the sidebar: expand every ancestor + // directory and scroll the row into view. Without this, opening a + // file via the finder (Esc-p) or the command line leaves the tree + // collapsed at the top, so the active-file highlight is set on a + // row nobody can see. listH mirrors Render's own list-area height + // (sidebarH - 2) so the "already visible" guard inside Reveal uses + // the same viewport the next paint will. + _, _, _, sh := a.sidebarRect() + listH := sh - 2 + if listH < 0 { + listH = 0 + } + a.tree.Reveal(path, listH) + } for i, t := range a.tabs { if t.Path == path { a.activeTab = i + t.GitLines = loadGitLineChanges(a.rootDir, t.Path) return } } @@ -1512,6 +1583,7 @@ func (a *App) openFile(path string) { } a.tabs = append(a.tabs, t) a.activeTab = len(a.tabs) - 1 + t.GitLines = loadGitLineChanges(a.rootDir, t.Path) a.flash(fmt.Sprintf("Opened %s", filepath.Base(path))) } @@ -1623,6 +1695,7 @@ func (a *App) closeTab(idx int) { if a.activeTab < 0 { a.activeTab = 0 } + a.syncActiveTreeFile() } // copySelection puts the active tab's selection on the system clipboard diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 4a856b3..5802735 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -1168,6 +1168,38 @@ func TestEditorPress_PlacesCaret(t *testing.T) { } } +// TestOpenGitHunkAt_OpensInfoOnMarker proves gutter markers are clickable. +func TestOpenGitHunkAt_OpensInfoOnMarker(t *testing.T) { + dir := t.TempDir() + target := filepath.Join(dir, "p.txt") + if err := os.WriteFile(target, []byte("hello\n"), 0644); err != nil { + t.Fatalf("seed: %v", err) + } + a := newTestApp(t, dir) + a.openFile(target) + tab := a.activeTabPtr() + tab.GitLines = map[int]editor.GitLineChange{0: editor.GitLineModified} + + if !a.openGitHunkAt(tab, 0, 0) { + t.Fatal("expected gutter marker click to be handled") + } + if !a.confirmOpen || !a.confirmInfo { + t.Fatal("expected git hunk click to open info modal") + } +} + +// TestOpenGitHunkAt_IgnoresCleanGutter keeps normal cursor placement intact. +func TestOpenGitHunkAt_IgnoresCleanGutter(t *testing.T) { + a := newTestApp(t, t.TempDir()) + tab, err := editor.NewTab("") + if err != nil { + t.Fatalf("NewTab: %v", err) + } + if a.openGitHunkAt(tab, 0, 0) { + t.Fatal("clean gutter should not be handled as a git preview") + } +} + // TestEditorPress_DoubleClickSelectsWord triggers the word-select path. func TestEditorPress_DoubleClickSelectsWord(t *testing.T) { dir := t.TempDir() From b4bf4967932733143b5eccfd7a5dc75fd3196545 Mon Sep 17 00:00:00 2001 From: Queaxtra <60826916+Queaxtra@users.noreply.github.com> Date: Sun, 21 Jun 2026 22:44:29 +0300 Subject: [PATCH 07/10] feat: add diff preview to info modal --- internal/app/modals.go | 23 ++++++++++++++++++++++- internal/app/modals_test.go | 24 +++++++++++++++++++++++- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/internal/app/modals.go b/internal/app/modals.go index b504762..324f36d 100644 --- a/internal/app/modals.go +++ b/internal/app/modals.go @@ -16,9 +16,12 @@ package app import ( + "strings" + "github.com/gdamore/tcell/v2" "github.com/cloudmanic/spice-edit/internal/filetree" + "github.com/cloudmanic/spice-edit/internal/theme" ) // Layout constants for the secondary modals. Width is wide enough to hold a @@ -548,7 +551,7 @@ func (a *App) drawConfirm() { if runeLen(line) > mw-4 { line = string([]rune(line)[:mw-4]) } - drawAt(a.screen, mx+2, my+3+i, line, bodyStyle) + drawAt(a.screen, mx+2, my+3+i, line, confirmInfoLineStyle(a.theme, bg, line)) } btnY := my + mh - 3 btnX := mx + (mw-10)/2 @@ -572,6 +575,24 @@ func (a *App) drawConfirm() { a.screen.HideCursor() } +// confirmInfoLineStyle colors git diff previews inside the info modal. +func confirmInfoLineStyle(th theme.Theme, bg tcell.Color, line string) tcell.Style { + style := tcell.StyleDefault.Background(bg).Foreground(th.Text) + if strings.HasPrefix(line, "+++") || strings.HasPrefix(line, "---") { + return style.Foreground(th.Muted) + } + if strings.HasPrefix(line, "+") { + return style.Foreground(th.GitAdded) + } + if strings.HasPrefix(line, "-") { + return style.Foreground(th.GitDeleted) + } + if strings.HasPrefix(line, "@@") { + return style.Foreground(th.AccentSoft).Bold(true) + } + return style +} + // ----------------------------------------------------------------------------- // Save / Discard / Cancel modal (unsaved-changes prompt) // ----------------------------------------------------------------------------- diff --git a/internal/app/modals_test.go b/internal/app/modals_test.go index d8a54f0..81b4c02 100644 --- a/internal/app/modals_test.go +++ b/internal/app/modals_test.go @@ -21,6 +21,7 @@ import ( "github.com/gdamore/tcell/v2" "github.com/cloudmanic/spice-edit/internal/filetree" + "github.com/cloudmanic/spice-edit/internal/theme" ) // TestCloseAllModals_ClearsEverything proves the helper turns off every @@ -61,6 +62,27 @@ func TestCloseAllModals_ClearsEverything(t *testing.T) { } } +// TestConfirmInfoLineStyle_ColorsDiffLines keeps git previews readable. +func TestConfirmInfoLineStyle_ColorsDiffLines(t *testing.T) { + th := theme.Default() + bg := th.LineHL + cases := []struct { + line string + want tcell.Color + }{ + {line: "+new code", want: th.GitAdded}, + {line: "-old code", want: th.GitDeleted}, + {line: "@@ -1 +1 @@", want: th.AccentSoft}, + {line: " context", want: th.Text}, + } + for _, tc := range cases { + fg, _, _ := confirmInfoLineStyle(th, bg, tc.line).Decompose() + if fg != tc.want { + t.Fatalf("%q fg = %v, want %v", tc.line, fg, tc.want) + } + } +} + // TestAnyModalOpen returns true for any one flag and false for none. func TestAnyModalOpen(t *testing.T) { a := newTestApp(t, t.TempDir()) @@ -530,7 +552,7 @@ func TestRuneLen(t *testing.T) { "": 0, "abc": 3, "héllo": 5, // five runes, one cell each by this helper's contract - "日本": 2, + "日本": 2, } for s, want := range cases { if got := runeLen(s); got != want { From 85bf6bbfd0746b189dca4aed13eb6ea29e8feb97 Mon Sep 17 00:00:00 2001 From: Queaxtra <60826916+Queaxtra@users.noreply.github.com> Date: Sun, 21 Jun 2026 22:44:30 +0300 Subject: [PATCH 08/10] feat: add tree reveal on finder open test --- internal/app/finder_test.go | 77 +++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/internal/app/finder_test.go b/internal/app/finder_test.go index 191f487..7e7d4ee 100644 --- a/internal/app/finder_test.go +++ b/internal/app/finder_test.go @@ -13,6 +13,7 @@ import ( "testing" "time" + "github.com/cloudmanic/spice-edit/internal/filetree" "github.com/cloudmanic/spice-edit/internal/finder" "github.com/gdamore/tcell/v2" ) @@ -243,6 +244,82 @@ func TestLeader_PFiresFinder(t *testing.T) { } } +// TestFinder_EnterRevealsFileInTree is the headline fix for the sidebar-sync +// bug: opening a file via the finder (Esc-p → type → Enter) used to set the +// active-file highlight on a row nobody could see, because the tree stayed +// collapsed at the top. After the fix, openFile calls tree.Reveal, which +// expands every ancestor and scrolls the row into view. This test opens a +// nested file under internal/finder/ through the finder keystroke loop and +// asserts both that the ancestor dir is expanded and that the file's row is +// inside the tree's viewport (via the public HitTest contract). +func TestFinder_EnterRevealsFileInTree(t *testing.T) { + a, dir := withFinder(t) + a.openFinder() + for _, r := range "score" { + a.handleFinderKey(tcell.NewEventKey(tcell.KeyRune, r, tcell.ModNone)) + } + if len(a.finderResults) == 0 { + t.Fatal("expected score results") + } + rel := a.finderResults[0].Path + want := filepath.Join(dir, rel) + + a.handleFinderKey(tcell.NewEventKey(tcell.KeyEnter, 0, tcell.ModNone)) + + // The finder returns paths like "internal/finder/score.go" — so the + // "internal" ancestor must now be expanded, and the file's row must be + // inside the tree's viewport. + internal := treeChildByName(a.tree.Root, "internal") + if internal == nil { + t.Fatal("internal/ ancestor missing from tree") + } + if !internal.Expanded { + t.Fatal("internal/ should be expanded after opening via finder") + } + if a.tree.ActiveFile != want { + t.Fatalf("ActiveFile: got %q, want %q", a.tree.ActiveFile, want) + } + // Re-render so the tree's visible-rows cache reflects the post-reveal + // flat list, then walk the list rows via HitTest and confirm the file + // is on screen. Using the public HitTest contract keeps the test honest + // about what a user would actually see. + sx, sy, sw, sh := a.sidebarRect() + a.tree.Render(a.screen, a.theme, sx, sy, sw, sh) + listH := sh - 2 + if listH < 0 { + listH = 0 + } + found := false + for row := 0; row < listH; row++ { + n, ok := a.tree.HitTest(0, row+2) // list rows start at localY 2 + if !ok || n == nil { + continue + } + if n.Path == want { + found = true + break + } + } + if !found { + t.Fatalf("opened file %q not visible in tree after reveal (ScrollY=%d)", rel, a.tree.ScrollY) + } +} + +// treeChildByName returns the direct child of n named name, or nil. A tiny +// local helper so the finder-reveal test can inspect ancestor expansion +// without reaching into the package's private fields. +func treeChildByName(n *filetree.Node, name string) *filetree.Node { + if n == nil { + return nil + } + for _, c := range n.Children { + if c.Name == name { + return c + } + } + return nil +} + // endsWith is a tiny string suffix check pulled in so the result- // path assertions in this file read as the rule they're enforcing. func endsWith(s, suffix string) bool { From 660fe9fbbce9b66502d32d0f477a65eb0ac2b030 Mon Sep 17 00:00:00 2001 From: Queaxtra <60826916+Queaxtra@users.noreply.github.com> Date: Tue, 7 Jul 2026 13:21:24 +0300 Subject: [PATCH 09/10] fix: address review feedback for git gutter PR Thanks for the thorough review, and no worries at all about the turnaround time. Genuinely appreciated, and glad the gutter bars, sidebar syncing, and diff popup landed well. All three requested points are addressed: 1. Multi-line comment / string colouring mid-scroll HighlightVisible now tokenises a bounded lead (256 rows) both above and below the viewport, then keeps only the visible rows in the output grid. The lead is bidirectional because chroma needs both delimiters of a multi-line block in range to colour the body; an upper-only lead still left the body uncoloured when the closer sat below the viewport. Keystroke cost stays tied to terminal height plus the fixed lead, not file size. 2. Highlight recompute cadence Render now re-tokenises only when the content changed (StyleStale, which was being set by every edit but never actually read) or the viewport shifted (scroll / height). Mouse moves and other no-op redraws reuse the cached grid. The grid is indexed by absolute line number and only carries the visible rows, so a scroll change still triggers a recompute. 3. Change-bar overlapping line numbers on 10000+ line files Replaced the fixed gutterWidth = 6 with gutterWidthFor(lineCount) = max(6, digits + 2), so the gutter grows by one cell per extra digit and the change-bar always sits in a blank leading cell instead of overwriting the first digit. EnsureVisible, Render, and HitTest all use the dynamic width. Tests: added TestHighlightVisible_LeadRecoversCommentState, TestTab_RenderSkipsHighlightWhenNotStale, TestTab_RenderRecomputesHighlightOnScroll, TestGutterWidthFor, and TestTab_Render_GitMarkerDoesNotOverlapLineNumber. Verified with `go test -race ./...` (all packages pass), `go vet ./...`, and `gofmt` clean. Amp-Thread-ID: https://ampcode.com/threads/T-019f3c09-f41d-77f2-8a3d-198764673a66 --- internal/editor/highlight.go | 46 ++++++++-- internal/editor/highlight_test.go | 47 +++++++++- internal/editor/tab.go | 60 ++++++++++--- internal/editor/tab_test.go | 144 +++++++++++++++++++++++++++--- 4 files changed, 266 insertions(+), 31 deletions(-) diff --git a/internal/editor/highlight.go b/internal/editor/highlight.go index 0aa70ce..a86f09e 100644 --- a/internal/editor/highlight.go +++ b/internal/editor/highlight.go @@ -29,8 +29,26 @@ func Highlight(filename, src string, t theme.Theme) [][]tcell.Style { return highlightSource(filename, src, t) } -// HighlightVisible returns a style grid for the current viewport. Only visible -// rows are tokenised so keystroke cost follows terminal height, not file size. +// highlightLeadLines is how many rows above and below the viewport we +// tokenise but discard, so the lexer can re-enter state it couldn't see +// otherwise, like a multi-line comment or quoted string that spans the +// viewport. Without this lead, scrolling into the middle of a long comment +// colours the body as plain code because the lexer starts fresh at the +// viewport. The lead runs both above and below: chroma needs both +// delimiters of a multi-line block in range to colour the body, so a +// comment that opens above and closes below the viewport needs the opener +// reached by the upper lead and the closer reached by the lower lead. +// Bounded so keystroke cost still follows terminal height, not file size: +// a block longer than the combined lead is the rare case that still +// mis-colours. +const highlightLeadLines = 256 + +// HighlightVisible returns a style grid for the current viewport. Only +// visible rows are kept in the output so keystroke cost follows terminal +// height, not file size. To stay correct inside multi-line comments and +// strings that span the viewport, tokenisation starts a bounded lead above +// the top and ends a bounded lead below the bottom; those lead rows are +// styled then thrown away. func HighlightVisible(filename string, lines []string, startLine, height int, t theme.Theme) [][]tcell.Style { styles := make([][]tcell.Style, len(lines)) if height <= 0 || startLine >= len(lines) { @@ -43,14 +61,26 @@ func HighlightVisible(filename string, lines []string, startLine, height int, t if endLine > len(lines) { endLine = len(lines) } - visible := strings.Join(lines[startLine:endLine], "\n") - visibleStyles := highlightSource(filename, visible, t) - for i, row := range visibleStyles { - lineIdx := startLine + i - if lineIdx >= endLine || lineIdx >= len(styles) { + // Tokenise a bounded lead above and below the viewport. The upper lead + // lets the lexer re-enter comment / string state opened earlier; the + // lower lead closes blocks that end past the viewport. We keep only the + // visible rows; the lead rows are styled then dropped. + leadStart := startLine - highlightLeadLines + if leadStart < 0 { + leadStart = 0 + } + leadEnd := endLine + highlightLeadLines + if leadEnd > len(lines) { + leadEnd = len(lines) + } + src := strings.Join(lines[leadStart:leadEnd], "\n") + leadStyles := highlightSource(filename, src, t) + for i := startLine; i < endLine; i++ { + idx := i - leadStart + if idx >= len(leadStyles) { break } - styles[lineIdx] = row + styles[i] = leadStyles[idx] } return styles } diff --git a/internal/editor/highlight_test.go b/internal/editor/highlight_test.go index 946e2e7..05996f1 100644 --- a/internal/editor/highlight_test.go +++ b/internal/editor/highlight_test.go @@ -162,7 +162,9 @@ func (f *Foo) Bar() string { } // TestHighlightVisible_LimitsTokenisingToViewport pins the fast path: -// off-screen rows stay empty while visible rows are tokenised. +// off-screen rows stay empty in the output while visible rows are tokenised. +// (A bounded lead above the viewport is also tokenised but discarded, so the +// output grid still only carries the visible rows.) func TestHighlightVisible_LimitsTokenisingToViewport(t *testing.T) { th := theme.Default() lines := make([]string, 20) @@ -182,3 +184,46 @@ func TestHighlightVisible_LimitsTokenisingToViewport(t *testing.T) { t.Fatal("visible row was not highlighted") } } + +// TestHighlightVisible_LeadRecoversCommentState proves the bounded lead +// fixes the regression where scrolling into the middle of a multi-line +// comment coloured the body as plain code. The comment opens above the +// viewport; without the lead the lexer would start fresh at the visible +// rows and never know it was inside a comment. +func TestHighlightVisible_LeadRecoversCommentState(t *testing.T) { + th := theme.Default() + // A multi-line comment opens on line 1 and closes on line 6. The + // viewport starts on line 4, inside the comment, with the opening /* + // above the visible rows. + lines := []string{ + "package main", // 0 + "/*", // 1 <- comment opens + " * body one", // 2 + " * body two", // 3 + " * body three", // 4 <- viewport start + " * body four", // 5 + " */", // 6 + "func main() {}", // 7 + } + got := HighlightVisible("main.go", lines, 4, 2, th) + comment := tcell.StyleDefault.Background(th.BG).Foreground(th.SynComment).Italic(true) + for _, i := range []int{4, 5} { + row := got[i] + if row == nil { + t.Fatalf("line %d was not highlighted", i) + } + // At least one rune on each visible comment body line must carry + // the comment style, proving the lead let the lexer re-enter the + // block instead of tokenising the body as plain text. + hasComment := false + for _, st := range row { + if st == comment { + hasComment = true + break + } + } + if !hasComment { + t.Errorf("line %d has no comment-styled rune; lead did not recover comment state", i) + } + } +} diff --git a/internal/editor/tab.go b/internal/editor/tab.go index f6940a4..eadfc19 100644 --- a/internal/editor/tab.go +++ b/internal/editor/tab.go @@ -12,6 +12,7 @@ import ( "image" "os" "path/filepath" + "strconv" "time" "github.com/gdamore/tcell/v2" @@ -19,10 +20,28 @@ import ( "github.com/cloudmanic/spice-edit/internal/theme" ) -// gutterWidth is the cell width reserved for the line-number column inside -// the editor area. Six gives us up to five-digit line numbers plus a one-cell -// pad on the right — comfortable for files of any realistic length. -const gutterWidth = 6 +// defaultGutterWidth is the line-number column width for files up to 9999 +// lines: five digits plus a one-cell pad on the right, with the git +// change-bar sitting in the blank cell at the far-left of the right-aligned +// number. Larger files grow the gutter via gutterWidthFor so the marker +// never overlaps the first digit. +const defaultGutterWidth = 6 + +// gutterWidthFor returns the line-number column width for a buffer of +// lineCount lines. It keeps defaultGutterWidth for files that fit and grows +// by one cell per extra digit so the git change-bar always has a blank +// leading cell to sit in. Without this, a 10000-line file would render +// "10000" as "▌0000" with the bar overwriting the first digit, because the +// right-aligned number fills every cell the marker shares. +func gutterWidthFor(lineCount int) int { + if lineCount <= 0 { + return defaultGutterWidth + } + if w := len(strconv.Itoa(lineCount)) + 2; w > defaultGutterWidth { + return w + } + return defaultGutterWidth +} // GitLineChange describes the marker rendered in the editor gutter for a line. type GitLineChange int @@ -49,6 +68,15 @@ type Tab struct { StyleStale bool GitLines map[int]GitLineChange + // lastHighlightScrollY / lastHighlightHeight record the viewport Render + // last tokenised for. Without them, every redraw (mouse moves included) + // would re-tokenise the visible rows even when nothing changed. Render + // recomputes only when the content changed (StyleStale) or the viewport + // shifted (scroll / height), since the grid is indexed by absolute line + // number and only carries the visible rows. + lastHighlightScrollY int + lastHighlightHeight int + // Mtime is the file's modification time as of the last successful // read or write. The app's periodic disk-reconcile loop compares it // against the live mtime to detect external edits. @@ -485,7 +513,7 @@ func (t *Tab) SelectAll() { // caller passes the editor area's width and height because the Tab itself // doesn't know its render rect. func (t *Tab) EnsureVisible(viewW, viewH int) { - contentW := viewW - gutterWidth - 1 + contentW := viewW - gutterWidthFor(t.Buffer.LineCount()) - 1 if contentW < 1 { contentW = 1 } @@ -524,8 +552,17 @@ func (t *Tab) Render(scr tcell.Screen, th theme.Theme, x, y, w, h int) { t.cursorMoved = false } t.clampScroll(h) - t.Styles = HighlightVisible(t.Path, t.Buffer.Lines, t.ScrollY, h, th) - t.StyleStale = false + // Re-tokenise only when the content changed (StyleStale) or the viewport + // shifted (scroll / height). Otherwise every redraw, including mouse + // moves, would re-tokenise the visible rows for nothing. The grid is + // indexed by absolute line number and only carries the visible rows, so + // a scroll change means different rows must be filled. + if t.StyleStale || t.ScrollY != t.lastHighlightScrollY || h != t.lastHighlightHeight { + t.Styles = HighlightVisible(t.Path, t.Buffer.Lines, t.ScrollY, h, th) + t.StyleStale = false + t.lastHighlightScrollY = t.ScrollY + t.lastHighlightHeight = h + } bg := th.BG bgStyle := tcell.StyleDefault.Background(bg).Foreground(th.Text) @@ -541,8 +578,9 @@ func (t *Tab) Render(scr tcell.Screen, th theme.Theme, x, y, w, h int) { selStart, selEnd := PosOrdered(t.Anchor, t.Cursor) hasSel := t.HasSelection() - contentX := x + gutterWidth + 1 - contentW := w - gutterWidth - 1 + gw := gutterWidthFor(t.Buffer.LineCount()) + contentX := x + gw + 1 + contentW := w - gw - 1 if contentW < 1 { contentW = 1 } @@ -569,7 +607,7 @@ func (t *Tab) Render(scr tcell.Screen, th theme.Theme, x, y, w, h int) { } // Gutter / line number, right-aligned with one trailing space. - numStr := fmt.Sprintf("%*d", gutterWidth-1, lineIdx+1) + numStr := fmt.Sprintf("%*d", gw-1, lineIdx+1) gutterStyle := tcell.StyleDefault.Background(lineBg).Foreground(th.Muted) if isCursorLine { gutterStyle = gutterStyle.Foreground(th.AccentSoft) @@ -699,7 +737,7 @@ func (t *Tab) HitTest(localX, localY, w, h int) (Position, bool) { if localY < 0 || localY >= h { return Position{}, false } - contentX := gutterWidth + 1 + contentX := gutterWidthFor(t.Buffer.LineCount()) + 1 line := t.ScrollY + localY if line < 0 || line >= t.Buffer.LineCount() { return Position{}, false diff --git a/internal/editor/tab_test.go b/internal/editor/tab_test.go index 6a00329..744bf6c 100644 --- a/internal/editor/tab_test.go +++ b/internal/editor/tab_test.go @@ -16,6 +16,7 @@ package editor import ( "os" "path/filepath" + "reflect" "runtime" "strings" "testing" @@ -694,7 +695,7 @@ func TestTab_Render_DrawsLineNumbersAndContent(t *testing.T) { if cy != 0 { t.Errorf("cursor row = %d, want 0", cy) } - if cx < gutterWidth { + if cx < defaultGutterWidth { t.Errorf("cursor col %d should be past the gutter", cx) } } @@ -716,7 +717,7 @@ func TestTab_Render_HighlightsSelection(t *testing.T) { if !vis { t.Fatal("cursor hidden") } - wantCx := gutterWidth + 1 + 5 + wantCx := defaultGutterWidth + 1 + 5 if cx != wantCx { t.Errorf("cursor x = %d, want %d", cx, wantCx) } @@ -748,7 +749,7 @@ func TestTab_HitTest_ContentClick(t *testing.T) { tab, _ := NewTab("") tab.Buffer = NewBuffer("hello\nworld") - pos, ok := tab.HitTest(gutterWidth+1+2, 1, 40, 10) + pos, ok := tab.HitTest(defaultGutterWidth+1+2, 1, 40, 10) if !ok { t.Fatal("expected ok") } @@ -797,7 +798,7 @@ func TestTab_HitTest_ClampsColumnAtLineEnd(t *testing.T) { tab, _ := NewTab("") tab.Buffer = NewBuffer("ab") - pos, ok := tab.HitTest(gutterWidth+1+50, 0, 80, 10) + pos, ok := tab.HitTest(defaultGutterWidth+1+50, 0, 80, 10) if !ok { t.Fatal("expected ok") } @@ -830,9 +831,9 @@ func TestTab_Render_ExpandsTabsToTabStops(t *testing.T) { } return c.Runes[0] } - // Content starts at col gutterWidth+1. The tab fills 4 cells, so + // Content starts at col defaultGutterWidth+1. The tab fills 4 cells, so // 'a' lands at content+4, 'b' at +5, 'c' at +6. - contentCol := gutterWidth + 1 + contentCol := defaultGutterWidth + 1 if w < contentCol+7 { t.Fatalf("simulated screen too narrow: w=%d", w) } @@ -855,7 +856,7 @@ func TestTab_HitTest_InsideTabSnapsToTab(t *testing.T) { tab, _ := NewTab("") tab.Buffer = NewBuffer("\tx") - contentX := gutterWidth + 1 + contentX := defaultGutterWidth + 1 for offset := 0; offset < 4; offset++ { pos, ok := tab.HitTest(contentX+offset, 0, 40, 10) if !ok { @@ -933,7 +934,7 @@ func TestTab_Render_OverflowIndicator_Right(t *testing.T) { defer scr.Fini() tab, _ := NewTab("") - // 30 chars on one line; viewport content width = 20 - gutterWidth - 1 = 13. + // 30 chars on one line; viewport content width = 20 - defaultGutterWidth - 1 = 13. tab.Buffer = NewBuffer(strings.Repeat("x", 30)) tab.Cursor = Position{Line: 0, Col: 0} tab.Anchor = tab.Cursor @@ -966,10 +967,10 @@ func TestTab_Render_OverflowIndicator_Left(t *testing.T) { scr.Show() cells, w, _ := scr.GetContents() - // First content cell is at column gutterWidth + 1. - left := cells[gutterWidth+1] + // First content cell is at column defaultGutterWidth + 1. + left := cells[defaultGutterWidth+1] if len(left.Runes) == 0 || left.Runes[0] != '‹' { - t.Fatalf("expected '‹' at row 0 col %d, got %q", gutterWidth+1, string(left.Runes)) + t.Fatalf("expected '‹' at row 0 col %d, got %q", defaultGutterWidth+1, string(left.Runes)) } _ = w } @@ -996,3 +997,124 @@ func TestTab_Render_NoOverflowIndicator_WhenLineFits(t *testing.T) { } } } + +// TestGutterWidthFor pins the dynamic gutter width: files up to 9999 lines +// keep the default six-cell gutter, and each extra digit grows it by one so +// the git change-bar always has a blank leading cell to sit in. +func TestGutterWidthFor(t *testing.T) { + cases := []struct { + lines int + want int + }{ + {0, 6}, {1, 6}, {999, 6}, {9999, 6}, + {10000, 7}, {99999, 7}, {100000, 8}, + } + for _, c := range cases { + if got := gutterWidthFor(c.lines); got != c.want { + t.Errorf("gutterWidthFor(%d) = %d, want %d", c.lines, got, c.want) + } + } +} + +// TestTab_RenderSkipsHighlightWhenNotStale confirms Render only re-tokenises +// the visible rows when something actually changed. Without this gate every +// redraw, including mouse moves, would re-tokenise for nothing, and the +// StyleStale flag set by every edit would be written but never read. We +// detect recompute by comparing the styles grid's backing array: a fresh +// make per recompute vs. reuse on skip. +func TestTab_RenderSkipsHighlightWhenNotStale(t *testing.T) { + scr := newSimScreen(t, 40, 10) + defer scr.Fini() + + tab, _ := NewTab("") + tab.Buffer = NewBuffer("package main\nfunc main() {}\n") + tab.StyleStale = true + + tab.Render(scr, theme.Default(), 0, 0, 40, 10) + if tab.Styles == nil { + t.Fatal("expected first render to highlight") + } + firstPtr := reflect.ValueOf(tab.Styles).Pointer() + + // Second render with no content, scroll, or height change: should reuse + // the existing styles grid instead of re-tokenising. + tab.Render(scr, theme.Default(), 0, 0, 40, 10) + if reflect.ValueOf(tab.Styles).Pointer() != firstPtr { + t.Fatal("expected Render to skip re-highlight when nothing changed") + } + + // An edit marks styles stale, so the next render must recompute. + tab.StyleStale = true + tab.Render(scr, theme.Default(), 0, 0, 40, 10) + if reflect.ValueOf(tab.Styles).Pointer() == firstPtr { + t.Fatal("expected Render to re-highlight after StyleStale set") + } +} + +// TestTab_RenderRecomputesHighlightOnScroll confirms Render re-tokenises +// when the viewport scrolls. The highlight grid is indexed by absolute line +// number and only carries the visible rows, so a scroll change means +// different rows must be filled even when the content is unchanged. +func TestTab_RenderRecomputesHighlightOnScroll(t *testing.T) { + scr := newSimScreen(t, 40, 10) + defer scr.Fini() + + tab, _ := NewTab("") + tab.Buffer = NewBuffer(strings.Repeat("package main\n", 50)) + tab.StyleStale = true + + tab.Render(scr, theme.Default(), 0, 0, 40, 10) // ScrollY clamps to 0 + if tab.Styles == nil { + t.Fatal("expected first render to highlight") + } + firstPtr := reflect.ValueOf(tab.Styles).Pointer() + + // Scroll without moving the cursor (cursorMoved=false so EnsureVisible + // doesn't snap ScrollY back). The visible rows changed, so Render must + // re-tokenise even though StyleStale is false. + tab.ScrollY = 20 + tab.cursorMoved = false + tab.Render(scr, theme.Default(), 0, 0, 40, 10) + if reflect.ValueOf(tab.Styles).Pointer() == firstPtr { + t.Fatal("expected Render to re-highlight after scroll") + } +} + +// TestTab_Render_GitMarkerDoesNotOverlapLineNumber is the regression test for +// the change-bar covering a line-number digit on files past 10000 lines. +// Before the dynamic gutter, "10000" rendered as "▌0000" with the bar +// overwriting the first digit. The gutter now grows by one cell per extra +// digit so the marker always sits in a blank leading cell. +func TestTab_Render_GitMarkerDoesNotOverlapLineNumber(t *testing.T) { + const w = 60 + scr := newSimScreen(t, w, 10) + defer scr.Fini() + + tab, _ := NewTab("") + tab.Buffer = NewBuffer(strings.Repeat("x\n", 9999) + "x") // exactly 10000 lines + tab.GitLines = map[int]GitLineChange{9999: GitLineModified} + tab.ScrollY = 9990 // line 9999 (display 10000) lands on the last visible row + tab.cursorMoved = false + + tab.Render(scr, theme.Default(), 0, 0, w, 10) + scr.Show() + + cells, _, _ := scr.GetContents() + const row = 9 // last visible row -> line 9999 + cellRune := func(col int) rune { + c := cells[row*w+col] + if len(c.Runes) == 0 { + return ' ' + } + return c.Runes[0] + } + if got := cellRune(0); got != '▌' { + t.Errorf("expected git marker at col 0, got %q", got) + } + want := "10000" + for i, r := range want { + if got := cellRune(1 + i); got != r { + t.Errorf("col %d: got %q, want %q", 1+i, got, string(r)) + } + } +} From be684cf724cd59208373e4c55e66e0bae6b52e61 Mon Sep 17 00:00:00 2001 From: Queaxtra <60826916+Queaxtra@users.noreply.github.com> Date: Sun, 12 Jul 2026 23:53:31 +0300 Subject: [PATCH 10/10] fix: scroll info modals and include staged diffs --- internal/app/app.go | 1 + internal/app/gitstatus.go | 4 +- internal/app/gitstatus_test.go | 21 +++++++++++ internal/app/modals.go | 67 ++++++++++++++++++++++++++++++---- internal/app/modals_test.go | 20 ++++++++++ 5 files changed, 103 insertions(+), 10 deletions(-) diff --git a/internal/app/app.go b/internal/app/app.go index f82d1e1..6ad125b 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -350,6 +350,7 @@ type App struct { // scp / ssh diagnostics that naturally wrap. confirmInfo bool confirmMessageLines []string + confirmInfoScroll int // Save/Discard/Cancel modal — used when closing a dirty tab or // quitting with unsaved changes. dirtyHover indexes the button row: diff --git a/internal/app/gitstatus.go b/internal/app/gitstatus.go index 13a4e04..a6a1c99 100644 --- a/internal/app/gitstatus.go +++ b/internal/app/gitstatus.go @@ -251,7 +251,7 @@ func loadGitLineChanges(rootDir, path string) map[int]editor.GitLineChange { if rootDir == "" || path == "" { return nil } - out, err := exec.Command("git", "-C", rootDir, "diff", "--unified=0", "--", path).Output() + out, err := exec.Command("git", "-C", rootDir, "diff", "--unified=0", "HEAD", "--", path).Output() if err != nil || len(out) == 0 { return nil } @@ -263,7 +263,7 @@ func loadGitHunkPreview(rootDir, path string, line int) []string { if rootDir == "" || path == "" || line < 0 { return nil } - out, err := exec.Command("git", "-C", rootDir, "diff", "--unified=3", "--", path).Output() + out, err := exec.Command("git", "-C", rootDir, "diff", "--unified=3", "HEAD", "--", path).Output() if err != nil || len(out) == 0 { return nil } diff --git a/internal/app/gitstatus_test.go b/internal/app/gitstatus_test.go index 98d2808..b0a1f1b 100644 --- a/internal/app/gitstatus_test.go +++ b/internal/app/gitstatus_test.go @@ -73,6 +73,27 @@ func TestLoadGitStatus_CleanRepo(t *testing.T) { } } +// TestLoadGitLineChanges_IncludesStagedChanges compares the worktree with HEAD, +// so staging a file does not remove its gutter markers. +func TestLoadGitLineChanges_IncludesStagedChanges(t *testing.T) { + requireGit(t) + repo := initRepo(t) + path := filepath.Join(repo, "a.txt") + writeFileT(t, path, "one\ntwo\nthree\n") + gitRun(t, repo, "add", "a.txt") + gitRun(t, repo, "commit", "-m", "init") + writeFileT(t, path, "one\nchanged\nthree\nfour\n") + gitRun(t, repo, "add", "a.txt") + + changes := loadGitLineChanges(repo, "a.txt") + if len(changes) == 0 { + t.Fatal("staged changes should produce gutter markers") + } + if got := changes[1]; got != editor.GitLineModified { + t.Fatalf("line 2 marker = %v, want modified", got) + } +} + // TestLoadGitBranch_NotARepo confirms the helper degrades quietly when // the directory isn't a git work tree — empty string, no panic, no // stderr noise reaching the editor. diff --git a/internal/app/modals.go b/internal/app/modals.go index 324f36d..8fcd9c2 100644 --- a/internal/app/modals.go +++ b/internal/app/modals.go @@ -80,6 +80,7 @@ func (a *App) closeAllModals() { a.formCallback = nil a.confirmInfo = false a.confirmMessageLines = nil + a.confirmInfoScroll = 0 // confirmCancelHook is parked here so an unrelated confirm modal // opened after a format-trust / format-install prompt can't // accidentally inherit the cancel hook. The flows that need a @@ -365,6 +366,7 @@ func (a *App) openInfo(title string, lines []string) { a.confirmInfo = true a.confirmTitle = title a.confirmMessageLines = lines + a.confirmInfoScroll = 0 a.confirmHover = 0 } @@ -406,6 +408,14 @@ func (a *App) handleConfirmKey(ev *tcell.EventKey) { switch ev.Key() { case tcell.KeyEsc, tcell.KeyEnter, tcell.KeyTab: a.closeAllModals() + case tcell.KeyUp: + a.scrollConfirmInfo(-1) + case tcell.KeyDown: + a.scrollConfirmInfo(1) + case tcell.KeyPgUp: + a.scrollConfirmInfo(-a.confirmInfoBodyRows()) + case tcell.KeyPgDn: + a.scrollConfirmInfo(a.confirmInfoBodyRows()) } return } @@ -438,6 +448,14 @@ func (a *App) handleConfirmMouse(x, y int, btn tcell.ButtonMask) { if a.confirmInfo { // Single OK button at row mh-3, centered. Outside the modal // dismisses too — same convention as the rest of the modals. + if btn&tcell.Button4 != 0 { + a.scrollConfirmInfo(-3) + return + } + if btn&tcell.Button5 != 0 { + a.scrollConfirmInfo(3) + return + } if btn&tcell.Button1 == 0 { return } @@ -491,13 +509,7 @@ func (a *App) confirmModalRect() (x, y, w, h int) { h = confirmModalHeight if a.confirmInfo { w = 84 - bodyRows := len(a.confirmMessageLines) - if bodyRows < 1 { - bodyRows = 1 - } - // Chrome budget: top border + title + divider + blank + button + blank + bottom = 7, - // but we want at least one blank between body and button. Match - // the layout in drawConfirm so this stays in lockstep. + bodyRows := a.confirmInfoBodyRows() h = bodyRows + 7 } x = (a.width - w) / 2 @@ -511,6 +523,39 @@ func (a *App) confirmModalRect() (x, y, w, h int) { return } +// confirmInfoBodyRows returns the visible diff viewport height. +func (a *App) confirmInfoBodyRows() int { + const chromeRows = 7 + rows := a.height - chromeRows + if rows < 1 { + return 1 + } + if len(a.confirmMessageLines) < rows { + if len(a.confirmMessageLines) < 1 { + return 1 + } + return len(a.confirmMessageLines) + } + return rows +} + +func (a *App) scrollConfirmInfo(delta int) { + if !a.confirmInfo { + return + } + maxScroll := len(a.confirmMessageLines) - a.confirmInfoBodyRows() + if maxScroll < 0 { + maxScroll = 0 + } + a.confirmInfoScroll += delta + if a.confirmInfoScroll < 0 { + a.confirmInfoScroll = 0 + } + if a.confirmInfoScroll > maxScroll { + a.confirmInfoScroll = maxScroll + } +} + // drawConfirm renders the Yes/No modal. // // Rows (relY): @@ -547,7 +592,13 @@ func (a *App) drawConfirm() { // OK button. The body is left-aligned because scp/ssh stderr // usually starts with file paths that read poorly when // centered. - for i, line := range a.confirmMessageLines { + bodyRows := a.confirmInfoBodyRows() + a.scrollConfirmInfo(0) + end := a.confirmInfoScroll + bodyRows + if end > len(a.confirmMessageLines) { + end = len(a.confirmMessageLines) + } + for i, line := range a.confirmMessageLines[a.confirmInfoScroll:end] { if runeLen(line) > mw-4 { line = string([]rune(line)[:mw-4]) } diff --git a/internal/app/modals_test.go b/internal/app/modals_test.go index 81b4c02..d02e0a3 100644 --- a/internal/app/modals_test.go +++ b/internal/app/modals_test.go @@ -83,6 +83,26 @@ func TestConfirmInfoLineStyle_ColorsDiffLines(t *testing.T) { } } +func TestConfirmInfoScroll_ClampsToViewport(t *testing.T) { + a := newTestApp(t, t.TempDir()) + a.width = 100 + a.height = 12 + lines := make([]string, 20) + a.openInfo("Git change", lines) + + if got := a.confirmInfoBodyRows(); got != 5 { + t.Fatalf("body rows = %d, want 5", got) + } + a.scrollConfirmInfo(100) + if want := len(lines) - 5; a.confirmInfoScroll != want { + t.Fatalf("scroll = %d, want %d", a.confirmInfoScroll, want) + } + a.scrollConfirmInfo(-100) + if a.confirmInfoScroll != 0 { + t.Fatalf("scroll = %d, want 0", a.confirmInfoScroll) + } +} + // TestAnyModalOpen returns true for any one flag and false for none. func TestAnyModalOpen(t *testing.T) { a := newTestApp(t, t.TempDir())