From 2c9b4770bcf2fbea44737a40adf872d0bbb89741 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 29 Dec 2023 14:27:48 +0100 Subject: [PATCH] Keep same branch selected when refreshing branches This wasn't necessary before, because the only available branch sorting option was by recency, so the sort order couldn't change except by checking out branches. Now, you can sort by committer date, so the branch order can change by fetching; in this case it's important to keep the same branch selected. One important use case is to rebase the checked-out branch onto master; you select master, press "f" to fetch it (this can now change its position in the list), and then press "r" to rebase. To make this work smoothly it's important to keep master selected after pressing "f". --- pkg/gui/controllers/branches_controller.go | 2 +- pkg/gui/controllers/helpers/refresh_helper.go | 20 ++++++++++++++----- pkg/gui/controllers/helpers/refs_helper.go | 10 ++++++---- pkg/gui/types/refresh.go | 7 +++++++ .../custom_commands/suggestions_command.go | 4 ++-- .../custom_commands/suggestions_preset.go | 4 ++-- .../tests/sync/fetch_when_sorted_by_date.go | 5 ----- 7 files changed, 33 insertions(+), 19 deletions(-) diff --git a/pkg/gui/controllers/branches_controller.go b/pkg/gui/controllers/branches_controller.go index 37a637202..8cac9537d 100644 --- a/pkg/gui/controllers/branches_controller.go +++ b/pkg/gui/controllers/branches_controller.go @@ -447,7 +447,7 @@ func (self *BranchesController) createNewBranchWithName(newBranchName string) er } self.context().SetSelection(0) - return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, KeepBranchSelectionIndex: true}) } func (self *BranchesController) checkedOutByOtherWorktree(branch *models.Branch) bool { diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 15ecd8d4e..a8f32d116 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -126,7 +126,7 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) error { refresh("commits and commit files", self.refreshCommitsAndCommitFiles) includeWorktreesWithBranches = scopeSet.Includes(types.WORKTREES) - refresh("reflog and branches", func() { self.refreshReflogAndBranches(includeWorktreesWithBranches) }) + refresh("reflog and branches", func() { self.refreshReflogAndBranches(includeWorktreesWithBranches, options.KeepBranchSelectionIndex) }) } else if scopeSet.Includes(types.REBASE_COMMITS) { // the above block handles rebase commits so we only need to call this one // if we've asked specifically for rebase commits and not those other things @@ -248,7 +248,7 @@ func (self *RefreshHelper) refreshReflogCommitsConsideringStartup() { case types.INITIAL: self.c.OnWorker(func(_ gocui.Task) { _ = self.refreshReflogCommits() - self.refreshBranches(false) + self.refreshBranches(false, true) self.c.State().GetRepoState().SetStartupStage(types.COMPLETE) }) @@ -257,10 +257,10 @@ func (self *RefreshHelper) refreshReflogCommitsConsideringStartup() { } } -func (self *RefreshHelper) refreshReflogAndBranches(refreshWorktrees bool) { +func (self *RefreshHelper) refreshReflogAndBranches(refreshWorktrees bool, keepBranchSelectionIndex bool) { self.refreshReflogCommitsConsideringStartup() - self.refreshBranches(refreshWorktrees) + self.refreshBranches(refreshWorktrees, keepBranchSelectionIndex) } func (self *RefreshHelper) refreshCommitsAndCommitFiles() { @@ -425,10 +425,12 @@ func (self *RefreshHelper) refreshStateSubmoduleConfigs() error { // 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) { +func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSelectionIndex bool) { self.c.Mutexes().RefreshingBranchesMutex.Lock() defer self.c.Mutexes().RefreshingBranchesMutex.Unlock() + prevSelectedBranch := self.c.Contexts().Branches.GetSelected() + reflogCommits := self.c.Model().FilteredReflogCommits if self.c.Modes().Filtering.Active() && self.c.AppState.LocalBranchSortOrder == "recency" { // in filter mode we filter our reflog commits to just those containing the path @@ -456,6 +458,14 @@ func (self *RefreshHelper) refreshBranches(refreshWorktrees bool) { } } + if !keepBranchSelectionIndex && prevSelectedBranch != nil { + _, 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) + } + } + if err := self.refreshView(self.c.Contexts().Branches); err != nil { self.c.Log.Error(err) } diff --git a/pkg/gui/controllers/helpers/refs_helper.go b/pkg/gui/controllers/helpers/refs_helper.go index 4e4461526..095ffc103 100644 --- a/pkg/gui/controllers/helpers/refs_helper.go +++ b/pkg/gui/controllers/helpers/refs_helper.go @@ -51,6 +51,8 @@ func (self *RefsHelper) CheckoutRef(ref string, options types.CheckoutRefOptions self.c.Contexts().LocalCommits.SetLimitCommits(true) } + refreshOptions := types.RefreshOptions{Mode: types.BLOCK_UI, KeepBranchSelectionIndex: true} + return self.c.WithWaitingStatus(waitingStatus, func(gocui.Task) error { if err := self.c.Git().Branch.Checkout(ref, cmdOptions); err != nil { // note, this will only work for english-language git commands. If we force git to use english, and the error isn't this one, then the user will receive an english command they may not understand. I'm not sure what the best solution to this is. Running the command once in english and a second time in the native language is one option @@ -74,12 +76,12 @@ func (self *RefsHelper) CheckoutRef(ref string, options types.CheckoutRefOptions onSuccess() if err := self.c.Git().Stash.Pop(0); err != nil { - if err := self.c.Refresh(types.RefreshOptions{Mode: types.BLOCK_UI}); err != nil { + if err := self.c.Refresh(refreshOptions); err != nil { return err } return self.c.Error(err) } - return self.c.Refresh(types.RefreshOptions{Mode: types.BLOCK_UI}) + return self.c.Refresh(refreshOptions) }, }) } @@ -90,7 +92,7 @@ func (self *RefsHelper) CheckoutRef(ref string, options types.CheckoutRefOptions } onSuccess() - return self.c.Refresh(types.RefreshOptions{Mode: types.BLOCK_UI}) + return self.c.Refresh(refreshOptions) }) } @@ -218,7 +220,7 @@ func (self *RefsHelper) NewBranch(from string, fromFormattedName string, suggest self.c.Contexts().LocalCommits.SetSelection(0) self.c.Contexts().Branches.SetSelection(0) - return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + return self.c.Refresh(types.RefreshOptions{Mode: types.BLOCK_UI, KeepBranchSelectionIndex: true}) }, }) } diff --git a/pkg/gui/types/refresh.go b/pkg/gui/types/refresh.go index 552bfae04..c20a5f54a 100644 --- a/pkg/gui/types/refresh.go +++ b/pkg/gui/types/refresh.go @@ -36,4 +36,11 @@ type RefreshOptions struct { Then func() 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 } diff --git a/pkg/integration/tests/custom_commands/suggestions_command.go b/pkg/integration/tests/custom_commands/suggestions_command.go index f54ec1bae..592c472cf 100644 --- a/pkg/integration/tests/custom_commands/suggestions_command.go +++ b/pkg/integration/tests/custom_commands/suggestions_command.go @@ -57,8 +57,8 @@ var SuggestionsCommand = NewIntegrationTest(NewIntegrationTestArgs{ t.Views().Branches(). Lines( - Contains("branch-three").IsSelected(), - Contains("branch-four"), + Contains("branch-three"), + Contains("branch-four").IsSelected(), Contains("branch-two"), Contains("branch-one"), ) diff --git a/pkg/integration/tests/custom_commands/suggestions_preset.go b/pkg/integration/tests/custom_commands/suggestions_preset.go index 40c888625..d4ae422ad 100644 --- a/pkg/integration/tests/custom_commands/suggestions_preset.go +++ b/pkg/integration/tests/custom_commands/suggestions_preset.go @@ -57,8 +57,8 @@ var SuggestionsPreset = NewIntegrationTest(NewIntegrationTestArgs{ t.Views().Branches(). Lines( - Contains("branch-three").IsSelected(), - Contains("branch-four"), + Contains("branch-three"), + Contains("branch-four").IsSelected(), Contains("branch-two"), Contains("branch-one"), ) diff --git a/pkg/integration/tests/sync/fetch_when_sorted_by_date.go b/pkg/integration/tests/sync/fetch_when_sorted_by_date.go index d3230b233..4a7af151a 100644 --- a/pkg/integration/tests/sync/fetch_when_sorted_by_date.go +++ b/pkg/integration/tests/sync/fetch_when_sorted_by_date.go @@ -41,14 +41,9 @@ var FetchWhenSortedByDate = NewIntegrationTest(NewIntegrationTestArgs{ NavigateToLine(Contains("master")). Press(keys.Branches.FetchRemote). Lines( - /* EXPECTED: Contains("* branch1"), Contains("master").IsSelected(), Contains("branch2"), - ACTUAL: */ - Contains("* branch1"), - Contains("master"), - Contains("branch2").IsSelected(), ) }, })