1
0
mirror of https://github.com/jesseduffield/lazygit.git synced 2026-05-18 10:01:09 +02:00

When drawing tainted views in ForceFlushViewsContentOnly, also draw views that overlap them

This prevents views from drawing over higher z-order views.

Currently this is not an issue in practice, because we use
ForceFlushViewsContentOnly only for the bottom line status spinner, and there
are never views on top of it. However, later in the branch we will use the
mechanism to redraw the inline spinners in panels (e.g. the "Pushing..." status
next to a branch name), and there could be a popup on top of it.

Co-authored-by: Stefan Haller <stefan@haller-berlin.de>
This commit is contained in:
Antoine Gaudreau Simard
2026-05-07 21:49:00 -04:00
committed by Stefan Haller
parent 4f0393f97b
commit b3deef31ad
2 changed files with 124 additions and 4 deletions
+90
View File
@@ -1,6 +1,7 @@
package gocui
import (
"strings"
"testing"
"github.com/stretchr/testify/assert"
@@ -200,3 +201,92 @@ func TestProcessRemainingEvents_EmptyQueue_ReturnsTrue(t *testing.T) {
assert.NoError(t, err)
assert.True(t, contentOnly, "should return true when no events are queued")
}
// Ensure an overlapping view that is not tainted does not get overdrawn
func TestFlushContentOnly_DoesNotOverdrawHigherZViews(t *testing.T) {
g := newTestGui(t)
// Base view
list, _ := g.SetView("list", 0, 0, 79, 23, 0)
list.Frame = false
list.SetContent(strings.Repeat("LIST LINE FILLER FILLER FILLER FILLER FILLER FILLER FILLER FILLER FILLER\n", 22))
// Overlapping 'popup'
popup, _ := g.SetView("popup", 20, 8, 60, 16, 0)
popup.Frame = false
popupLine := strings.Repeat("P", 60)
popup.SetContent(strings.Repeat(popupLine+"\n", 16))
// Full flush — popup ends up on top.
assert.NoError(t, g.flush())
cellAt := func(x, y int) string {
s, _, _ := g.screen.Get(x, y)
return s
}
// Taint only the list view
list.SetContent(strings.Repeat(strings.Repeat("X", 80)+"\n", 22))
assert.True(t, list.IsTainted(), "list should be tainted after SetContent")
assert.False(t, popup.IsTainted(), "popup should not be tainted")
// flushContentOnly is what spinner ticks ultimately invoke.
assert.NoError(t, g.flushContentOnly(g.views))
assert.Equal(t, "P", cellAt(21, 9),
"popup region must still show popup content after flushContentOnly; "+
"if this fails the popup-overdraw bug is present")
// Additional checks to be sure
assert.Equal(t, "P", cellAt(40, 11), "interior popup cell should still show popup content")
assert.Equal(t, "P", cellAt(58, 14), "near-edge popup cell should still show popup content")
// Ensure tainted view was updated
assert.Equal(t, "X", cellAt(5, 5), "list cell outside popup should show new list content")
assert.Equal(t, "X", cellAt(70, 20), "list cell outside popup should show new list content")
}
// Ensure transitive overlap: with views in z-order [a, b, c] where b overlaps a
// and c overlaps b but c does NOT overlap a, tainting a must redraw all three —
// otherwise b's redraw paints over c.
func TestFlushContentOnly_RedrawsTransitivelyOverlappingViews(t *testing.T) {
g := newTestGui(t)
// Geometry: b straddles a and c; a and c are disjoint.
// a: (0,0)-(40,10) b: (30,5)-(60,15) c: (50,12)-(75,20)
a, _ := g.SetView("a", 0, 0, 40, 10, 0)
a.Frame = false
a.SetContent(strings.Repeat(strings.Repeat("A", 60)+"\n", 20))
b, _ := g.SetView("b", 30, 5, 60, 15, 0)
b.Frame = false
b.SetContent(strings.Repeat(strings.Repeat("B", 60)+"\n", 20))
c, _ := g.SetView("c", 50, 12, 75, 20, 0)
c.Frame = false
c.SetContent(strings.Repeat(strings.Repeat("C", 60)+"\n", 20))
assert.NoError(t, g.flush())
cellAt := func(x, y int) string {
s, _, _ := g.screen.Get(x, y)
return s
}
// Taint only a.
a.SetContent(strings.Repeat(strings.Repeat("X", 60)+"\n", 20))
assert.True(t, a.IsTainted())
assert.False(t, b.IsTainted())
assert.False(t, c.IsTainted())
assert.NoError(t, g.flushContentOnly(g.views))
// a redrawn (direct).
assert.Equal(t, "X", cellAt(5, 5), "a should be redrawn (tainted)")
// b redrawn (overlaps a).
assert.Equal(t, "B", cellAt(45, 7), "b should be redrawn (overlaps a)")
// c redrawn transitively (overlaps b, which overlaps a). Without the
// transitive case, b's redraw would paint over c at this cell.
assert.Equal(t, "C", cellAt(55, 14),
"c should be redrawn transitively; if 'B' here, b's redraw painted over c")
}
+34 -4
View File
@@ -13,7 +13,9 @@ import (
"github.com/gdamore/tcell/v3"
"github.com/go-errors/errors"
"github.com/jesseduffield/generics/set"
"github.com/rivo/uniseg"
"github.com/samber/lo"
)
// OutputMode represents an output mode, which determines how colors
@@ -1170,11 +1172,9 @@ func (g *Gui) flush() error {
// Redraws only tainted views and skips the layout pass.
// tcell's cell-level dirty tracking ensures only
// actually-changed cells are emitted to the terminal.
// Will also redraw any views that overlap tainted views
func (g *Gui) flushContentOnly(views []*View) error {
for _, v := range views {
if !v.tainted {
continue
}
for _, v := range viewsToRedrawContentOnly(views) {
if err := g.draw(v); err != nil {
return err
}
@@ -1184,6 +1184,36 @@ func (g *Gui) flushContentOnly(views []*View) error {
return nil
}
func viewsToRedrawContentOnly(views []*View) []*View {
redrawIndexes := set.New[int]()
for i, v := range views {
if !v.tainted && !redrawIndexes.Includes(i) {
continue
}
redrawIndexes.Add(i)
for j, above := range views[i+1:] {
aboveIndex := i + 1 + j
if !redrawIndexes.Includes(aboveIndex) && rectsOverlap(v, above) {
redrawIndexes.Add(aboveIndex)
}
}
}
return lo.FilterMap(views, func(view *View, i int) (*View, bool) {
return view, redrawIndexes.Includes(i)
})
}
// Reports whether two views' rectangles share at least one cell.
func rectsOverlap(a, b *View) bool {
ax0, ay0, ax1, ay1 := a.Dimensions()
bx0, by0, bx1, by1 := b.Dimensions()
return ax0 <= bx1 && ax1 >= bx0 && ay0 <= by1 && ay1 >= by0
}
func (g *Gui) ForceLayoutAndRedraw() error {
return g.flush()
}