From bd6081d601389a4814b5c964b94a950b53b8b266 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 4 Jul 2026 21:18:38 +0200 Subject: [PATCH] Select the checked-out branch via a refresh intent, not off-thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Operations that check something out (checkout, create branch, move commits to a new branch, fetch-and-checkout) selected the newly checked-out branch by calling SelectFirstBranchAndFirstCommit() before the refresh and passing KeepBranchSelectionIndex so the refresh wouldn't override it. That set the selection directly, usually from a worker goroutine (WithWaitingStatus/WithInlineStatus). Now that the refresh's own selection write is bounced onto the UI thread, the two writes could land in either order, and under load the refresh's "restore the previously-selected branch" write would win — leaving the old branch selected instead of the new one (flaky move_commits_to_new_branch_from_base_branch). Replace it with declarative selection intents applied inside the refresh's own bounce, so the selection is set on the UI thread and atomically with the list write (no off-thread write, and no BLOCK_UI needed to avoid a flicker): - BranchSelection: SelectCheckedOutBranch selects the checked-out branch (top of the list). The default, KeepBranchSelectionByName, restores the previously-selected branch by name as before. This replaces the KeepBranchSelectionIndex bool. - CommitSelection: SelectHeadCommit (already existed) for the commit. - SelectTopReflogCommit selects the top reflog entry, since a checkout adds a new entry there (reflog/checkout relies on this). SelectFirstBranchAndFirstCommit is gone. The previously-selected branch is now read at the top of the branches bounce, before the list is overwritten, so that read moves onto the UI thread too. fetchAndCheckout's refresh changes from ASYNC to SYNC so its post-refresh focus switch can run in Then on the UI thread; SYNC keeps the inline fetch spinner spinning (only BLOCK_UI would freeze it). Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/gui/controllers/branches_controller.go | 8 +-- pkg/gui/controllers/helpers/refresh_helper.go | 59 +++++++++++++------ pkg/gui/controllers/helpers/refs_helper.go | 47 ++++++--------- pkg/gui/controllers/remotes_controller.go | 17 ++++-- pkg/gui/types/refresh.go | 31 ++++++++-- 5 files changed, 99 insertions(+), 63 deletions(-) diff --git a/pkg/gui/controllers/branches_controller.go b/pkg/gui/controllers/branches_controller.go index 131d19439..a5c55884b 100644 --- a/pkg/gui/controllers/branches_controller.go +++ b/pkg/gui/controllers/branches_controller.go @@ -599,11 +599,11 @@ func (self *BranchesController) createNewBranchWithName(newBranchName string) er return err } - self.c.Helpers().Refs.SelectFirstBranchAndFirstCommit() self.c.Refresh(types.RefreshOptions{ - Mode: types.ASYNC, - KeepBranchSelectionIndex: true, - CommitSelection: types.KeepCommitSelectionIndex, + Mode: types.ASYNC, + BranchSelection: types.SelectCheckedOutBranch, + CommitSelection: types.SelectHeadCommit, + SelectTopReflogCommit: true, }) return nil } diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index c0543d095..cb1856aa9 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -183,7 +183,7 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) { if self.c.UserConfig().Git.LocalBranchSortOrder == "recency" { branchesAndRemotesWg.Add(1) refresh("reflog and branches", func() { - self.refreshReflogAndBranches(includeWorktreesWithBranches, options.KeepBranchSelectionIndex, options.Background) + self.refreshReflogAndBranches(includeWorktreesWithBranches, options.BranchSelection, options.SelectTopReflogCommit, options.Background) branchesAndRemotesWg.Done() }) } else { @@ -192,10 +192,10 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) { // Not a recency sort, so branches doesn't depend on the reflog // being fresh; it runs concurrently with the reflog refresh // below and reads whatever's in the model, as it always has. - self.refreshBranches(includeWorktreesWithBranches, options.KeepBranchSelectionIndex, true, self.c.Model().ReflogCommits, options.Background) + self.refreshBranches(includeWorktreesWithBranches, options.BranchSelection, true, self.c.Model().ReflogCommits, options.Background) branchesAndRemotesWg.Done() }) - refresh("reflog", func() { _, _ = self.refreshReflogCommits(options.Background) }) + refresh("reflog", func() { _, _ = self.refreshReflogCommits(options.Background, options.SelectTopReflogCommit) }) } } else if scopeSet.Includes(types.REBASE_COMMITS) { // the above block handles rebase commits so we only need to call this one @@ -399,21 +399,21 @@ func getModeName(mode types.RefreshMode) string { // order gives the immediate (non-recency) load a lower branch-load sequence // than the async (recency) load, so the sequence guard in refreshBranches keeps // the recency-sorted result even if the two loads' bounces land out of order. -func (self *RefreshHelper) refreshReflogAndBranches(refreshWorktrees bool, keepBranchSelectionIndex bool, background bool) { +func (self *RefreshHelper) refreshReflogAndBranches(refreshWorktrees bool, branchSelection types.BranchSelectionBehavior, selectTopReflogCommit bool, background bool) { switch self.c.State().GetRepoState().GetStartupStage() { case types.INITIAL: - self.refreshBranches(refreshWorktrees, keepBranchSelectionIndex, false, self.c.Model().ReflogCommits, background) + self.refreshBranches(refreshWorktrees, branchSelection, false, self.c.Model().ReflogCommits, background) self.onWorker(background, func(_ gocui.Task) error { - reflogCommits, _ := self.refreshReflogCommits(background) - self.refreshBranches(false, true, true, reflogCommits, background) + reflogCommits, _ := self.refreshReflogCommits(background, false) + self.refreshBranches(false, types.SelectCheckedOutBranch, true, reflogCommits, background) self.c.State().GetRepoState().SetStartupStage(types.COMPLETE) return nil }) case types.COMPLETE: - reflogCommits, _ := self.refreshReflogCommits(background) - self.refreshBranches(refreshWorktrees, keepBranchSelectionIndex, true, reflogCommits, background) + reflogCommits, _ := self.refreshReflogCommits(background, selectTopReflogCommit) + self.refreshBranches(refreshWorktrees, branchSelection, true, reflogCommits, background) } } @@ -728,7 +728,7 @@ func (self *RefreshHelper) refreshStateSubmoduleConfigs() ([]*models.SubmoduleCo // self.refreshStatus is called at the end of this because that's when we can // be sure there is a State.Model.Branches array to pick the current branch from -func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSelectionIndex bool, loadBehindCounts bool, reflogCommits []*models.Commit, background bool) { +func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, branchSelection types.BranchSelectionBehavior, loadBehindCounts bool, reflogCommits []*models.Commit, background bool) { loadSeq := self.branchLoadSeq.Add(1) generation := self.c.State().GetRepoGeneration() @@ -754,8 +754,6 @@ func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSele self.c.Log.Error(err) } - prevSelectedBranch := self.c.Contexts().Branches.GetSelected() - var worktrees []*models.Worktree if refreshWorktrees { worktrees = self.loadWorktrees() @@ -772,6 +770,11 @@ func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSele } self.appliedBranchLoadSeq = loadSeq + // Read the currently-selected branch before overwriting the list, so we + // can restore it by name below. Reading it here in the bounce keeps it on + // the UI thread. + prevSelectedBranch := self.c.Contexts().Branches.GetSelected() + self.c.Model().Branches = branches // Rebuilding here (rather than on the worker) means the map is built from // the branches we just wrote, on the UI thread. @@ -782,14 +785,25 @@ func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSele self.refreshView(self.c.Contexts().Worktrees, background) } - if !keepBranchSelectionIndex && prevSelectedBranch != nil { - self.searchHelper.ReApplyFilter(self.c.Contexts().Branches) + // Setting the selection here, in the same bounce that writes the list, + // keeps it on the UI thread and keeps the list and selection updating in + // the same frame. + switch branchSelection { + case types.KeepBranchSelectionByName: + if prevSelectedBranch != nil { + self.searchHelper.ReApplyFilter(self.c.Contexts().Branches) - _, idx, found := lo.FindIndexOf(self.c.Contexts().Branches.GetItems(), - func(b *models.Branch) bool { return b.Name == prevSelectedBranch.Name }) - if found { - self.c.Contexts().Branches.SetSelectedLineIdx(idx) + _, idx, found := lo.FindIndexOf(self.c.Contexts().Branches.GetItems(), + func(b *models.Branch) bool { return b.Name == prevSelectedBranch.Name }) + if found { + self.c.Contexts().Branches.SetSelectedLineIdx(idx) + } } + case types.SelectCheckedOutBranch: + // The checked-out branch is always at the top of the list. Setting + // the selection doesn't scroll the view, so also reset the origin. + self.c.Contexts().Branches.SetSelectedLineIdx(0) + self.c.Contexts().Branches.GetView().SetOriginY(0) } // Need to re-render the commits view because the visualization of local @@ -965,7 +979,7 @@ func (self *RefreshHelper) refreshStateFiles(background bool, submoduleConfigs [ // refreshReflogCommits returns the (non-filtered) ReflogCommits it loaded, so // that a subsequent branches refresh can use them for recency sorting without // having to read them back out of the model. -func (self *RefreshHelper) refreshReflogCommits(background bool) ([]*models.Commit, error) { +func (self *RefreshHelper) refreshReflogCommits(background bool, selectTopEntry bool) ([]*models.Commit, error) { generation := self.c.State().GetRepoGeneration() // pulling state into its own variable in case it gets swapped out for another state // and we get an out of bounds exception @@ -1008,6 +1022,13 @@ func (self *RefreshHelper) refreshReflogCommits(background bool) ([]*models.Comm self.onUIThreadUnlessRepoChanged(generation, background, func() error { model.ReflogCommits = reflogCommits model.FilteredReflogCommits = filteredReflogCommits + // Setting the selection here, in the same bounce that writes the list, + // keeps it on the UI thread and atomic with the list update. Setting the + // selection doesn't scroll the view, so also reset the origin. + if selectTopEntry { + self.c.Contexts().ReflogCommits.SetSelectedLineIdx(0) + self.c.Contexts().ReflogCommits.GetView().SetOriginY(0) + } return nil }) diff --git a/pkg/gui/controllers/helpers/refs_helper.go b/pkg/gui/controllers/helpers/refs_helper.go index b90b9150b..8d5e8397c 100644 --- a/pkg/gui/controllers/helpers/refs_helper.go +++ b/pkg/gui/controllers/helpers/refs_helper.go @@ -31,15 +31,6 @@ func NewRefsHelper( } } -func (self *RefsHelper) SelectFirstBranchAndFirstCommit() { - self.c.Contexts().Branches.SetSelection(0) - self.c.Contexts().ReflogCommits.SetSelection(0) - self.c.Contexts().LocalCommits.SetSelection(0) - self.c.Contexts().Branches.GetView().SetOriginY(0) - self.c.Contexts().ReflogCommits.GetView().SetOriginY(0) - self.c.Contexts().LocalCommits.GetView().SetOriginY(0) -} - func (self *RefsHelper) CheckoutRef(ref string, options types.CheckoutRefOptions) error { waitingStatus := options.WaitingStatus if waitingStatus == "" { @@ -49,8 +40,6 @@ func (self *RefsHelper) CheckoutRef(ref string, options types.CheckoutRefOptions cmdOptions := git_commands.CheckoutOptions{Force: false, EnvVars: options.EnvVars} refresh := func() { - self.SelectFirstBranchAndFirstCommit() - // loading a heap of commits is slow so we limit them whenever doing a reset self.c.Contexts().LocalCommits.SetLimitCommits(true) @@ -67,10 +56,11 @@ func (self *RefsHelper) CheckoutRef(ref string, options types.CheckoutRefOptions scope = append(scope, types.PULL_REQUESTS) } self.c.Refresh(types.RefreshOptions{ - Mode: types.BLOCK_UI, - Scope: scope, - KeepBranchSelectionIndex: true, - CommitSelection: types.KeepCommitSelectionIndex, + Mode: types.BLOCK_UI, + Scope: scope, + BranchSelection: types.SelectCheckedOutBranch, + CommitSelection: types.SelectHeadCommit, + SelectTopReflogCommit: true, }) } @@ -375,12 +365,11 @@ func (self *RefsHelper) NewBranch(from string, fromFormattedName string, suggest self.c.Context().Push(self.c.Contexts().Branches, types.OnFocusOpts{}) } - self.SelectFirstBranchAndFirstCommit() - self.c.Refresh(types.RefreshOptions{ - Mode: types.BLOCK_UI, - KeepBranchSelectionIndex: true, - CommitSelection: types.KeepCommitSelectionIndex, + Mode: types.BLOCK_UI, + BranchSelection: types.SelectCheckedOutBranch, + CommitSelection: types.SelectHeadCommit, + SelectTopReflogCommit: true, }) } @@ -534,12 +523,11 @@ func (self *RefsHelper) moveCommitsToNewBranchStackedOnCurrentBranch(newBranchNa } } - self.SelectFirstBranchAndFirstCommit() - self.c.Refresh(types.RefreshOptions{ - Mode: types.BLOCK_UI, - KeepBranchSelectionIndex: true, - CommitSelection: types.KeepCommitSelectionIndex, + Mode: types.BLOCK_UI, + BranchSelection: types.SelectCheckedOutBranch, + CommitSelection: types.SelectHeadCommit, + SelectTopReflogCommit: true, }) return nil } @@ -576,12 +564,11 @@ func (self *RefsHelper) moveCommitsToNewBranchOffOfMainBranch(newBranchName stri } } - self.SelectFirstBranchAndFirstCommit() - self.c.Refresh(types.RefreshOptions{ - Mode: types.BLOCK_UI, - KeepBranchSelectionIndex: true, - CommitSelection: types.KeepCommitSelectionIndex, + Mode: types.BLOCK_UI, + BranchSelection: types.SelectCheckedOutBranch, + CommitSelection: types.SelectHeadCommit, + SelectTopReflogCommit: true, }) return nil } diff --git a/pkg/gui/controllers/remotes_controller.go b/pkg/gui/controllers/remotes_controller.go index e4f606ca3..8bd19ad81 100644 --- a/pkg/gui/controllers/remotes_controller.go +++ b/pkg/gui/controllers/remotes_controller.go @@ -376,10 +376,19 @@ func (self *RemotesController) fetchAndCheckout(remote *models.Remote, branchNam if branchName != "" { err = self.c.Git().Branch.New(branchName, remote.Name+"/"+branchName) if err == nil { - self.c.Context().Push(self.c.Contexts().Branches, types.OnFocusOpts{}) - self.c.Helpers().Refs.SelectFirstBranchAndFirstCommit() - refreshOptions.KeepBranchSelectionIndex = true - refreshOptions.CommitSelection = types.KeepCommitSelectionIndex + // Branch.New checks the new branch out, so HEAD moves: refresh the + // reflog (and, via scope expansion, the commits) as well, and select + // the newly checked-out branch and its head commit. + refreshOptions.Scope = append(refreshOptions.Scope, types.REFLOG) + refreshOptions.BranchSelection = types.SelectCheckedOutBranch + refreshOptions.CommitSelection = types.SelectHeadCommit + refreshOptions.SelectTopReflogCommit = true + // Focus the branches panel on the UI thread once the refresh has + // selected the newly checked-out branch. + refreshOptions.Then = func() error { + self.c.Context().Push(self.c.Contexts().Branches, types.OnFocusOpts{}) + return nil + } } } self.c.Refresh(refreshOptions) diff --git a/pkg/gui/types/refresh.go b/pkg/gui/types/refresh.go index 591aff5f3..f4041bb2e 100644 --- a/pkg/gui/types/refresh.go +++ b/pkg/gui/types/refresh.go @@ -55,22 +55,41 @@ const ( SelectHeadCommit ) +// BranchSelectionBehavior controls which local branch is selected after the +// branches list is reloaded by a refresh. +type BranchSelectionBehavior int + +const ( + // Keep the same branch selected by name, restoring it at its new position if + // the order changed. This is the right default whenever the list reloads + // underneath a selection the user hasn't deliberately changed. + KeepBranchSelectionByName BranchSelectionBehavior = iota + + // Select the checked-out branch (the one at the top of the list). Used after + // operations that check something out - checkout, creating a branch, moving + // commits to a new branch - so the newly checked-out ref ends up selected. + SelectCheckedOutBranch +) + type RefreshOptions struct { 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 - // Normally a refresh of the branches tries to keep the same branch selected - // (by name); this is usually important in case the order of branches - // changes. Passing true for KeepBranchSelectionIndex suppresses this and - // keeps the selection index the same. Useful after checking out a detached - // head, and selecting index 0. - KeepBranchSelectionIndex bool + // Controls which local branch is selected after the refresh. Defaults to + // KeepBranchSelectionByName. + BranchSelection BranchSelectionBehavior // Controls which local commit is selected after the refresh. Defaults to // KeepCommitSelectionByHash. CommitSelection CommitSelectionBehavior + // When true, select the top (most recent) reflog entry after the refresh. + // Used alongside SelectCheckedOutBranch by operations that check something + // out, since the checkout adds a new reflog entry at the top. Defaults to + // keeping the reflog selection where it is. + SelectTopReflogCommit bool + // When true, this refresh was initiated by a background routine rather than // by a user action. Every git command suppresses optional locks by default // so it can't contend for index.lock (see git_commands.OptionalLocksEnvVar);