diff --git a/pkg/gocui/flush_test.go b/pkg/gocui/flush_test.go new file mode 100644 index 000000000..2447901f5 --- /dev/null +++ b/pkg/gocui/flush_test.go @@ -0,0 +1,202 @@ +package gocui + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func newTestGui(t *testing.T) *Gui { + t.Helper() + g, err := NewGui(NewGuiOpts{ + OutputMode: OutputNormal, + Headless: true, + Width: 80, + Height: 24, + }) + assert.NoError(t, err) + t.Cleanup(func() { g.Close() }) + return g +} + +// setupViews creates a few views and does an initial full flush so all views +// start in a clean (non-tainted) state. +func setupViews(t *testing.T, g *Gui) (*View, *View) { + t.Helper() + + status, _ := g.SetView("status", 0, 22, 40, 24, 0) + status.Frame = false + main, _ := g.SetView("main", 0, 0, 80, 22, 0) + + // Initial content + status.SetContent("Ready") + main.SetContent("hello world") + + // Full flush to draw everything and clear tainted flags + assert.NoError(t, g.flush()) + + return status, main +} + +// pushContentOnly pushes a content-only event directly to the channel +// (synchronous, deterministic — unlike Update which spawns a goroutine). +func pushContentOnly(g *Gui, f func(*Gui) error) { + g.userEvents <- userEvent{f: f, task: g.NewTask(), contentOnly: true} +} + +// pushRegular pushes a regular event directly to the channel. +func pushRegular(g *Gui, f func(*Gui) error) { + g.userEvents <- userEvent{f: f, task: g.NewTask(), contentOnly: false} +} + +func TestFlushContentOnly_SkipsUntaintedViews(t *testing.T) { + g := newTestGui(t) + status, main := setupViews(t, g) + + // After initial flush, both views should be untainted + assert.False(t, status.IsTainted(), "status view should not be tainted after flush") + assert.False(t, main.IsTainted(), "main view should not be tainted after flush") + + // Modify only the status view + status.SetContent("Fetching /") + + assert.True(t, status.IsTainted(), "status view should be tainted after SetContent") + assert.False(t, main.IsTainted(), "main view should not be tainted (was not modified)") + + // flushContentOnly should succeed and clear status tainted flag + assert.NoError(t, g.flushContentOnly()) + + assert.False(t, status.IsTainted(), "status view should not be tainted after flushContentOnly") + assert.False(t, main.IsTainted(), "main view should not be tainted after flushContentOnly") +} + +func TestFlushContentOnly_WritesCorrectContent(t *testing.T) { + g := newTestGui(t) + status, _ := setupViews(t, g) + + status.SetContent("Fetching |") + assert.NoError(t, g.flushContentOnly()) + + assert.Equal(t, "Fetching |", status.Buffer()) +} + +func TestProcessEvent_ContentOnlyEvent_SkipsTaintedCheck(t *testing.T) { + g := newTestGui(t) + status, main := setupViews(t, g) + + // Send a content-only event that modifies only the status view + pushContentOnly(g, func(gui *Gui) error { + status.SetContent("Fetching /") + return nil + }) + + assert.NoError(t, g.processEvent()) + + // status was modified and drawn → tainted cleared + assert.False(t, status.IsTainted(), "status should not be tainted after processEvent with contentOnly") + // main was NOT modified → should still be untainted + assert.False(t, main.IsTainted(), "main should not be tainted after processEvent with contentOnly") +} + +func TestProcessEvent_RegularEvent_UsesFullFlush(t *testing.T) { + g := newTestGui(t) + status, _ := setupViews(t, g) + + // Regular event (not content-only) should trigger full flush + pushRegular(g, func(gui *Gui) error { + status.SetContent("Fetching \\") + return nil + }) + + assert.NoError(t, g.processEvent()) + + assert.False(t, status.IsTainted(), "status should not be tainted after full flush") +} + +func TestProcessEvent_MixedBatch_UsesFullFlush(t *testing.T) { + g := newTestGui(t) + status, main := setupViews(t, g) + + // Queue a content-only event followed by a regular event. + // processEvent picks up the first; processRemainingEvents picks up + // the second. Since the second is not contentOnly, full flush runs. + pushContentOnly(g, func(gui *Gui) error { + status.SetContent("Fetching -") + return nil + }) + pushRegular(g, func(gui *Gui) error { + main.SetContent("updated main") + return nil + }) + + assert.NoError(t, g.processEvent()) + + // Both views were modified and should have been drawn by full flush + assert.False(t, status.IsTainted(), "status should not be tainted after full flush") + assert.False(t, main.IsTainted(), "main should not be tainted after full flush") +} + +func TestProcessEvent_RegularThenContentOnly_UsesFullFlush(t *testing.T) { + g := newTestGui(t) + status, main := setupViews(t, g) + + // Even if a regular event comes first and the remaining are contentOnly, + // the batch must use full flush. + pushRegular(g, func(gui *Gui) error { + main.SetContent("new main content") + return nil + }) + pushContentOnly(g, func(gui *Gui) error { + status.SetContent("Fetching |") + return nil + }) + + assert.NoError(t, g.processEvent()) + + assert.False(t, status.IsTainted(), "status should not be tainted after full flush") + assert.False(t, main.IsTainted(), "main should not be tainted after full flush") +} + +func TestProcessRemainingEvents_AllContentOnly_ReturnsTrue(t *testing.T) { + g := newTestGui(t) + status, _ := setupViews(t, g) + + pushContentOnly(g, func(gui *Gui) error { + status.SetContent("a") + return nil + }) + pushContentOnly(g, func(gui *Gui) error { + status.SetContent("b") + return nil + }) + + contentOnly, err := g.processRemainingEvents() + assert.NoError(t, err) + assert.True(t, contentOnly, "should return true when all events are contentOnly") +} + +func TestProcessRemainingEvents_MixedEvents_ReturnsFalse(t *testing.T) { + g := newTestGui(t) + status, _ := setupViews(t, g) + + pushContentOnly(g, func(gui *Gui) error { + status.SetContent("a") + return nil + }) + pushRegular(g, func(gui *Gui) error { + status.SetContent("b") + return nil + }) + + contentOnly, err := g.processRemainingEvents() + assert.NoError(t, err) + assert.False(t, contentOnly, "should return false when any event is not contentOnly") +} + +func TestProcessRemainingEvents_EmptyQueue_ReturnsTrue(t *testing.T) { + g := newTestGui(t) + + contentOnly, err := g.processRemainingEvents() + assert.NoError(t, err) + assert.True(t, contentOnly, "should return true when no events are queued") +} diff --git a/pkg/gocui/gui.go b/pkg/gocui/gui.go index dded5dd4a..73e358f55 100644 --- a/pkg/gocui/gui.go +++ b/pkg/gocui/gui.go @@ -604,6 +604,10 @@ func (g *Gui) SetRenderSearchStatusFunc(renderSearchStatusFunc func(*View, int, type userEvent struct { f func(*Gui) error task Task + // Signals that this event only modifies view content (e.g. SetContent). + // When all events in a batch are contentOnly, processEvent + // can skip the expensive layout() call in flush(). + contentOnly bool } // Update executes the passed function. This method can be called safely from a @@ -630,6 +634,12 @@ func (g *Gui) updateAsyncAux(f func(*Gui) error, task Task) { g.userEvents <- userEvent{f: f, task: task} } +// Like Update, but signals that the callback only modifies content. +func (g *Gui) UpdateContentOnly(f func(*Gui) error) { + task := g.NewTask() + g.userEvents <- userEvent{f: f, task: task, contentOnly: true} +} + // Calls a function in a goroutine. Handles panics gracefully and tracks // number of background tasks. // Always use this when you want to spawn a goroutine and you want lazygit to @@ -743,6 +753,8 @@ func (g *Gui) handleError(err error) error { } func (g *Gui) processEvent() error { + contentOnly := false + select { case ev := <-g.gEvents: task := g.NewTask() @@ -752,6 +764,7 @@ func (g *Gui) processEvent() error { return err } case ev := <-g.userEvents: + contentOnly = ev.contentOnly defer func() { ev.task.Done() }() if err := g.handleError(ev.f(g)); err != nil { @@ -759,32 +772,38 @@ func (g *Gui) processEvent() error { } } - if err := g.processRemainingEvents(); err != nil { - return err - } - if err := g.flush(); err != nil { + remainingContentOnly, err := g.processRemainingEvents() + if err != nil { return err } + contentOnly = contentOnly && remainingContentOnly - return nil + if contentOnly { + return g.flushContentOnly() + } + return g.flush() } // processRemainingEvents handles the remaining events in the events pool. -func (g *Gui) processRemainingEvents() error { +// Returns true if all processed events were content-only. +func (g *Gui) processRemainingEvents() (bool, error) { + contentOnly := true for { select { case ev := <-g.gEvents: + contentOnly = false if err := g.handleError(g.handleEvent(&ev)); err != nil { - return err + return false, err } case ev := <-g.userEvents: + contentOnly = ev.contentOnly && contentOnly err := g.handleError(ev.f(g)) ev.task.Done() if err != nil { - return err + return false, err } default: - return nil + return contentOnly, nil } } } @@ -1169,6 +1188,23 @@ func (g *Gui) ForceRedrawViews(views ...*View) error { return nil } +// 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. +func (g *Gui) flushContentOnly() error { + for _, v := range g.views { + if !v.tainted { + continue + } + if err := g.draw(v); err != nil { + return err + } + } + + Screen.Show() + return nil +} + // draw manages the cursor and calls the draw function of a view. func (g *Gui) draw(v *View) error { if g.suspended { diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index 1f49b857a..458e33804 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -1188,6 +1188,12 @@ func (gui *Gui) onUIThread(f func() error) { }) } +func (gui *Gui) onUIThreadContentOnly(f func() error) { + gui.g.UpdateContentOnly(func(*gocui.Gui) error { + return f() + }) +} + func (gui *Gui) onWorker(f func(gocui.Task) error) { gui.g.OnWorker(f) } diff --git a/pkg/gui/gui_common.go b/pkg/gui/gui_common.go index 13f49c46b..07945c350 100644 --- a/pkg/gui/gui_common.go +++ b/pkg/gui/gui_common.go @@ -120,6 +120,10 @@ func (self *guiCommon) OnUIThread(f func() error) { self.gui.onUIThread(f) } +func (self *guiCommon) OnUIThreadContentOnly(f func() error) { + self.gui.onUIThreadContentOnly(f) +} + func (self *guiCommon) OnWorker(f func(gocui.Task) error) { self.gui.onWorker(f) } diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index 0afbd5ed8..5856bb5e0 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -71,6 +71,10 @@ type IGuiCommon interface { // Only necessary to call if you're not already on the UI thread i.e. you're inside a goroutine. // All controller handlers are executed on the UI thread. OnUIThread(f func() error) + // Like OnUIThread, but signals that the callback only modifies view + // content (e.g. spinner), allows the event loop to skip + // the expensive layout recalculation when only content changed. + OnUIThreadContentOnly(f func() error) // Runs a function in a goroutine. Use this whenever you want to run a goroutine and keep track of the fact // that lazygit is still busy. See docs/dev/Busy.md OnWorker(f func(gocui.Task) error)