diff --git a/pkg/gocui/flush_test.go b/pkg/gocui/flush_test.go index a27943d7c98..59bae427ccf 100644 --- a/pkg/gocui/flush_test.go +++ b/pkg/gocui/flush_test.go @@ -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") +} diff --git a/pkg/gocui/gui.go b/pkg/gocui/gui.go index dac7295a3f1..645329d6b10 100644 --- a/pkg/gocui/gui.go +++ b/pkg/gocui/gui.go @@ -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() } diff --git a/pkg/gui/context/list_context_trait.go b/pkg/gui/context/list_context_trait.go index 98833fdb2f7..597fc99df29 100644 --- a/pkg/gui/context/list_context_trait.go +++ b/pkg/gui/context/list_context_trait.go @@ -124,7 +124,6 @@ func (self *ListContextTrait) HandleRender() { content := self.renderLines(-1, -1) self.GetViewTrait().SetContent(content) } - self.c.Render() self.setFooter() } diff --git a/pkg/gui/context/suggestions_context.go b/pkg/gui/context/suggestions_context.go index eafe7fb7cda..fb69b34d98e 100644 --- a/pkg/gui/context/suggestions_context.go +++ b/pkg/gui/context/suggestions_context.go @@ -67,10 +67,17 @@ func NewSuggestionsContext( } func (self *SuggestionsContext) SetSuggestions(suggestions []*types.Suggestion) { - self.State.Suggestions = suggestions - self.SetSelection(0) - self.c.ResetViewOrigin(self.GetView()) - self.HandleRender() + // SetSuggestions is invoked from AsyncHandler (a worker goroutine) when + // the prompt input changes, as well as from prepareConfirmationPanel on + // the UI thread. Bounce to the UI thread either way so the worker path + // keeps flushing once HandleRender stops calling Render() itself. + self.c.OnUIThread(func() error { + self.State.Suggestions = suggestions + self.SetSelection(0) + self.c.ResetViewOrigin(self.GetView()) + self.HandleRender() + return nil + }) } func (self *SuggestionsContext) RefreshSuggestions() { diff --git a/pkg/gui/controllers/commits_files_controller.go b/pkg/gui/controllers/commits_files_controller.go index 7cde6c15da2..b12819c398b 100644 --- a/pkg/gui/controllers/commits_files_controller.go +++ b/pkg/gui/controllers/commits_files_controller.go @@ -476,7 +476,11 @@ func (self *CommitFilesController) toggleForPatch(selectedNodes []*filetree.Comm self.c.Git().Patch.PatchBuilder.Reset() } - self.c.PostRefreshUpdate(self.context()) + self.c.OnUIThread(func() error { + self.c.PostRefreshUpdate(self.context()) + return nil + }) + return nil }) } diff --git a/pkg/gui/controllers/helpers/app_status_helper.go b/pkg/gui/controllers/helpers/app_status_helper.go index d71faeb6542..fa402962cc4 100644 --- a/pkg/gui/controllers/helpers/app_status_helper.go +++ b/pkg/gui/controllers/helpers/app_status_helper.go @@ -6,6 +6,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/gocui" "github.com/jesseduffield/lazygit/pkg/gui/status" "github.com/jesseduffield/lazygit/pkg/gui/types" + "github.com/jesseduffield/lazygit/pkg/utils" ) type AppStatusHelper struct { @@ -93,13 +94,24 @@ func (self *AppStatusHelper) renderAppStatus() { self.c.OnWorker(func(_ gocui.Task) error { ticker := time.NewTicker(time.Millisecond * time.Duration(self.c.UserConfig().Gui.Spinner.Rate)) defer ticker.Stop() + prevAppStatus := "" for range ticker.C { appStatus, color := self.statusMgr().GetStatusString(self.c.UserConfig()) - self.c.Views().AppStatus.FgColor = color - self.c.OnUIThreadContentOnly(func() error { + + update := self.c.OnUIThreadContentOnly + if utils.StringWidth(appStatus) != utils.StringWidth(prevAppStatus) { + // Need a full layout whenever the width of the status string changes. This can't + // happen during normal spinning because we validate that all spinner frames have + // the same width, so typically this will only be triggered at the beginning and end + // of a status, or if the status string changes midway for some reason. + update = self.c.OnUIThread + } + update(func() error { + self.c.Views().AppStatus.FgColor = color self.c.SetViewContent(self.c.Views().AppStatus, appStatus) return nil }) + prevAppStatus = appStatus if appStatus == "" { break diff --git a/pkg/gui/controllers/helpers/inline_status_helper.go b/pkg/gui/controllers/helpers/inline_status_helper.go index 13c1128116e..02afcdd501e 100644 --- a/pkg/gui/controllers/helpers/inline_status_helper.go +++ b/pkg/gui/controllers/helpers/inline_status_helper.go @@ -149,7 +149,7 @@ func (self *InlineStatusHelper) stop(opts InlineStatusOpts) { } func (self *InlineStatusHelper) renderContext(contextKey types.ContextKey) { - self.c.OnUIThread(func() error { + self.c.OnUIThreadContentOnly(func() error { self.c.ContextForKey(contextKey).HandleRender() return nil }) diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 4a61bde1851..77de9ca4a0f 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -532,9 +532,12 @@ func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSele // Need to re-render the commits view because the visualization of local // branch heads might have changed - self.c.Mutexes().LocalCommitsMutex.Lock() - self.c.Contexts().LocalCommits.HandleRender() - self.c.Mutexes().LocalCommitsMutex.Unlock() + self.c.OnUIThread(func() error { + self.c.Mutexes().LocalCommitsMutex.Lock() + self.c.Contexts().LocalCommits.HandleRender() + self.c.Mutexes().LocalCommitsMutex.Unlock() + return nil + }) self.refreshStatus() } @@ -780,22 +783,27 @@ func (self *RefreshHelper) refForLog() string { } func (self *RefreshHelper) refreshView(context types.Context) { - // Re-applying the filter must be done before re-rendering the view, so that - // the filtered list model is up to date for rendering. - self.searchHelper.ReApplyFilter(context) - - self.c.PostRefreshUpdate(context) - - self.c.AfterLayout(func() error { - // Re-applying the search must be done after re-rendering the view though, - // so that the "x of y" status is shown correctly. - // - // Also, it must be done after layout, because otherwise FocusPoint - // hasn't been called yet (see ListContextTrait.FocusLine), which means - // that the scroll position might be such that the entire visible - // content is outside the viewport. And this would cause problems in - // searchModelCommits. - self.searchHelper.ReApplySearch(context) + // refreshView is called from the worker goroutine that drives async + // refreshes, so bounce to the UI thread before mutating view content. + self.c.OnUIThread(func() error { + // Re-applying the filter must be done before re-rendering the view, so that + // the filtered list model is up to date for rendering. + self.searchHelper.ReApplyFilter(context) + + self.c.PostRefreshUpdate(context) + + self.c.AfterLayout(func() error { + // Re-applying the search must be done after re-rendering the view though, + // so that the "x of y" status is shown correctly. + // + // Also, it must be done after layout, because otherwise FocusPoint + // hasn't been called yet (see ListContextTrait.FocusLine), which means + // that the scroll position might be such that the entire visible + // content is outside the viewport. And this would cause problems in + // searchModelCommits. + self.searchHelper.ReApplySearch(context) + return nil + }) return nil }) } @@ -941,7 +949,11 @@ func (self *RefreshHelper) setGithubPullRequests(authToken string, baseRemote *m self.savePullRequestsToCache(prs) self.rebuildPullRequestsMap() - self.c.PostRefreshUpdate(self.c.Contexts().Branches) + self.c.OnUIThread(func() error { + self.c.PostRefreshUpdate(self.c.Contexts().Branches) + return nil + }) + return nil } diff --git a/pkg/gui/layout.go b/pkg/gui/layout.go index 21d83ca04d9..dacd93f68bc 100644 --- a/pkg/gui/layout.go +++ b/pkg/gui/layout.go @@ -5,6 +5,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/gocui" "github.com/jesseduffield/lazygit/pkg/gui/types" + "github.com/jesseduffield/lazygit/pkg/utils" "github.com/samber/lo" ) @@ -23,7 +24,11 @@ func (gui *Gui) layout(g *gocui.Gui) error { informationStr := gui.informationStr() - appStatus := gui.helpers.AppStatus.GetStatusString() + var appStatus string + appStatusView, err := g.View("appStatus") + if err == nil { + appStatus = utils.Decolorise(appStatusView.Buffer()) + } viewDimensions := gui.getWindowDimensions(informationStr, appStatus)