Return all tied candidates from base-branch detection

The four callers of GetBaseBranch all ultimately want a single ref,
but the disambiguation prompt we're about to add needs to know when
multiple main branches are genuinely tied at the closest position so
it can ask the user. Change the function to return all min-ahead refs
in config order and rename to GetBaseBranchCandidates; each caller
now takes candidates[0] as a placeholder until the prompt wiring
lands.
This commit is contained in:
Stefan Haller 2026-05-21 20:41:42 +02:00
parent 5a9e8c6af9
commit 84bd2b6fe1
5 changed files with 52 additions and 40 deletions

View file

@ -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 {

View file

@ -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()
}

View file

@ -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}

View file

@ -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}

View file

@ -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(