diff --git a/pkg/commands/git_commands/branch_loader.go b/pkg/commands/git_commands/branch_loader.go index 31cbf108e..2a519cf2f 100644 --- a/pkg/commands/git_commands/branch_loader.go +++ b/pkg/commands/git_commands/branch_loader.go @@ -171,10 +171,14 @@ func (self *BranchLoader) getBehindBaseBranchValuesLegacy( for _, branch := range branches { errg.Go(func() error { - baseBranch, err := self.GetBaseBranch(branch, mainBranches) + candidates, err := self.GetBaseBranchCandidates(branch, mainBranches) if err != nil { return err } + baseBranch := "" + if len(candidates) > 0 { + baseBranch = candidates[0] + } behind := 0 // prime it in case something below fails if baseBranch != "" { output, err := self.cmd.New( @@ -354,18 +358,16 @@ func (self *BranchLoader) getBehindBaseBranchValuesFast( return nil } -// Find the base branch for the given branch (i.e. the main branch that the -// given branch was forked off of) -// -// Note that this function may return an empty string even if the returned error -// is nil, e.g. when none of the configured main branches exist. This is not -// considered an error condition, so callers need to check both the returned -// error and whether the returned base branch is empty (and possibly react -// differently in both cases). -func (self *BranchLoader) GetBaseBranch(branch *models.Branch, mainBranches *MainBranches) (string, error) { +// GetBaseBranchCandidates returns the configured main branches that are the +// closest base for the given branch — typically a single ref, but more +// when the closeness rule (smallest ahead value) leaves a tie. Candidates +// are returned in config order, so callers wanting one answer can use +// candidates[0] as the config-order tiebreak. An empty slice (with nil +// error) means no configured main branch contains the branch's merge-base. +func (self *BranchLoader) GetBaseBranchCandidates(branch *models.Branch, mainBranches *MainBranches) ([]string, error) { mergeBase := mainBranches.GetMergeBase(branch.FullRefName()) if mergeBase == "" { - return "", nil + return nil, nil } mainBranchRefs := mainBranches.Get() @@ -378,33 +380,30 @@ func (self *BranchLoader) GetBaseBranch(branch *models.Branch, mainBranches *Mai ToArgv(), ).DontLog().RunWithOutput() if err != nil { - return "", err + return nil, err } trimmedOutput := strings.TrimSpace(output) if trimmedOutput == "" { - return "", nil + return nil, nil } contained := strings.Split(trimmedOutput, "\n") // for-each-ref sorts its output alphabetically by refname regardless of // the order we passed the refs in. Restore the user's configured order so // it can serve as the natural tiebreaker. - candidates := lo.Filter(mainBranchRefs, func(ref string, _ int) bool { + containing := lo.Filter(mainBranchRefs, func(ref string, _ int) bool { return lo.Contains(contained, ref) }) - if len(candidates) == 0 { - return "", nil - } - if len(candidates) == 1 { - return candidates[0], nil + if len(containing) <= 1 { + return containing, nil } // Multiple main branches contain the merge-base. Measure ahead/behind // against each and hand off to selectBaseForBranch — the same selector // the fast path uses — so both paths agree on the closeness rule and // the config-order tiebreak. - aheadBehinds := make([]aheadBehind, len(candidates)) - for i, ref := range candidates { + aheadBehinds := make([]aheadBehind, len(containing)) + for i, ref := range containing { revListOutput, err := self.cmd.New( NewGitCmd("rev-list"). Arg("--left-right"). @@ -413,17 +412,17 @@ func (self *BranchLoader) GetBaseBranch(branch *models.Branch, mainBranches *Mai ToArgv(), ).DontLog().RunWithOutput() if err != nil { - return "", err + return nil, err } aheadBehinds[i] = parseAheadBehindField(strings.TrimSpace(revListOutput)) } - winner, _, _ := selectBaseForBranch(aheadBehinds, candidates) - if winner == "" { + _, _, candidates := selectBaseForBranch(aheadBehinds, containing) + if len(candidates) == 0 { // Every rev-list output was malformed; fall back to config order. - return candidates[0], nil + return containing, nil } - return winner, nil + return candidates, nil } func (self *BranchLoader) obtainBranches() []*models.Branch { diff --git a/pkg/commands/git_commands/branch_loader_test.go b/pkg/commands/git_commands/branch_loader_test.go index fe004adc9..07b6d1584 100644 --- a/pkg/commands/git_commands/branch_loader_test.go +++ b/pkg/commands/git_commands/branch_loader_test.go @@ -492,8 +492,8 @@ func TestGetBehindBaseBranchValuesForAllBranches_LegacyPath(t *testing.T) { {Name: "feat-x"}, } - // In legacy path: per-branch GetBaseBranch (merge-base + for-each-ref --contains) - // then rev-list --left-right --count. + // In legacy path: per-branch GetBaseBranchCandidates (merge-base + + // for-each-ref --contains) then rev-list --left-right --count. runner := oscommands.NewFakeRunner(t). ExpectGitArgs([]string{"merge-base", "refs/heads/feat-x", "refs/heads/master"}, "abc123\n", nil). ExpectGitArgs([]string{"for-each-ref", "--contains", "abc123", "--format=%(refname)", "refs/heads/master"}, "refs/heads/master\n", nil). @@ -527,10 +527,10 @@ func TestGetBehindBaseBranchValuesForAllBranches_LegacyPath(t *testing.T) { } // When the branch's merge-base is contained in more than one configured main -// branch and the ahead counts are equal, the chosen base must respect the -// user's configured order rather than the alphabetical order of +// branch and the ahead counts are equal, the candidate list must preserve +// the user's configured order rather than the alphabetical order of // for-each-ref's output. -func TestGetBaseBranch_AmbiguousFallsBackToConfigOrder(t *testing.T) { +func TestGetBaseBranchCandidates_AmbiguousReturnsAllInConfigOrder(t *testing.T) { mainBranchRefs := []string{"refs/heads/main", "refs/heads/develop"} branch := &models.Branch{Name: "feat-x"} @@ -566,9 +566,9 @@ func TestGetBaseBranch_AmbiguousFallsBackToConfigOrder(t *testing.T) { previousMainBranches: gitCommon.Common.UserConfig().Git.MainBranches, } - baseBranch, err := loader.GetBaseBranch(branch, mainBranches) + candidates, err := loader.GetBaseBranchCandidates(branch, mainBranches) assert.NoError(t, err) - assert.Equal(t, "refs/heads/main", baseBranch) + assert.Equal(t, []string{"refs/heads/main", "refs/heads/develop"}, candidates) runner.CheckForMissingCalls() } @@ -576,8 +576,9 @@ func TestGetBaseBranch_AmbiguousFallsBackToConfigOrder(t *testing.T) { // When a configured main branch has a strictly smaller ahead count than any // other (e.g. the branch was forked off `main` after main's last merge into // `develop`, so `develop` doesn't yet contain the fork point's recent main -// history), that base wins outright regardless of config order. -func TestGetBaseBranch_UnambiguousPicksSmallestAhead(t *testing.T) { +// history), that base wins outright regardless of config order, so only +// that one ref is returned. +func TestGetBaseBranchCandidates_UnambiguousReturnsSmallestAheadOnly(t *testing.T) { mainBranchRefs := []string{"refs/heads/develop", "refs/heads/main"} branch := &models.Branch{Name: "feat-x"} @@ -613,9 +614,9 @@ func TestGetBaseBranch_UnambiguousPicksSmallestAhead(t *testing.T) { previousMainBranches: gitCommon.Common.UserConfig().Git.MainBranches, } - baseBranch, err := loader.GetBaseBranch(branch, mainBranches) + candidates, err := loader.GetBaseBranchCandidates(branch, mainBranches) assert.NoError(t, err) - assert.Equal(t, "refs/heads/main", baseBranch) + assert.Equal(t, []string{"refs/heads/main"}, candidates) runner.CheckForMissingCalls() } diff --git a/pkg/gui/controllers/branches_controller.go b/pkg/gui/controllers/branches_controller.go index 27bef4b66..c82773525 100644 --- a/pkg/gui/controllers/branches_controller.go +++ b/pkg/gui/controllers/branches_controller.go @@ -291,10 +291,14 @@ func (self *BranchesController) viewUpstreamOptions(selectedBranch *models.Branc } var disabledReason *types.DisabledReason - baseBranch, err := self.c.Git().Loaders.BranchLoader.GetBaseBranch(selectedBranch, self.c.Model().MainBranches) + candidates, err := self.c.Git().Loaders.BranchLoader.GetBaseBranchCandidates(selectedBranch, self.c.Model().MainBranches) if err != nil { return err } + baseBranch := "" + if len(candidates) > 0 { + baseBranch = candidates[0] + } if baseBranch == "" { baseBranch = self.c.Tr.CouldNotDetermineBaseBranch disabledReason = &types.DisabledReason{Text: self.c.Tr.CouldNotDetermineBaseBranch} diff --git a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go index be1d7b3a9..1707d89bc 100644 --- a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go +++ b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go @@ -343,10 +343,14 @@ func (self *MergeAndRebaseHelper) RebaseOntoRef(ref string) error { disabledReason = &types.DisabledReason{Text: self.c.Tr.CantRebaseOntoSelf} } - baseBranch, err := self.c.Git().Loaders.BranchLoader.GetBaseBranch(checkedOutBranch, self.c.Model().MainBranches) + candidates, err := self.c.Git().Loaders.BranchLoader.GetBaseBranchCandidates(checkedOutBranch, self.c.Model().MainBranches) if err != nil { return err } + baseBranch := "" + if len(candidates) > 0 { + baseBranch = candidates[0] + } if baseBranch == "" { baseBranch = self.c.Tr.CouldNotDetermineBaseBranch baseBranchDisabledReason = &types.DisabledReason{Text: self.c.Tr.CouldNotDetermineBaseBranch} diff --git a/pkg/gui/controllers/helpers/refs_helper.go b/pkg/gui/controllers/helpers/refs_helper.go index 99e9f47ec..619c6c1e1 100644 --- a/pkg/gui/controllers/helpers/refs_helper.go +++ b/pkg/gui/controllers/helpers/refs_helper.go @@ -428,10 +428,14 @@ func (self *RefsHelper) NewBranch(from string, fromFormattedName string, suggest func (self *RefsHelper) MoveCommitsToNewBranch() error { currentBranch := self.c.Model().Branches[0] - baseBranchRef, err := self.c.Git().Loaders.BranchLoader.GetBaseBranch(currentBranch, self.c.Model().MainBranches) + candidates, err := self.c.Git().Loaders.BranchLoader.GetBaseBranchCandidates(currentBranch, self.c.Model().MainBranches) if err != nil { return err } + baseBranchRef := "" + if len(candidates) > 0 { + baseBranchRef = candidates[0] + } withNewBranchNamePrompt := func(baseBranchName string, f func(string) error) error { prompt := utils.ResolvePlaceholderString(