From 164f7da33dc82610bd910b71a566e15170414fe4 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 4 May 2026 18:39:54 +0200 Subject: [PATCH 1/6] Fix missing layout call after switching repos This didn't cause a bug so far because switching repos always happens from within an OnWaitingStatus, so the spinner would take care of calling layout and draw. However, later in this branch we are going to optimize the spinner so that it no longer calls layout, at which point this would break, so make sure we rerender at the point where it's needed. --- pkg/gui/gui.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index ce22f42404b..1f49b857a51 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -428,6 +428,8 @@ func (gui *Gui) onNewRepo(startArgs appTypes.StartArgs, contextKey types.Context gui.c.Context().Push(contextToPush, types.OnFocusOpts{}) + gui.render() + return nil } From 79727f1780db4ba5d9c627930d08fb47ea98dab3 Mon Sep 17 00:00:00 2001 From: Antoine Gaudreau Simard Date: Mon, 4 May 2026 14:53:33 -0400 Subject: [PATCH 2/6] Honor the spinner rate configurate with sync spinner --- pkg/gui/controllers/helpers/app_status_helper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/gui/controllers/helpers/app_status_helper.go b/pkg/gui/controllers/helpers/app_status_helper.go index 89de221b8b2..e7a75f17cee 100644 --- a/pkg/gui/controllers/helpers/app_status_helper.go +++ b/pkg/gui/controllers/helpers/app_status_helper.go @@ -111,7 +111,7 @@ func (self *AppStatusHelper) renderAppStatus() { func (self *AppStatusHelper) renderAppStatusSync(stop chan struct{}) { go func() { - ticker := time.NewTicker(time.Millisecond * 50) + ticker := time.NewTicker(time.Millisecond * time.Duration(self.c.UserConfig().Gui.Spinner.Rate)) defer ticker.Stop() // Forcing a re-layout and redraw after we added the waiting status; From 9c3e7dac888394543513b93cc0cd4b84c19ff481 Mon Sep 17 00:00:00 2001 From: Antoine Gaudreau Simard Date: Tue, 5 May 2026 18:34:51 -0400 Subject: [PATCH 3/6] Support to request a content-only UI refresh This skips the whole-UI layout calculations, and lets tcell's dirty cell handling redraw only changed cells. --- pkg/gocui/flush_test.go | 202 ++++++++++++++++++++++++++++++++++++++++ pkg/gocui/gui.go | 54 +++++++++-- pkg/gui/gui.go | 6 ++ pkg/gui/gui_common.go | 4 + pkg/gui/types/common.go | 4 + 5 files changed, 261 insertions(+), 9 deletions(-) create mode 100644 pkg/gocui/flush_test.go diff --git a/pkg/gocui/flush_test.go b/pkg/gocui/flush_test.go new file mode 100644 index 00000000000..2447901f53a --- /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 dded5dd4a45..73e358f55e8 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 1f49b857a51..458e338046f 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 13f49c46bbc..07945c350ed 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 0afbd5ed8a7..5856bb5e0fb 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) From 0d195077e4a32a4241b19b1c29939a2f274fc3dd Mon Sep 17 00:00:00 2001 From: Antoine Gaudreau Simard Date: Tue, 5 May 2026 18:35:50 -0400 Subject: [PATCH 4/6] Improve performance when drawing the spinner on background events --- pkg/gui/controllers/helpers/app_status_helper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/gui/controllers/helpers/app_status_helper.go b/pkg/gui/controllers/helpers/app_status_helper.go index e7a75f17cee..4a6e7726e12 100644 --- a/pkg/gui/controllers/helpers/app_status_helper.go +++ b/pkg/gui/controllers/helpers/app_status_helper.go @@ -96,7 +96,7 @@ func (self *AppStatusHelper) renderAppStatus() { for range ticker.C { appStatus, color := self.statusMgr().GetStatusString(self.c.UserConfig()) self.c.Views().AppStatus.FgColor = color - self.c.OnUIThread(func() error { + self.c.OnUIThreadContentOnly(func() error { self.c.SetViewContent(self.c.Views().AppStatus, appStatus) return nil }) From 5dcc93e8cc67b22e5493fedb7fbb0c59cd1689df Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 3 May 2026 14:14:23 +0200 Subject: [PATCH 5/6] Validate that gui.spinner.frames must all have the same width The spinner looks weird if they don't. While we're at it, validate that frames must not be empty, which would have crashed with a division by zero. --- pkg/config/user_config_validation.go | 19 ++++++++++++++ pkg/config/user_config_validation_test.go | 30 +++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/pkg/config/user_config_validation.go b/pkg/config/user_config_validation.go index 23215eab607..7eeab32dd52 100644 --- a/pkg/config/user_config_validation.go +++ b/pkg/config/user_config_validation.go @@ -1,6 +1,7 @@ package config import ( + "errors" "fmt" "log" "reflect" @@ -8,6 +9,8 @@ import ( "strings" "github.com/jesseduffield/lazygit/pkg/constants" + "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/samber/lo" ) func (config *UserConfig) Validate() error { @@ -49,6 +52,22 @@ func (config *UserConfig) Validate() error { if err := validateCustomCommands(config.CustomCommands); err != nil { return err } + if err := validateSpinner(config.Gui.Spinner); err != nil { + return err + } + return nil +} + +func validateSpinner(spinner SpinnerConfig) error { + if len(spinner.Frames) == 0 { + return errors.New("gui.spinner.frames must not be empty.") + } + firstWidth := utils.StringWidth(spinner.Frames[0]) + if lo.SomeBy(spinner.Frames, func(frame string) bool { + return utils.StringWidth(frame) != firstWidth + }) { + return errors.New("All gui.spinner.frames entries must have the same width.") + } return nil } diff --git a/pkg/config/user_config_validation_test.go b/pkg/config/user_config_validation_test.go index 7fbfd00d42d..a2841684d52 100644 --- a/pkg/config/user_config_validation_test.go +++ b/pkg/config/user_config_validation_test.go @@ -289,3 +289,33 @@ func TestUserConfigValidate_enums(t *testing.T) { }) } } + +func TestUserConfigValidate_spinnerFrames(t *testing.T) { + scenarios := []struct { + name string + frames []string + valid bool + }{ + {name: "empty", frames: []string{}, valid: false}, + {name: "single frame", frames: []string{"|"}, valid: true}, + {name: "all same width", frames: []string{"|", "/", "-", "\\"}, valid: true}, + {name: "all same width, multi-char", frames: []string{". ", ".. ", "..."}, valid: true}, + {name: "all same width, wide runes", frames: []string{"⠋", "⠙", "⠹"}, valid: true}, + {name: "differing widths", frames: []string{"|", "//"}, valid: false}, + {name: "first differs from rest", frames: []string{"||", "/", "-"}, valid: false}, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + config := GetDefaultConfig() + config.Gui.Spinner.Frames = s.frames + err := config.Validate() + + if s.valid { + assert.NoError(t, err) + } else { + assert.Error(t, err) + } + }) + } +} From b7edcbad3a805990dbc4198c44e4066543121bb4 Mon Sep 17 00:00:00 2001 From: Antoine Gaudreau Simard Date: Tue, 5 May 2026 18:39:07 -0400 Subject: [PATCH 6/6] Improve performance of spinner in Synchronized events Instead of redrawing the two views on each tick, only redraw the spinner --- pkg/gocui/flush_test.go | 4 +- pkg/gocui/gui.go | 38 +++++++------------ .../controllers/helpers/app_status_helper.go | 2 +- 3 files changed, 17 insertions(+), 27 deletions(-) diff --git a/pkg/gocui/flush_test.go b/pkg/gocui/flush_test.go index 2447901f53a..a27943d7c98 100644 --- a/pkg/gocui/flush_test.go +++ b/pkg/gocui/flush_test.go @@ -64,7 +64,7 @@ func TestFlushContentOnly_SkipsUntaintedViews(t *testing.T) { 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.NoError(t, g.flushContentOnly(g.views)) 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") @@ -75,7 +75,7 @@ func TestFlushContentOnly_WritesCorrectContent(t *testing.T) { status, _ := setupViews(t, g) status.SetContent("Fetching |") - assert.NoError(t, g.flushContentOnly()) + assert.NoError(t, g.flushContentOnly(g.views)) assert.Equal(t, "Fetching |", status.Buffer()) } diff --git a/pkg/gocui/gui.go b/pkg/gocui/gui.go index 73e358f55e8..dac7295a3f1 100644 --- a/pkg/gocui/gui.go +++ b/pkg/gocui/gui.go @@ -779,7 +779,7 @@ func (g *Gui) processEvent() error { contentOnly = contentOnly && remainingContentOnly if contentOnly { - return g.flushContentOnly() + return g.flushContentOnly(g.views) } return g.flush() } @@ -1167,32 +1167,11 @@ func (g *Gui) flush() error { return nil } -func (g *Gui) ForceLayoutAndRedraw() error { - return g.flush() -} - -// force redrawing one or more views outside of the normal main loop. Useful during longer -// operations that block the main thread, to update a spinner in a status view. -func (g *Gui) ForceRedrawViews(views ...*View) error { - for _, m := range g.managers { - if err := m.Layout(g); err != nil { - return err - } - } - - for _, v := range views { - v.draw() - } - - Screen.Show() - 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 { +func (g *Gui) flushContentOnly(views []*View) error { + for _, v := range views { if !v.tainted { continue } @@ -1205,6 +1184,17 @@ func (g *Gui) flushContentOnly() error { return nil } +func (g *Gui) ForceLayoutAndRedraw() error { + return g.flush() +} + +// Redraws only tainted views outside of the normal main +// loop, without a layout pass. Useful during longer operations that block the +// main thread, e.g. to update a spinner in a status view. +func (g *Gui) ForceFlushViewsContentOnly(views []*View) error { + return g.flushContentOnly(views) +} + // 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/controllers/helpers/app_status_helper.go b/pkg/gui/controllers/helpers/app_status_helper.go index 4a6e7726e12..d71faeb6542 100644 --- a/pkg/gui/controllers/helpers/app_status_helper.go +++ b/pkg/gui/controllers/helpers/app_status_helper.go @@ -136,7 +136,7 @@ func (self *AppStatusHelper) renderAppStatusSync(stop chan struct{}) { self.c.Views().AppStatus, self.c.Views().Options, self.c.Views().Information, self.c.Views().StatusSpacer1, self.c.Views().StatusSpacer2, } - _ = self.c.GocuiGui().ForceRedrawViews(bottomLineViews...) + _ = self.c.GocuiGui().ForceFlushViewsContentOnly(bottomLineViews) case <-stop: break outer }