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);