From 9c8a02f9014b27f24c18e2d9da1841a226becefe Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 20 May 2026 08:32:39 +0200 Subject: [PATCH] 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) } }