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) + } + }) + } +} diff --git a/pkg/gocui/flush_test.go b/pkg/gocui/flush_test.go new file mode 100644 index 00000000000..a27943d7c98 --- /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(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") +} + +func TestFlushContentOnly_WritesCorrectContent(t *testing.T) { + g := newTestGui(t) + status, _ := setupViews(t, g) + + status.SetContent("Fetching |") + assert.NoError(t, g.flushContentOnly(g.views)) + + 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..dac7295a3f1 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(g.views) + } + 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 } } } @@ -1148,27 +1167,34 @@ 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 { +// 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(views []*View) error { + for _, v := range views { + if !v.tainted { + continue + } + if err := g.draw(v); err != nil { return err } } - for _, v := range views { - v.draw() - } - Screen.Show() 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 89de221b8b2..d71faeb6542 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 }) @@ -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; @@ -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 } diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index ce22f42404b..458e338046f 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 } @@ -1186,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)