From 2f18db437781e7842310403fac44ef23b17b628d Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 19 May 2026 14:15:02 +0200 Subject: [PATCH 1/7] Add regression tests for trailing-fill rendering The next few commits restructure how the view's draw() decides the fg/bg of cells past the end of a line's content. Pin down three existing behaviors first so the restructuring stays a refactor: - '\n' should reset attributes for the trailing area so a reversed final cell doesn't bleed into empty space. - An unterminated line with AttrReverse on its last cell should propagate that to the right edge (otherwise the rendered bg abruptly stops at the last character). - '\x1b[K' on a line that fits within InnerWidth should fill the remaining cells with the current bg color. Introduce a small WithSimulationScreen helper that swaps in a tcell mock terminal so tests can call view.draw() and inspect rendered cells via Screen.Get(). Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/gocui/view_test.go | 91 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/pkg/gocui/view_test.go b/pkg/gocui/view_test.go index a7023be43..ef56d6756 100644 --- a/pkg/gocui/view_test.go +++ b/pkg/gocui/view_test.go @@ -8,10 +8,28 @@ import ( "strings" "testing" + "github.com/gdamore/tcell/v3" + "github.com/gdamore/tcell/v3/color" "github.com/rivo/uniseg" "github.com/stretchr/testify/assert" ) +// WithSimulationScreen swaps the package-level Screen for a tcell +// terminfo-backed mock terminal so tests can call view.draw() and +// inspect rendered cells via Screen.Get(). The previous Screen is +// restored on test cleanup. +func WithSimulationScreen(t *testing.T, width, height int) { + t.Helper() + saved := Screen + if err := (&Gui{}).tcellInitSimulation(width, height); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + Screen.Fini() + Screen = saved + }) +} + func TestWriteString(t *testing.T) { tests := []struct { existingLines []string @@ -413,3 +431,76 @@ func TestLineWrap(t *testing.T) { }) } } + +// TestNewlineTerminatedLineClearsTrailingBg verifies that a '\n' resets +// any attributes (e.g. AttrReverse-driven background) past the line's +// content, so a reversed cell at the end doesn't bleed into the empty +// area to the right. +func TestNewlineTerminatedLineClearsTrailingBg(t *testing.T) { + WithSimulationScreen(t, 14, 5) + + v := NewView("name", 0, 0, 11, 4, OutputNormal) + + // \x1b[7m sets reverse; \x1b[31m sets fg=red. With reverse the cell + // renders with bg=red. The trailing area past "foo" must NOT extend + // the red bg because '\n' marks the line as cleanly terminated. + v.writeString("\x1b[7m\x1b[31mfoo\x1b[0m\n") + v.draw() + + // First row: cells 1..3 are "foo" (render with red bg via reverse), + // cells 4..10 are trailing and should be plain default. + for x := 4; x <= 10; x++ { + _, style, _ := Screen.Get(x, 1) + assert.Equal(t, tcell.ColorDefault, style.GetForeground(), + "trailing cell at (%d, 1) should have default fg", x) + assert.False(t, style.HasReverse(), + "trailing cell at (%d, 1) should not have reverse attribute", x) + } +} + +// TestUnterminatedReverseLineExtendsToEdge verifies that without a +// terminating '\n' or '\x1b[K', the line's last cell's attributes +// (including AttrReverse) propagate through the trailing area so a +// reversed-bg line extends all the way to the right edge. +func TestUnterminatedReverseLineExtendsToEdge(t *testing.T) { + WithSimulationScreen(t, 14, 5) + + v := NewView("name", 0, 0, 11, 4, OutputNormal) + + // Reverse + red fg, "foo", no termination. Each "foo" cell renders + // with bg=red via reverse, and the trailing cells past "foo" must + // keep the reverse so the rendered bg extends to the right edge. + v.writeString("\x1b[7m\x1b[31mfoo") + v.draw() + + // Cells 1..3 are content; cells 4..10 are trailing. All ten should + // have reverse on with red fg (so they all render with bg=red). + for x := 1; x <= 10; x++ { + _, style, _ := Screen.Get(x, 1) + assert.Equal(t, color.Maroon, style.GetForeground(), + "cell at (%d, 1) should have red fg under reverse", x) + assert.True(t, style.HasReverse(), + "cell at (%d, 1) should have reverse attribute", x) + } +} + +// TestShortFilledLineExtendsBgWithoutWrap verifies that '\x1b[K' fills +// the rest of the line with the current bg color for a line that's +// short enough to fit within the view's inner width. +func TestShortFilledLineExtendsBgWithoutWrap(t *testing.T) { + WithSimulationScreen(t, 14, 5) + + v := NewView("name", 0, 0, 11, 4, OutputNormal) + + // \x1b[41m sets bg=red. "hi" fits within InnerWidth=10; \x1b[K should + // fill the remaining 8 cells with red. + v.writeString("\x1b[41mhi\x1b[K\x1b[0m\n") + v.draw() + + // All ten cells at (1..10, 1) should have red bg. + for x := 1; x <= 10; x++ { + _, style, _ := Screen.Get(x, 1) + assert.Equal(t, color.Maroon, style.GetBackground(), + "cell at (%d, 1) should have red bg", x) + } +} From 6c55823492dbe3d57810a284038f366855e501a4 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 19 May 2026 14:15:44 +0200 Subject: [PATCH 2/7] Demonstrate broken background fill on wrapped \x1b[K-padded lines Tools like delta emit each diff line with the bg color set, then \x1b[K to fill the rest of the row with that bg color. When the content fits within the view's inner width, gocui's \x1b[K handling appends explicit padding cells and rendering works. When the content exceeds the inner width, \x1b[K adds no cells (negative repeat count), the line is wrapped into multiple segments, and the partial tail segment's trailing cells fall back to the view default bg instead of continuing the fill color. Add a test that drives draw() against a tcell mock terminal and asserts the current (buggy) trailing background. Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/gocui/view_test.go | 86 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/pkg/gocui/view_test.go b/pkg/gocui/view_test.go index ef56d6756..8e9f0e196 100644 --- a/pkg/gocui/view_test.go +++ b/pkg/gocui/view_test.go @@ -504,3 +504,89 @@ func TestShortFilledLineExtendsBgWithoutWrap(t *testing.T) { "cell at (%d, 1) should have red bg", x) } } + +// TestWrappedFilledLineExtendsBgToEdge demonstrates that when a line is +// filled to the edge with \x1b[K (the pattern used by `delta` for diff +// lines) but exceeds the view's inner width, every wrapped segment loses +// the fill background past its content. +func TestWrappedFilledLineExtendsBgToEdge(t *testing.T) { + WithSimulationScreen(t, 14, 6) + + // View dimensions: Width=12 (x0=0..x1=11), Height=6; InnerWidth=10, + // InnerHeight=4. Frame inset of 1 places content cells at screen + // (1..10, 1..4). + v := NewView("name", 0, 0, 11, 5, OutputNormal) + v.Wrap = true + + // Content with spaces so word wrap ends each segment before the + // right edge: "aaa bbb ccc ddd eee" wraps at InnerWidth=10 to three + // segments — "aaa bbb" / "ccc ddd" / "eee". Each row's trailing area + // should pick up the red fill from \x1b[K but currently falls back to + // the view default bg. + v.writeString("\x1b[41m" + "aaa bbb ccc ddd eee" + "\x1b[0m\x1b[41m\x1b[K\x1b[0m\n") + v.draw() + + // trailingFrom is 1-indexed: each row's content ends at column + // trailingFrom[y]-1, so columns trailingFrom[y]..10 are the trailing + // fill area where the bug shows. + trailingFrom := []int{8, 8, 4} + for y := 1; y <= 3; y++ { + for x := trailingFrom[y-1]; x <= 10; x++ { + _, style, _ := Screen.Get(x, y) + /* EXPECTED: + assert.Equal(t, color.Maroon, style.GetBackground(), + "trailing cell at (%d, %d) should have red bg", x, y) + ACTUAL: */ + assert.Equal(t, tcell.ColorDefault, style.GetBackground(), + "trailing cell at (%d, %d) falls back to default bg", x, y) + } + } +} + +// TestMulticolorWrappedFillUsesLastCellOfEachSegment demonstrates that +// when a wrapped line switches bg color part-way through and ends with +// \x1b[K, the trailing area on each wrapped row should match the bg +// that was active where that row's content ended — not the \x1b[K bg, +// which would bleed the color from the end of the logical line back +// into the earlier wrapped rows. +func TestMulticolorWrappedFillUsesLastCellOfEachSegment(t *testing.T) { + WithSimulationScreen(t, 14, 6) + + // View dimensions: Width=12 (x0=0..x1=11), Height=6; InnerWidth=10, + // InnerHeight=4. Frame inset of 1 places content cells at screen + // (1..10, 1..4). + v := NewView("name", 0, 0, 11, 5, OutputNormal) + v.Wrap = true + + // Content "aaa bbb ccc" is 11 cells; lineWrap breaks at the space + // between "bbb" and "ccc" (index 7) so segment 1 is "aaa bbb" (red, + // last cell red) and segment 2 is "ccc" (green, last cell green). + // \x1b[K records the green bg on the source line. + v.writeString("\x1b[41maaa bbb\x1b[42m ccc\x1b[K\x1b[0m\n") + v.draw() + + // Row 1's content ends with a red cell at x=7, so trailing columns + // 8..10 should pick up red rather than the \x1b[K's green. + for x := 8; x <= 10; x++ { + _, style, _ := Screen.Get(x, 1) + /* EXPECTED: + assert.Equal(t, color.Maroon, style.GetBackground(), + "trailing cell at (%d, 1) should have red bg (matching segment's last cell)", x) + ACTUAL: */ + assert.Equal(t, tcell.ColorDefault, style.GetBackground(), + "trailing cell at (%d, 1) falls back to default bg", x) + } + + // Row 2's content ends with a green cell at x=3, so trailing + // columns 4..10 should pick up green (matching both the segment's + // last cell and the \x1b[K bg — these happen to agree here). + for x := 4; x <= 10; x++ { + _, style, _ := Screen.Get(x, 2) + /* EXPECTED: + assert.Equal(t, color.Green, style.GetBackground(), + "trailing cell at (%d, 2) should have green bg", x) + ACTUAL: */ + assert.Equal(t, tcell.ColorDefault, style.GetBackground(), + "trailing cell at (%d, 2) falls back to default bg", x) + } +} From d234d8b3580e8cca037647e060963d003d9ddce5 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 18 May 2026 19:42:25 +0200 Subject: [PATCH 3/7] Remove dead \x00 filtering from string conversion helpers The four ReplaceAll(str, "\x00", "") calls (and the equivalent rune-by-rune skip in linesToString) are leftover from when cell.chr was a rune and \x00 was used as an internal sentinel. With chr now being a string and no code path writing \x00, the filtering never strips anything. Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/gocui/view.go | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index 166cb0e2c..ea6f80399 100644 --- a/pkg/gocui/view.go +++ b/pkg/gocui/view.go @@ -1411,9 +1411,7 @@ func (v *View) BufferLines() []string { lines := make([]string, len(v.lines)) for i, l := range v.lines { - str := lineType(l).String() - str = strings.ReplaceAll(str, "\x00", "") - lines[i] = str + lines[i] = lineType(l).String() } return lines } @@ -1434,9 +1432,7 @@ func (v *View) ViewBufferLines() []string { lines := make([]string, len(v.viewLines)) for i, l := range v.viewLines { - str := lineType(l.line).String() - str = strings.ReplaceAll(str, "\x00", "") - lines[i] = str + lines[i] = lineType(l.line).String() } return lines } @@ -1605,14 +1601,7 @@ func lineWrap(line []cell, columns int) [][]cell { func linesToString(lines [][]cell) string { str := make([]string, len(lines)) for i := range lines { - rns := make([]rune, 0, len(lines[i])) - line := lineType(lines[i]).String() - for _, c := range line { - if c != '\x00' { - rns = append(rns, c) - } - } - str[i] = string(rns) + str[i] = lineType(lines[i]).String() } return strings.Join(str, "\n") @@ -1682,9 +1671,7 @@ func (v *View) SelectedLines() []string { } func (v *View) lineContentAtIdx(idx int) string { - line := v.lines[idx] - str := lineType(line).String() - return strings.ReplaceAll(str, "\x00", "") + return lineType(v.lines[idx]).String() } func (v *View) SelectedPoint() (int, int) { From 1788eaa91ff128eb40fcabf04990b88609060420 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 20 May 2026 18:54:35 +0200 Subject: [PATCH 4/7] Rename lineType to cells This is to free up the name lineType for something else in the next commit. --- pkg/gocui/view.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index ea6f80399..e780fa292 100644 --- a/pkg/gocui/view.go +++ b/pkg/gocui/view.go @@ -453,7 +453,7 @@ type cell struct { hyperlink string } -type lineType []cell +type cells []cell func characterEquals(chr []byte, b byte) bool { return len(chr) == 1 && chr[0] == b @@ -464,7 +464,7 @@ func isCRLF(chr []byte) bool { } // String returns a string from a given cell slice. -func (l lineType) String() string { +func (l cells) String() string { var str strings.Builder for _, c := range l { str.WriteString(c.chr) @@ -1411,7 +1411,7 @@ func (v *View) BufferLines() []string { lines := make([]string, len(v.lines)) for i, l := range v.lines { - lines[i] = lineType(l).String() + lines[i] = cells(l).String() } return lines } @@ -1432,7 +1432,7 @@ func (v *View) ViewBufferLines() []string { lines := make([]string, len(v.viewLines)) for i, l := range v.viewLines { - lines[i] = lineType(l.line).String() + lines[i] = cells(l.line).String() } return lines } @@ -1474,7 +1474,7 @@ func (v *View) Line(y int) (string, bool) { return "", false } - return lineType(v.lines[y]).String(), true + return cells(v.lines[y]).String(), true } // Word returns a string with the word of the view's internal buffer @@ -1489,7 +1489,7 @@ func (v *View) Word(x, y int) (string, bool) { return "", false } - str := lineType(v.lines[y]).String() + str := cells(v.lines[y]).String() nl := strings.LastIndexFunc(str[:x], indexFunc) if nl == -1 { @@ -1601,7 +1601,7 @@ func lineWrap(line []cell, columns int) [][]cell { func linesToString(lines [][]cell) string { str := make([]string, len(lines)) for i := range lines { - str[i] = lineType(lines[i]).String() + str[i] = cells(lines[i]).String() } return strings.Join(str, "\n") @@ -1671,7 +1671,7 @@ func (v *View) SelectedLines() []string { } func (v *View) lineContentAtIdx(idx int) string { - return lineType(v.lines[idx]).String() + return cells(v.lines[idx]).String() } func (v *View) SelectedPoint() (int, int) { From ad8335aaa8d2eba8f808cc7d3afd6dd409602b71 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 18 May 2026 19:51:03 +0200 Subject: [PATCH 5/7] Wrap v.lines cells in a line struct The cells of a source line will soon need to carry metadata about how the line was terminated (newline vs filled to edge via \x1b[K). Move to a struct so there's somewhere to put it; this commit only renames [][]cell to []line{cells: ...} with no behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/gocui/gui.go | 2 +- pkg/gocui/view.go | 93 ++++++++++++++++++++++-------------------- pkg/gocui/view_test.go | 18 ++++---- 3 files changed, 61 insertions(+), 52 deletions(-) diff --git a/pkg/gocui/gui.go b/pkg/gocui/gui.go index 7b691f6d7..ad8ba1e41 100644 --- a/pkg/gocui/gui.go +++ b/pkg/gocui/gui.go @@ -1345,7 +1345,7 @@ func (g *Gui) onKey(ev *GocuiEvent) error { } visibleLineWidth := 0 - for _, c := range v.lines[newY] { + for _, c := range v.lines[newY].cells { visibleLineWidth += c.width } if visibleLineWidth < newX { diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index e780fa292..ed18c3e54 100644 --- a/pkg/gocui/view.go +++ b/pkg/gocui/view.go @@ -28,12 +28,12 @@ const ( // position. type View struct { name string - x0, y0, x1, y1 int // left top right bottom - ox, oy int // view offsets - cx, cy int // cursor position - rx, ry int // Read() offsets - wx, wy int // Write() offsets - lines [][]cell // All the data + x0, y0, x1, y1 int // left top right bottom + ox, oy int // view offsets + cx, cy int // cursor position + rx, ry int // Read() offsets + wx, wy int // Write() offsets + lines []lineType // All the data outMode OutputMode // The y position of the first line of a range selection. // This is not relative to the view's origin: it is relative to the first line @@ -446,6 +446,12 @@ type viewLine struct { line []cell } +// lineType is one of v.lines: the cells of a source lineType, plus any per-lineType +// metadata about how it was terminated (added in later commits). +type lineType struct { + cells cells +} + type cell struct { chr string // a grapheme cluster width int // number of terminal cells occupied by chr (always 1 or 2) @@ -738,20 +744,20 @@ func (v *View) makeWriteable(x, y int) { } v.lines = v.lines[:newLen] } else { - v.lines = append(v.lines, nil) + v.lines = append(v.lines, lineType{}) } } // cell `x` need not be index-able (that's why `<`) // append should be used by `lines[y]` user if he wants to write beyond `x` - for len(v.lines[y]) < x { - if cap(v.lines[y]) > len(v.lines[y]) { - newLen := cap(v.lines[y]) + for len(v.lines[y].cells) < x { + if cap(v.lines[y].cells) > len(v.lines[y].cells) { + newLen := cap(v.lines[y].cells) if newLen > x { newLen = x } - v.lines[y] = v.lines[y][:newLen] + v.lines[y].cells = v.lines[y].cells[:newLen] } else { - v.lines[y] = append(v.lines[y], cell{}) + v.lines[y].cells = append(v.lines[y].cells, cell{}) } } } @@ -761,7 +767,7 @@ func (v *View) makeWriteable(x, y int) { func (v *View) writeCells(cells []cell) { var newLen int // use maximum len available - line := v.lines[v.wy][:cap(v.lines[v.wy])] + line := v.lines[v.wy].cells[:cap(v.lines[v.wy].cells)] maxCopy := len(line) - v.wx if maxCopy < len(cells) { copy(line[v.wx:], cells[:maxCopy]) @@ -770,11 +776,11 @@ func (v *View) writeCells(cells []cell) { } else { // maxCopy >= len(cells) copy(line[v.wx:], cells) newLen = v.wx + len(cells) - if newLen < len(v.lines[v.wy]) { - newLen = len(v.lines[v.wy]) + if newLen < len(v.lines[v.wy].cells) { + newLen = len(v.lines[v.wy].cells) } } - v.lines[v.wy] = line[:newLen] + v.lines[v.wy].cells = line[:newLen] v.wx += len(cells) } @@ -800,7 +806,7 @@ func (v *View) write(p []byte) { finishLine := func() { v.autoRenderHyperlinksInCurrentLine() - if v.wx >= len(v.lines[v.wy]) { + if v.wx >= len(v.lines[v.wy].cells) { v.writeCells([]cell{{ chr: "", width: 0, @@ -814,7 +820,7 @@ func (v *View) write(p []byte) { v.wx = 0 v.wy++ if v.wy >= len(v.lines) { - v.lines = append(v.lines, nil) + v.lines = append(v.lines, lineType{}) } } @@ -851,7 +857,7 @@ func (v *View) write(p []byte) { } v.writeCells(cells) if truncateLine { - v.lines[v.wy] = v.lines[v.wy][:v.wx] + v.lines[v.wy].cells = v.lines[v.wy].cells[:v.wx] } } } @@ -910,7 +916,7 @@ func (v *View) autoRenderHyperlinksInCurrentLine() { return } - line := v.lines[v.wy] + line := v.lines[v.wy].cells start := 0 for { linkStart := findLinkStart(line[start:]) @@ -927,7 +933,7 @@ func (v *View) autoRenderHyperlinksInCurrentLine() { link.WriteString(line[linkEnd].chr) } for i := linkStart; i < linkEnd; i++ { - v.lines[v.wy][i].hyperlink = link.String() + v.lines[v.wy].cells[i].hyperlink = link.String() } start = linkEnd } @@ -958,7 +964,7 @@ func (v *View) parseInput(ch []byte, width int, x int, _ int) (bool, []cell) { // fill rest of line v.ei.instructionRead() cx := 0 - for _, cell := range v.lines[v.wy][0:v.wx] { + for _, cell := range v.lines[v.wy].cells[0:v.wx] { cx += cell.width } repeatCount = v.InnerWidth() - cx @@ -1010,8 +1016,8 @@ func (v *View) Read(p []byte) (n int, err error) { v.readBuffer = nil } for v.ry < len(v.lines) { - for v.rx < len(v.lines[v.ry]) { - s := v.lines[v.ry][v.rx].chr + for v.rx < len(v.lines[v.ry].cells) { + s := v.lines[v.ry].cells[v.rx].chr count := len(s) copy(p[offset:], s) v.rx++ @@ -1175,9 +1181,9 @@ func (v *View) updateSearchPositions() { } // If a view line exists for this line index: - if v.lines[result.Y] != nil { + if v.lines[result.Y].cells != nil { // search this view line for the search string - positions := searchPositionsForLine(v.lines[result.Y], result.Y) + positions := searchPositionsForLine(v.lines[result.Y].cells, result.Y) if len(positions) > 0 { // If we found any occurrences, add them v.searcher.searchPositions = append(v.searcher.searchPositions, positions...) @@ -1318,7 +1324,7 @@ func (v *View) refreshViewLinesIfNeeded() { wrap = maxX } - ls := lineWrap(line, wrap) + ls := lineWrap(line.cells, wrap) for j := range ls { vline := viewLine{linesX: j, linesY: i, line: ls[j]} @@ -1411,7 +1417,7 @@ func (v *View) BufferLines() []string { lines := make([]string, len(v.lines)) for i, l := range v.lines { - lines[i] = cells(l).String() + lines[i] = l.cells.String() } return lines } @@ -1454,12 +1460,12 @@ func (v *View) ViewLinesHeight() int { // ViewBuffer returns a string with the contents of the view's buffer that is // shown to the user. func (v *View) ViewBuffer() string { - lines := make([][]cell, len(v.viewLines)) + strs := make([]string, len(v.viewLines)) for i := range v.viewLines { - lines[i] = v.viewLines[i].line + strs[i] = cells(v.viewLines[i].line).String() } - return linesToString(lines) + return strings.Join(strs, "\n") } // Line returns a string with the line of the view's internal buffer @@ -1474,7 +1480,7 @@ func (v *View) Line(y int) (string, bool) { return "", false } - return cells(v.lines[y]).String(), true + return v.lines[y].cells.String(), true } // Word returns a string with the word of the view's internal buffer @@ -1485,11 +1491,11 @@ func (v *View) Word(x, y int) (string, bool) { return "", false } - if x < 0 || y < 0 || y >= len(v.lines) || x >= len(v.lines[y]) { + if x < 0 || y < 0 || y >= len(v.lines) || x >= len(v.lines[y].cells) { return "", false } - str := cells(v.lines[y]).String() + str := v.lines[y].cells.String() nl := strings.LastIndexFunc(str[:x], indexFunc) if nl == -1 { @@ -1519,9 +1525,8 @@ func (v *View) SetHighlight(y int, on bool) { return } - line := v.lines[y] - cells := make([]cell, 0) - for _, c := range line { + cells := make([]cell, 0, len(v.lines[y].cells)) + for _, c := range v.lines[y].cells { if on { c.bgColor = v.SelBgColor c.fgColor = v.SelFgColor @@ -1532,7 +1537,7 @@ func (v *View) SetHighlight(y int, on bool) { cells = append(cells, c) } v.tainted = true - v.lines[y] = cells + v.lines[y].cells = cells v.clearHover() } @@ -1598,10 +1603,10 @@ func lineWrap(line []cell, columns int) [][]cell { return lines } -func linesToString(lines [][]cell) string { +func linesToString(lines []lineType) string { str := make([]string, len(lines)) for i := range lines { - str[i] = cells(lines[i]).String() + str[i] = lines[i].cells.String() } return strings.Join(str, "\n") @@ -1671,7 +1676,7 @@ func (v *View) SelectedLines() []string { } func (v *View) lineContentAtIdx(idx int) string { - return cells(v.lines[idx]).String() + return v.lines[idx].cells.String() } func (v *View) SelectedPoint() (int, int) { @@ -1775,11 +1780,11 @@ func (v *View) OverwriteLinesAndClearEverythingElse(lineCount int, y int, conten v.overwriteLines(y, content) for i := range y { - v.lines[i] = nil + v.lines[i] = lineType{} } for i := v.wy + 1; i < len(v.lines); i += 1 { - v.lines[i] = nil + v.lines[i] = lineType{} } } @@ -1922,7 +1927,7 @@ func (v *View) scrollMargin() int { // foreground color func (v *View) ContainsColoredText(fgColor string, text string) bool { for _, line := range v.lines { - if containsColoredTextInLine(fgColor, text, line) { + if containsColoredTextInLine(fgColor, text, line.cells) { return true } } diff --git a/pkg/gocui/view_test.go b/pkg/gocui/view_test.go index 8e9f0e196..35b48269b 100644 --- a/pkg/gocui/view_test.go +++ b/pkg/gocui/view_test.go @@ -101,14 +101,14 @@ func TestWriteString(t *testing.T) { for _, test := range tests { v := NewView("name", 0, 0, 10, 10, OutputNormal) for _, l := range test.existingLines { - v.lines = append(v.lines, stringToCells(l)) + v.lines = append(v.lines, lineType{cells: stringToCells(l)}) } for _, s := range test.stringsToWrite { v.writeString(s) } var resultingLines [][]string for _, l := range v.lines { - resultingLines = append(resultingLines, cellsToStrings(l)) + resultingLines = append(resultingLines, cellsToStrings(l.cells)) } assert.Equal(t, test.expectedLines, resultingLines) } @@ -144,19 +144,19 @@ func TestAutoRenderingHyperlinks(t *testing.T) { v.writeString("htt") // No hyperlinks are generated for incomplete URLs - assert.Equal(t, "", v.lines[0][0].hyperlink) + assert.Equal(t, "", v.lines[0].cells[0].hyperlink) // Writing more characters to the same line makes the link complete (even // though we didn't see a newline yet) v.writeString("ps://example.com") - assert.Equal(t, "https://example.com", v.lines[0][0].hyperlink) + assert.Equal(t, "https://example.com", v.lines[0].cells[0].hyperlink) v.Clear() // Valid but incomplete URL v.writeString("https://exa") - assert.Equal(t, "https://exa", v.lines[0][0].hyperlink) + assert.Equal(t, "https://exa", v.lines[0].cells[0].hyperlink) // Writing more characters to the same fixes the link v.writeString("mple.com") - assert.Equal(t, "https://example.com", v.lines[0][0].hyperlink) + assert.Equal(t, "https://example.com", v.lines[0].cells[0].hyperlink) } func TestContainsColoredText(t *testing.T) { @@ -229,7 +229,11 @@ func TestContainsColoredText(t *testing.T) { } for i, test := range tests { - v := &View{lines: test.lines} + lines := make([]lineType, len(test.lines)) + for j, cells := range test.lines { + lines[j] = lineType{cells: cells} + } + v := &View{lines: lines} assert.Equal(t, test.expected, v.ContainsColoredText(test.fgColorStr, test.text), "Test %d failed", i) } } From 9c8a02f9014b27f24c18e2d9da1841a226becefe Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 20 May 2026 08:32:39 +0200 Subject: [PATCH 6/7] Remove the '\n' sentinel cell MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sentinel was appended to every \n-terminated line solely so that draw()'s prevFgColor tracking would reset to default for the trailing area; without it, an AttrReverse-styled last cell would carry its rendered bg past the end of the line. The same prevFgColor mechanism propagated AttrReverse past content on *unterminated* lines too — which doesn't match real terminal behavior (try `print '\x1b[7m\x1b[31mfoo'` in a shell: the reverse stops at the last character) and isn't relied on by anything in lazygit, since all our writers terminate lines with \n. Drop the sentinel cell, drop prevFgColor, and just have draw() paint trailing cells with the view's default fg/bg. The TestUnterminatedReverseLineExtendsToEdge regression test inverts to document the new (terminal-matching) behavior, renamed accordingly. TestWriteString expectations also drop the trailing "" that came from the sentinel. Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/gocui/view.go | 14 -------------- pkg/gocui/view_test.go | 39 +++++++++++++++++++-------------------- 2 files changed, 19 insertions(+), 34 deletions(-) diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index ed18c3e54..e8947a665 100644 --- a/pkg/gocui/view.go +++ b/pkg/gocui/view.go @@ -806,14 +806,6 @@ func (v *View) write(p []byte) { finishLine := func() { v.autoRenderHyperlinksInCurrentLine() - if v.wx >= len(v.lines[v.wy].cells) { - v.writeCells([]cell{{ - chr: "", - width: 0, - fgColor: 0, - bgColor: 0, - }}) - } } advanceToNextLine := func() { @@ -1254,7 +1246,6 @@ func (v *View) draw() { } emptyCell := cell{chr: " ", width: 1, fgColor: ColorDefault, bgColor: ColorDefault} - var prevFgColor Attribute for y, vline := range v.viewLines[start:] { if y >= maxY { @@ -1284,13 +1275,8 @@ func (v *View) draw() { // if we're out of cells to write, we'll just print empty cells. if cellIdx > len(vline.line)-1 { c = emptyCell - c.fgColor = prevFgColor } else { c = vline.line[cellIdx] - // capturing previous foreground colour so that if we're using the reverse - // attribute we honour the final character's colour and don't awkwardly switch - // to a new background colour for the remainder of the line - prevFgColor = c.fgColor } fgColor := c.fgColor diff --git a/pkg/gocui/view_test.go b/pkg/gocui/view_test.go index 35b48269b..9d0e9a4ef 100644 --- a/pkg/gocui/view_test.go +++ b/pkg/gocui/view_test.go @@ -44,17 +44,17 @@ func TestWriteString(t *testing.T) { { []string{}, []string{"1\n"}, - [][]string{{"1", ""}}, + [][]string{{"1"}}, }, { []string{}, []string{"1\n", "2\n"}, - [][]string{{"1", ""}, {"2", ""}}, + [][]string{{"1"}, {"2"}}, }, { []string{"a"}, []string{"1\n"}, - [][]string{{"1", ""}}, + [][]string{{"1"}}, }, { []string{"a\x00"}, @@ -74,12 +74,12 @@ func TestWriteString(t *testing.T) { { []string{}, []string{"1\r"}, - [][]string{{"1", ""}}, + [][]string{{"1"}}, }, { []string{"a"}, []string{"1\r"}, - [][]string{{"1", ""}}, + [][]string{{"1"}}, }, { []string{"a\x00"}, @@ -462,29 +462,28 @@ func TestNewlineTerminatedLineClearsTrailingBg(t *testing.T) { } } -// TestUnterminatedReverseLineExtendsToEdge verifies that without a -// terminating '\n' or '\x1b[K', the line's last cell's attributes -// (including AttrReverse) propagate through the trailing area so a -// reversed-bg line extends all the way to the right edge. -func TestUnterminatedReverseLineExtendsToEdge(t *testing.T) { +// TestUnterminatedReverseLineDoesNotExtend verifies that an unterminated +// line ending with an AttrReverse cell does NOT propagate the reversed +// background past the line's content — matching real terminal behavior +// (try `print '\x1b[7m\x1b[31mfoo'` in a shell). The trailing area +// is rendered as plain default. +func TestUnterminatedReverseLineDoesNotExtend(t *testing.T) { WithSimulationScreen(t, 14, 5) v := NewView("name", 0, 0, 11, 4, OutputNormal) - // Reverse + red fg, "foo", no termination. Each "foo" cell renders - // with bg=red via reverse, and the trailing cells past "foo" must - // keep the reverse so the rendered bg extends to the right edge. + // Reverse + red fg, "foo", no termination. The trailing cells past + // "foo" should be plain default, NOT a continuation of the red bg. v.writeString("\x1b[7m\x1b[31mfoo") v.draw() - // Cells 1..3 are content; cells 4..10 are trailing. All ten should - // have reverse on with red fg (so they all render with bg=red). - for x := 1; x <= 10; x++ { + // Cells 4..10 are trailing and should be default with no reverse. + for x := 4; x <= 10; x++ { _, style, _ := Screen.Get(x, 1) - assert.Equal(t, color.Maroon, style.GetForeground(), - "cell at (%d, 1) should have red fg under reverse", x) - assert.True(t, style.HasReverse(), - "cell at (%d, 1) should have reverse attribute", x) + assert.Equal(t, tcell.ColorDefault, style.GetForeground(), + "trailing cell at (%d, 1) should have default fg", x) + assert.False(t, style.HasReverse(), + "trailing cell at (%d, 1) should not have reverse attribute", x) } } From 51b409383c6b74bda724a2f134de5fd0a90b4f56 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 20 May 2026 08:34:34 +0200 Subject: [PATCH 7/7] Extend the fill background to wrapped tail segments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tools like delta paint each diff line's background with '\x1b[K' so the color reaches the right edge. Up to now the '\x1b[K' handler appended (InnerWidth - cx) explicit padding cells with the fill bg so rendering picked up the color. That worked for short lines but silently degraded once content exceeded InnerWidth: the repeat count went non-positive, no cells were added, and after wrapping the partial tail segment was left without any cells carrying the fill color, so draw() fell back to the view's default bg. Record the fill colors on the source line as optional trailingFillAttributes. In the '\x1b[K' handler set them (and drop the padding-cell loop — the metadata covers both the wrap and the non-wrap cases). In draw(), once per source line, pick the trailing cell's fg/bg from the metadata if present and otherwise from the view defaults; then the inner-loop fills past-content cells with that. Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/gocui/view.go | 72 ++++++++++++++++++++++++++++++++++-------- pkg/gocui/view_test.go | 32 ++++++------------- 2 files changed, 68 insertions(+), 36 deletions(-) diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index e8947a665..77492b9b1 100644 --- a/pkg/gocui/view.go +++ b/pkg/gocui/view.go @@ -444,12 +444,28 @@ type SearchPosition struct { type viewLine struct { linesX, linesY int // coordinates relative to v.lines line []cell + + // Colors used to extend the bg past this wrapped segment's content. + // Derived at wrap time from the source line — see refreshViewLinesIfNeeded + // for the per-segment rule. + trailingFillAttributes *trailingFillAttributes } -// lineType is one of v.lines: the cells of a source lineType, plus any per-lineType -// metadata about how it was terminated (added in later commits). +// lineType is one of v.lines: the cells of a source line, plus optional +// trailingFillAttributes recording the colors used to extend the bg +// past the line's content when the writer emitted '\x1b[K'. type lineType struct { - cells cells + cells cells + trailingFillAttributes *trailingFillAttributes +} + +// trailingFillAttributes describes the fg/bg colors that draw() should +// use for cells past the end of a wrapped segment's content. On a source +// line this records what the writer asked for via '\x1b[K' (and so opts +// the line in to trailing fill at all); the per-segment values on each +// viewLine are derived from it at wrap time. +type trailingFillAttributes struct { + fg, bg Attribute } type cell struct { @@ -953,16 +969,19 @@ func (v *View) parseInput(ch []byte, width int, x int, _ int) (bool, []cell) { } else { repeatCount := 1 if _, ok := v.ei.instruction.(eraseInLineFromCursor); ok { - // fill rest of line + // Discard any old content past the cursor and record the + // fill colors so draw() paints the trailing area with them. + // This extends the bg to the right edge in both the + // content-fits and content-wraps cases — for the latter, + // the metadata is what reaches every wrapped segment past + // the last word. v.ei.instructionRead() - cx := 0 - for _, cell := range v.lines[v.wy].cells[0:v.wx] { - cx += cell.width - } - repeatCount = v.InnerWidth() - cx - ch = []byte{' '} - width = 1 truncateLine = true + v.lines[v.wy].trailingFillAttributes = &trailingFillAttributes{ + fg: v.ei.curFgColor, + bg: v.ei.curBgColor, + } + return truncateLine, []cell{} } else if isEscape { // do not output anything return truncateLine, nil @@ -1252,6 +1271,15 @@ func (v *View) draw() { break } + // Decide the colors used for cells past the end of vline.line: + // the source line's trailingFillAttributes (set by '\x1b[K') if + // any, otherwise plain defaults. + trailingCell := emptyCell + if attrs := vline.trailingFillAttributes; attrs != nil { + trailingCell.fgColor = attrs.fg + trailingCell.bgColor = attrs.bg + } + // x tracks the current x position in the view, and cellIdx tracks the // index of the cell. If we print a double-sized rune, we increment cellIdx // by one but x by two. @@ -1274,7 +1302,7 @@ func (v *View) draw() { // if we're out of cells to write, we'll just print empty cells. if cellIdx > len(vline.line)-1 { - c = emptyCell + c = trailingCell } else { c = vline.line[cellIdx] } @@ -1312,7 +1340,25 @@ func (v *View) refreshViewLinesIfNeeded() { ls := lineWrap(line.cells, wrap) for j := range ls { - vline := viewLine{linesX: j, linesY: i, line: ls[j]} + // Per-segment trailing fill. When the source line opted in + // via '\x1b[K', the LAST wrapped segment uses those colors + // directly; earlier segments use the colors of their own + // last cell, so the trailing area matches the bg active + // where that segment ended rather than bleeding the + // '\x1b[K' bg back across color changes in the line. + var attrs *trailingFillAttributes + if line.trailingFillAttributes != nil { + if j == len(ls)-1 { + attrs = line.trailingFillAttributes + } else if len(ls[j]) > 0 { + last := ls[j][len(ls[j])-1] + attrs = &trailingFillAttributes{fg: last.fgColor, bg: last.bgColor} + } + } + vline := viewLine{ + linesX: j, linesY: i, line: ls[j], + trailingFillAttributes: attrs, + } if lineIdx > len(v.viewLines)-1 { v.viewLines = append(v.viewLines, vline) diff --git a/pkg/gocui/view_test.go b/pkg/gocui/view_test.go index 9d0e9a4ef..f65418821 100644 --- a/pkg/gocui/view_test.go +++ b/pkg/gocui/view_test.go @@ -508,10 +508,10 @@ func TestShortFilledLineExtendsBgWithoutWrap(t *testing.T) { } } -// TestWrappedFilledLineExtendsBgToEdge demonstrates that when a line is +// TestWrappedFilledLineExtendsBgToEdge verifies that when a line is // filled to the edge with \x1b[K (the pattern used by `delta` for diff -// lines) but exceeds the view's inner width, every wrapped segment loses -// the fill background past its content. +// lines) but exceeds the view's inner width, every wrapped segment +// extends the fill background past its content to the right edge. func TestWrappedFilledLineExtendsBgToEdge(t *testing.T) { WithSimulationScreen(t, 14, 6) @@ -524,24 +524,18 @@ func TestWrappedFilledLineExtendsBgToEdge(t *testing.T) { // Content with spaces so word wrap ends each segment before the // right edge: "aaa bbb ccc ddd eee" wraps at InnerWidth=10 to three // segments — "aaa bbb" / "ccc ddd" / "eee". Each row's trailing area - // should pick up the red fill from \x1b[K but currently falls back to - // the view default bg. + // must pick up the red fill from \x1b[K. v.writeString("\x1b[41m" + "aaa bbb ccc ddd eee" + "\x1b[0m\x1b[41m\x1b[K\x1b[0m\n") v.draw() - // trailingFrom is 1-indexed: each row's content ends at column - // trailingFrom[y]-1, so columns trailingFrom[y]..10 are the trailing - // fill area where the bug shows. - trailingFrom := []int{8, 8, 4} + // All three wrapped rows should have the red fill background across + // the full InnerWidth, including the trailing cells past each row's + // last word. for y := 1; y <= 3; y++ { - for x := trailingFrom[y-1]; x <= 10; x++ { + for x := 1; x <= 10; x++ { _, style, _ := Screen.Get(x, y) - /* EXPECTED: assert.Equal(t, color.Maroon, style.GetBackground(), - "trailing cell at (%d, %d) should have red bg", x, y) - ACTUAL: */ - assert.Equal(t, tcell.ColorDefault, style.GetBackground(), - "trailing cell at (%d, %d) falls back to default bg", x, y) + "cell at (%d, %d) should have red bg", x, y) } } } @@ -572,12 +566,8 @@ func TestMulticolorWrappedFillUsesLastCellOfEachSegment(t *testing.T) { // 8..10 should pick up red rather than the \x1b[K's green. for x := 8; x <= 10; x++ { _, style, _ := Screen.Get(x, 1) - /* EXPECTED: assert.Equal(t, color.Maroon, style.GetBackground(), "trailing cell at (%d, 1) should have red bg (matching segment's last cell)", x) - ACTUAL: */ - assert.Equal(t, tcell.ColorDefault, style.GetBackground(), - "trailing cell at (%d, 1) falls back to default bg", x) } // Row 2's content ends with a green cell at x=3, so trailing @@ -585,11 +575,7 @@ func TestMulticolorWrappedFillUsesLastCellOfEachSegment(t *testing.T) { // last cell and the \x1b[K bg — these happen to agree here). for x := 4; x <= 10; x++ { _, style, _ := Screen.Get(x, 2) - /* EXPECTED: assert.Equal(t, color.Green, style.GetBackground(), "trailing cell at (%d, 2) should have green bg", x) - ACTUAL: */ - assert.Equal(t, tcell.ColorDefault, style.GetBackground(), - "trailing cell at (%d, 2) falls back to default bg", x) } }