From 717448f105589033f562e17ac195b1a9e4725110 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 2 Jul 2026 22:33:33 +0200 Subject: [PATCH] Make RefreshOptions.Then a func() error, queue it via OnUIThread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is preparation for upcoming commits that will bounce refresh-scope model updates (e.g. Model.Files) onto the UI thread by enqueuing the write via OnUIThread instead of applying it directly on the worker goroutine. Once that lands, a Then callback that reads the model must run after that queued write has been processed, not synchronously at wg.Wait() time — at that point the workers have returned, but a bounce they queued may not have been processed yet. Queuing Then via OnUIThread here, ahead of that change, guarantees the right ordering once it lands: a bounce queued earlier in the same refresh is already sitting in the channel by the time wg.Wait() returns, so Then enqueued after it will always be processed after, and see the post-refresh model. The signature change to func() error lets Then propagate errors through gocui's normal error handler (the same path key-handler errors take). Co-Authored-By: Claude Sonnet 5 --- pkg/gui/controllers/bisect_controller.go | 7 +++++-- pkg/gui/controllers/filtering_menu_action.go | 3 ++- pkg/gui/controllers/helpers/mode_helper.go | 3 ++- pkg/gui/controllers/helpers/refresh_helper.go | 8 +++++++- pkg/gui/controllers/local_commits_controller.go | 8 +++----- pkg/gui/types/refresh.go | 2 +- 6 files changed, 20 insertions(+), 11 deletions(-) diff --git a/pkg/gui/controllers/bisect_controller.go b/pkg/gui/controllers/bisect_controller.go index 1066237c1..eb568240b 100644 --- a/pkg/gui/controllers/bisect_controller.go +++ b/pkg/gui/controllers/bisect_controller.go @@ -274,10 +274,11 @@ func (self *BisectController) afterMark(selectCurrent bool, waitToReselect bool) } func (self *BisectController) afterBisectMarkRefresh(selectCurrent bool, waitToReselect bool) error { - selectFn := func() { + selectFn := func() error { if selectCurrent { self.selectCurrentBisectCommit() } + return nil } if waitToReselect { @@ -285,7 +286,9 @@ func (self *BisectController) afterBisectMarkRefresh(selectCurrent bool, waitToR return nil } - selectFn() + if err := selectFn(); err != nil { + return err + } self.c.Helpers().Bisect.PostBisectCommandRefresh() return nil diff --git a/pkg/gui/controllers/filtering_menu_action.go b/pkg/gui/controllers/filtering_menu_action.go index 01a236f7a..7ae26c4ef 100644 --- a/pkg/gui/controllers/filtering_menu_action.go +++ b/pkg/gui/controllers/filtering_menu_action.go @@ -122,9 +122,10 @@ func (self *FilteringMenuAction) setFiltering() error { self.c.Context().Push(self.c.Contexts().LocalCommits, types.OnFocusOpts{}) - self.c.Refresh(types.RefreshOptions{Scope: helpers.ScopesToRefreshWhenFilteringModeChanges(), Then: func() { + self.c.Refresh(types.RefreshOptions{Scope: helpers.ScopesToRefreshWhenFilteringModeChanges(), Then: func() error { self.c.Contexts().LocalCommits.SetSelection(0) self.c.Contexts().LocalCommits.HandleFocus(types.OnFocusOpts{}) + return nil }}) return nil diff --git a/pkg/gui/controllers/helpers/mode_helper.go b/pkg/gui/controllers/helpers/mode_helper.go index e44d7b01b..4947e42d1 100644 --- a/pkg/gui/controllers/helpers/mode_helper.go +++ b/pkg/gui/controllers/helpers/mode_helper.go @@ -191,7 +191,7 @@ func (self *ModeHelper) ClearFiltering() error { self.c.Refresh(types.RefreshOptions{ Scope: ScopesToRefreshWhenFilteringModeChanges(), - Then: func() { + Then: func() error { // Find the commit that was last selected in filtering mode, and select it again after refreshing if !self.c.Contexts().LocalCommits.SelectCommitByHash(selectedCommitHash) { // If we couldn't find it (either because no commit was selected @@ -202,6 +202,7 @@ func (self *ModeHelper) ClearFiltering() error { } self.c.PostRefreshUpdate(self.c.Contexts().LocalCommits) + return nil }, }) return nil diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 866ec90c3..2933d9378 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -255,7 +255,13 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) { wg.Wait() if options.Then != nil { - options.Then() + // Queue Then via OnUIThread so it runs *after* the refresh-scope + // functions' model-update bounces (which are already queued by + // now), not synchronously here — at this point the workers have + // returned but their bounces haven't been processed yet, so + // invoking Then synchronously would run it on a model that's + // still pre-refresh. + self.c.OnUIThread(options.Then) } } diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 9f3acb1d2..5150e8fba 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -616,7 +616,7 @@ func (self *LocalCommitsController) startInteractiveRebaseWithEdit( err := self.c.Git().Rebase.EditRebase(commitsToEdit[len(commitsToEdit)-1].Hash()) return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( err, - types.RefreshOptions{Mode: types.BLOCK_UI, Then: func() { + types.RefreshOptions{Mode: types.BLOCK_UI, Then: func() error { todos := make([]*models.Commit, 0, len(commitsToEdit)-1) for _, c := range commitsToEdit[:len(commitsToEdit)-1] { // Merge commits can't be set to "edit", so just skip them @@ -625,11 +625,9 @@ func (self *LocalCommitsController) startInteractiveRebaseWithEdit( } } if len(todos) > 0 { - err := self.updateTodos(todo.Edit, todos) - if err != nil { - self.c.Log.Errorf("error when updating todos: %v", err) - } + return self.updateTodos(todo.Edit, todos) } + return nil }}) }) } diff --git a/pkg/gui/types/refresh.go b/pkg/gui/types/refresh.go index 8d9704d55..591aff5f3 100644 --- a/pkg/gui/types/refresh.go +++ b/pkg/gui/types/refresh.go @@ -56,7 +56,7 @@ const ( ) type RefreshOptions struct { - Then func() + Then func() error Scope []RefreshableView // e.g. []RefreshableView{COMMITS, BRANCHES}. Leave empty to refresh everything Mode RefreshMode // one of SYNC (default), ASYNC, and BLOCK_UI