From b4b21f9c65b54a42c4e418926db6ab5eb2cf1aea Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 23 Dec 2025 15:28:18 +0100 Subject: [PATCH 1/3] Fix race condition in HandleRender Move SetContentLineCount into OverwriteLinesAndClearEverythingElse. Calling it separately beforehand is not concurrency safe; we need both to happen when the view's writeMutex is locked. --- go.mod | 2 +- go.sum | 4 ++-- pkg/gui/context/list_context_trait.go | 3 +-- pkg/gui/context/view_trait.go | 8 ++------ pkg/gui/types/context.go | 3 +-- vendor/github.com/jesseduffield/gocui/view.go | 6 ++++-- vendor/modules.txt | 2 +- 7 files changed, 12 insertions(+), 16 deletions(-) diff --git a/go.mod b/go.mod index 278aebf6b58..e93e040a93e 100644 --- a/go.mod +++ b/go.mod @@ -18,7 +18,7 @@ require ( github.com/integrii/flaggy v1.4.0 github.com/jesseduffield/generics v0.0.0-20250517122708-b0b4a53a6f5c github.com/jesseduffield/go-git/v5 v5.14.1-0.20250407170251-e1a013310ccd - github.com/jesseduffield/gocui v0.3.1-0.20251223143206-950739ccd44a + github.com/jesseduffield/gocui v0.3.1-0.20251223144240-29fe12e8d53f github.com/jesseduffield/lazycore v0.0.0-20221012050358-03d2e40243c5 github.com/kardianos/osext v0.0.0-20190222173326-2bc1f35cddc0 github.com/karimkhaleel/jsonschema v0.0.0-20231001195015-d933f0d94ea3 diff --git a/go.sum b/go.sum index 5ef0c6593c0..b9f905f7838 100644 --- a/go.sum +++ b/go.sum @@ -194,8 +194,8 @@ github.com/jesseduffield/generics v0.0.0-20250517122708-b0b4a53a6f5c h1:tC2Paiis github.com/jesseduffield/generics v0.0.0-20250517122708-b0b4a53a6f5c/go.mod h1:F2fEBk0ddf6ixrBrJjY7phfQ3hL9rXG0uSjvwYe50bE= github.com/jesseduffield/go-git/v5 v5.14.1-0.20250407170251-e1a013310ccd h1:ViKj6qth8FgcIWizn9KiACWwPemWSymx62OPN0tHT+Q= github.com/jesseduffield/go-git/v5 v5.14.1-0.20250407170251-e1a013310ccd/go.mod h1:lRhCiBr6XjQrvcQVa+UYsy/99d3wMXn/a0nSQlhnhlA= -github.com/jesseduffield/gocui v0.3.1-0.20251223143206-950739ccd44a h1:XRsyqrSljes4TlaPczQViIAA4xqdnB0fKEEpZdqWWTA= -github.com/jesseduffield/gocui v0.3.1-0.20251223143206-950739ccd44a/go.mod h1:sLIyZ2J42R6idGdtemZzsiR3xY5EF0KsvYEGh3dQv3s= +github.com/jesseduffield/gocui v0.3.1-0.20251223144240-29fe12e8d53f h1:5ArylWehV98WTxJM7AcSa53YNskEFpHHv+VePONQn58= +github.com/jesseduffield/gocui v0.3.1-0.20251223144240-29fe12e8d53f/go.mod h1:sLIyZ2J42R6idGdtemZzsiR3xY5EF0KsvYEGh3dQv3s= github.com/jesseduffield/lazycore v0.0.0-20221012050358-03d2e40243c5 h1:CDuQmfOjAtb1Gms6a1p5L2P8RhbLUq5t8aL7PiQd2uY= github.com/jesseduffield/lazycore v0.0.0-20221012050358-03d2e40243c5/go.mod h1:qxN4mHOAyeIDLP7IK7defgPClM/z1Kze8VVQiaEjzsQ= github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= diff --git a/pkg/gui/context/list_context_trait.go b/pkg/gui/context/list_context_trait.go index 2ea58360bfd..374c570c325 100644 --- a/pkg/gui/context/list_context_trait.go +++ b/pkg/gui/context/list_context_trait.go @@ -102,10 +102,9 @@ func (self *ListContextTrait) HandleRender() { if self.getNonModelItems != nil { totalLength += len(self.getNonModelItems()) } - self.GetViewTrait().SetContentLineCount(totalLength) startIdx, length := self.GetViewTrait().ViewPortYBounds() content := self.renderLines(startIdx, startIdx+length) - self.GetViewTrait().SetViewPortContentAndClearEverythingElse(content) + self.GetViewTrait().SetViewPortContentAndClearEverythingElse(totalLength, content) } else { content := self.renderLines(-1, -1) self.GetViewTrait().SetContent(content) diff --git a/pkg/gui/context/view_trait.go b/pkg/gui/context/view_trait.go index 9a30bde6f77..d3825b9cf3d 100644 --- a/pkg/gui/context/view_trait.go +++ b/pkg/gui/context/view_trait.go @@ -34,13 +34,9 @@ func (self *ViewTrait) SetViewPortContent(content string) { self.view.OverwriteLines(y, content) } -func (self *ViewTrait) SetViewPortContentAndClearEverythingElse(content string) { +func (self *ViewTrait) SetViewPortContentAndClearEverythingElse(lineCount int, content string) { _, y := self.view.Origin() - self.view.OverwriteLinesAndClearEverythingElse(y, content) -} - -func (self *ViewTrait) SetContentLineCount(lineCount int) { - self.view.SetContentLineCount(lineCount) + self.view.OverwriteLinesAndClearEverythingElse(lineCount, y, content) } func (self *ViewTrait) SetContent(content string) { diff --git a/pkg/gui/types/context.go b/pkg/gui/types/context.go index 339cf2b492a..91d4f314b80 100644 --- a/pkg/gui/types/context.go +++ b/pkg/gui/types/context.go @@ -205,8 +205,7 @@ type IViewTrait interface { SetRangeSelectStart(yIdx int) CancelRangeSelect() SetViewPortContent(content string) - SetViewPortContentAndClearEverythingElse(content string) - SetContentLineCount(lineCount int) + SetViewPortContentAndClearEverythingElse(lineCount int, content string) SetContent(content string) SetFooter(value string) SetOriginX(value int) diff --git a/vendor/github.com/jesseduffield/gocui/view.go b/vendor/github.com/jesseduffield/gocui/view.go index 87f61eebd31..0d0b19ef49c 100644 --- a/vendor/github.com/jesseduffield/gocui/view.go +++ b/vendor/github.com/jesseduffield/gocui/view.go @@ -1728,10 +1728,12 @@ func (v *View) OverwriteLines(y int, content string) { } // only call this function if you don't care where v.wx and v.wy end up -func (v *View) OverwriteLinesAndClearEverythingElse(y int, content string) { +func (v *View) OverwriteLinesAndClearEverythingElse(lineCount int, y int, content string) { v.writeMutex.Lock() defer v.writeMutex.Unlock() + v.setContentLineCount(lineCount) + v.overwriteLines(y, content) for i := 0; i < y; i += 1 { @@ -1743,7 +1745,7 @@ func (v *View) OverwriteLinesAndClearEverythingElse(y int, content string) { } } -func (v *View) SetContentLineCount(lineCount int) { +func (v *View) setContentLineCount(lineCount int) { if lineCount > 0 { v.makeWriteable(0, lineCount-1) } diff --git a/vendor/modules.txt b/vendor/modules.txt index 49ff6fe7ae0..dd946f71d3d 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -221,7 +221,7 @@ github.com/jesseduffield/go-git/v5/utils/merkletrie/internal/frame github.com/jesseduffield/go-git/v5/utils/merkletrie/noder github.com/jesseduffield/go-git/v5/utils/sync github.com/jesseduffield/go-git/v5/utils/trace -# github.com/jesseduffield/gocui v0.3.1-0.20251223143206-950739ccd44a +# github.com/jesseduffield/gocui v0.3.1-0.20251223144240-29fe12e8d53f ## explicit; go 1.12 github.com/jesseduffield/gocui # github.com/jesseduffield/lazycore v0.0.0-20221012050358-03d2e40243c5 From f7d4efc59e69576ca29d397f910cc9d96d8a63cc Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 22 Dec 2025 17:04:43 +0100 Subject: [PATCH 2/3] Rerender visible lines when scrolling by page This fixes a bug in ListContextTrait.FocusLine whereby the view would go blank when scrolling by page (using ',' or '.') in views that have renderOnlyVisibleLines set to true but refreshViewportOnChange set to false. Currently we don't have any such views; the only ones who use renderOnlyVisibleLines are commits and subcommits, and they also use refreshViewportOnChange. However, we are going to add one in the next commit, and eventually it might be a good idea to convert all our list views to that by default, and get rid of the renderOnlyVisibleLines flag. --- pkg/gui/context/list_context_trait.go | 12 ++++++++++-- pkg/gui/controllers/list_controller.go | 7 +++++++ pkg/gui/types/context.go | 1 + 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/pkg/gui/context/list_context_trait.go b/pkg/gui/context/list_context_trait.go index 374c570c325..f04c24ffeca 100644 --- a/pkg/gui/context/list_context_trait.go +++ b/pkg/gui/context/list_context_trait.go @@ -21,6 +21,9 @@ type ListContextTrait struct { // If this is true, we only render the visible lines of the list. Useful for lists that can // get very long, because it can save a lot of memory renderOnlyVisibleLines bool + // If renderOnlyVisibleLines is true, needRerenderVisibleLines indicates whether we need to + // rerender the visible lines e.g. because the scroll position changed + needRerenderVisibleLines bool } func (self *ListContextTrait) IsListContext() {} @@ -50,8 +53,8 @@ func (self *ListContextTrait) FocusLine(scrollIntoView bool) { self.refreshViewport() } else if self.renderOnlyVisibleLines { newOrigin, _ := self.GetViewTrait().ViewPortYBounds() - if oldOrigin != newOrigin { - self.HandleRender() + if oldOrigin != newOrigin || self.needRerenderVisibleLines { + self.refreshViewport() } } return nil @@ -105,6 +108,7 @@ func (self *ListContextTrait) HandleRender() { startIdx, length := self.GetViewTrait().ViewPortYBounds() content := self.renderLines(startIdx, startIdx+length) self.GetViewTrait().SetViewPortContentAndClearEverythingElse(totalLength, content) + self.needRerenderVisibleLines = false } else { content := self.renderLines(-1, -1) self.GetViewTrait().SetContent(content) @@ -141,6 +145,10 @@ func (self *ListContextTrait) RenderOnlyVisibleLines() bool { return self.renderOnlyVisibleLines } +func (self *ListContextTrait) SetNeedRerenderVisibleLines() { + self.needRerenderVisibleLines = true +} + func (self *ListContextTrait) TotalContentHeight() int { result := self.list.Len() if self.getNonModelItems != nil { diff --git a/pkg/gui/controllers/list_controller.go b/pkg/gui/controllers/list_controller.go index a9107439269..cebc0f62e6f 100644 --- a/pkg/gui/controllers/list_controller.go +++ b/pkg/gui/controllers/list_controller.go @@ -178,6 +178,13 @@ func (self *ListController) handlePageChange(delta int) error { } } + // Since we already scrolled the view above, the normal mechanism that + // ListContextTrait.FocusLine uses for deciding whether rerendering is needed won't work. It is + // based on checking whether the origin was changed by the call to FocusPoint in that function, + // but since we scrolled the view directly above, the origin has already been updated. So we + // must tell it explicitly to rerender. + self.context.SetNeedRerenderVisibleLines() + // Since we are maintaining the scroll position ourselves above, there's no point in passing // ScrollSelectionIntoView=true here. self.context.HandleFocus(types.OnFocusOpts{}) diff --git a/pkg/gui/types/context.go b/pkg/gui/types/context.go index 91d4f314b80..790f4c84bd1 100644 --- a/pkg/gui/types/context.go +++ b/pkg/gui/types/context.go @@ -182,6 +182,7 @@ type IListContext interface { IsListContext() // used for type switch RangeSelectEnabled() bool RenderOnlyVisibleLines() bool + SetNeedRerenderVisibleLines() IndexForGotoBottom() int } From 84be082fb5556abeaceb140bf8df4931d56d99e5 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 22 Dec 2025 14:09:18 +0100 Subject: [PATCH 3/3] Draw only visible part of the reflogs panel Since the reflog can get very long, this saves some memory but especially some UI thread lag. In one of my repos I had over 11'000 reflog entries (I guess I should prune them more regularly...), and rendering them took ~600ms; since this happens on the UI thread, there was an annoying stall for half a second after every background fetch, for example. --- pkg/gui/context/reflog_commits_context.go | 25 +++++++++++++++-------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/pkg/gui/context/reflog_commits_context.go b/pkg/gui/context/reflog_commits_context.go index 1a882bf120a..6358fbbb003 100644 --- a/pkg/gui/context/reflog_commits_context.go +++ b/pkg/gui/context/reflog_commits_context.go @@ -26,9 +26,14 @@ func NewReflogCommitsContext(c *ContextCommon) *ReflogCommitsContext { }, ) - getDisplayStrings := func(_ int, _ int) [][]string { + getDisplayStrings := func(startIdx int, endIdx int) [][]string { + commits := viewModel.GetItems() + if startIdx >= len(commits) { + return nil + } + return presentation.GetReflogCommitListDisplayStrings( - viewModel.GetItems(), + commits[startIdx:endIdx], c.State().GetRepoState().GetScreenMode() != types.SCREEN_NORMAL, c.Modes().CherryPicking.SelectedHashSet(), c.Modes().Diffing.Ref, @@ -43,18 +48,20 @@ func NewReflogCommitsContext(c *ContextCommon) *ReflogCommitsContext { FilteredListViewModel: viewModel, ListContextTrait: &ListContextTrait{ Context: NewSimpleContext(NewBaseContext(NewBaseContextOpts{ - View: c.Views().ReflogCommits, - WindowName: "commits", - Key: REFLOG_COMMITS_CONTEXT_KEY, - Kind: types.SIDE_CONTEXT, - Focusable: true, - NeedsRerenderOnWidthChange: types.NEEDS_RERENDER_ON_WIDTH_CHANGE_WHEN_SCREEN_MODE_CHANGES, + View: c.Views().ReflogCommits, + WindowName: "commits", + Key: REFLOG_COMMITS_CONTEXT_KEY, + Kind: types.SIDE_CONTEXT, + Focusable: true, + NeedsRerenderOnWidthChange: types.NEEDS_RERENDER_ON_WIDTH_CHANGE_WHEN_SCREEN_MODE_CHANGES, + NeedsRerenderOnHeightChange: true, })), ListRenderer: ListRenderer{ list: viewModel, getDisplayStrings: getDisplayStrings, }, - c: c, + c: c, + renderOnlyVisibleLines: true, }, } }