diff --git a/pkg/commands/git_commands/branch_loader.go b/pkg/commands/git_commands/branch_loader.go index 321141c69..f5f96f1d3 100644 --- a/pkg/commands/git_commands/branch_loader.go +++ b/pkg/commands/git_commands/branch_loader.go @@ -206,9 +206,13 @@ func (self *BranchLoader) getBehindBaseBranchValuesLegacy( return err } -// Holds parsed values from a single %(ahead-behind:) field. +// Holds parsed values from a single %(ahead-behind:) field. `valid` +// is false when the field failed to parse (e.g. the base was unreachable +// from this ref); the entry is preserved so that the slice stays index- +// aligned with the configured main branches. type aheadBehind struct { ahead, behind int + valid bool } type branchAheadBehind struct { @@ -222,7 +226,7 @@ type branchAheadBehind struct { // // Lines whose NUL-split column count doesn't match (1 + numBases) are dropped. // Blank lines are ignored. -// Individual malformed ahead-behind fields produce {valid: false} entries +// Individual malformed ahead-behind fields produce {valid: false} entries. func parseAheadBehindForEachRefOutput( output string, numBases int, // number of %(ahead-behind:...) tokens @@ -238,7 +242,7 @@ func parseAheadBehindForEachRefOutput( continue } refName := cols[0] - aheadBehinds := lo.FilterMap(cols[1:], func(col string, _ int) (aheadBehind, bool) { + aheadBehinds := lo.Map(cols[1:], func(col string, _ int) aheadBehind { return parseAheadBehindField(col) }) entry := branchAheadBehind{ @@ -250,27 +254,48 @@ func parseAheadBehindForEachRefOutput( return result } -func parseAheadBehindField(s string) (aheadBehind, bool) { +func parseAheadBehindField(s string) aheadBehind { parts := strings.Fields(s) if len(parts) != 2 { - return aheadBehind{}, false + return aheadBehind{} } ahead, err1 := strconv.Atoi(parts[0]) behind, err2 := strconv.Atoi(parts[1]) if err1 != nil || err2 != nil { - return aheadBehind{}, false + return aheadBehind{} } - return aheadBehind{ahead: ahead, behind: behind}, true + return aheadBehind{ahead: ahead, behind: behind, valid: true} } -// Picks the "closest" base by smallest ahead value (commits the branch -// has that the base doesn't = roughly "since fork point") and returns -// its behind value. -// Ties are broken by index order -func selectBehindForBranch(aheadBehinds []aheadBehind) int { - return lo.MinBy(aheadBehinds, func(a, b aheadBehind) bool { - return a.ahead < b.ahead - }).behind +// selectBaseForBranch picks the closest base for a branch given (ahead, +// behind) measurements against each configured main branch. "Closest" = +// smallest ahead value (fewest branch commits not in the base). Ties are +// broken by the order of mainRefs (i.e. config order). +// +// aheadBehinds must be index-aligned with mainRefs; invalid entries are +// skipped. Returns the winning ref, its behind value, and the full set +// of refs tied at the minimum ahead (in config order). The caller can +// detect ambiguity via `len(candidates) > 1`. With no valid entry the +// return is ("", 0, nil). +func selectBaseForBranch( + aheadBehinds []aheadBehind, mainRefs []string, +) (winner string, behind int, candidates []string) { + bestAhead := -1 + for i, ab := range aheadBehinds { + if !ab.valid { + continue + } + switch { + case bestAhead < 0 || ab.ahead < bestAhead: + bestAhead = ab.ahead + winner = mainRefs[i] + behind = ab.behind + candidates = []string{mainRefs[i]} + case ab.ahead == bestAhead: + candidates = append(candidates, mainRefs[i]) + } + } + return winner, behind, candidates } // The output format is: @@ -313,7 +338,7 @@ func (self *BranchLoader) getBehindBaseBranchValuesFast( for _, p := range parsed { if branch, ok := branchByRef[p.refName]; ok { - behind := selectBehindForBranch(p.aheadBehinds) + _, behind, _ := selectBaseForBranch(p.aheadBehinds, mainBranchRefs) branch.BehindBaseBranch.Store(int32(behind)) delete(branchByRef, p.refName) } diff --git a/pkg/commands/git_commands/branch_loader_test.go b/pkg/commands/git_commands/branch_loader_test.go index 2a0da414e..fe004adc9 100644 --- a/pkg/commands/git_commands/branch_loader_test.go +++ b/pkg/commands/git_commands/branch_loader_test.go @@ -142,7 +142,7 @@ func TestParseAheadBehindForEachRefOutput(t *testing.T) { expected: []branchAheadBehind{ { refName: "refs/heads/feat", - aheadBehinds: []aheadBehind{{ahead: 2, behind: 5}}, + aheadBehinds: []aheadBehind{{ahead: 2, behind: 5, valid: true}}, }, }, }, @@ -155,15 +155,15 @@ func TestParseAheadBehindForEachRefOutput(t *testing.T) { { refName: "refs/heads/feat", aheadBehinds: []aheadBehind{ - {ahead: 2, behind: 5}, - {ahead: 10, behind: 1}, + {ahead: 2, behind: 5, valid: true}, + {ahead: 10, behind: 1, valid: true}, }, }, { refName: "refs/heads/main", aheadBehinds: []aheadBehind{ - {ahead: 0, behind: 0}, - {ahead: 0, behind: 0}, + {ahead: 0, behind: 0, valid: true}, + {ahead: 0, behind: 0, valid: true}, }, }, }, @@ -176,7 +176,8 @@ func TestParseAheadBehindForEachRefOutput(t *testing.T) { { refName: "refs/heads/feat", aheadBehinds: []aheadBehind{ - {ahead: 2, behind: 5}, + {valid: false}, + {ahead: 2, behind: 5, valid: true}, }, }, }, @@ -188,7 +189,7 @@ func TestParseAheadBehindForEachRefOutput(t *testing.T) { expected: []branchAheadBehind{ { refName: "refs/heads/feat/foo-bar", - aheadBehinds: []aheadBehind{{ahead: 1, behind: 2}}, + aheadBehinds: []aheadBehind{{ahead: 1, behind: 2, valid: true}}, }, }, }, @@ -199,7 +200,7 @@ func TestParseAheadBehindForEachRefOutput(t *testing.T) { expected: []branchAheadBehind{ { refName: "refs/heads/feat", - aheadBehinds: []aheadBehind{{ahead: 1, behind: 2}}, + aheadBehinds: []aheadBehind{{ahead: 1, behind: 2, valid: true}}, }, }, }, @@ -212,11 +213,11 @@ func TestParseAheadBehindForEachRefOutput(t *testing.T) { expected: []branchAheadBehind{ { refName: "refs/heads/good", - aheadBehinds: []aheadBehind{{ahead: 1, behind: 2}}, + aheadBehinds: []aheadBehind{{ahead: 1, behind: 2, valid: true}}, }, { refName: "refs/heads/also_good", - aheadBehinds: []aheadBehind{{ahead: 3, behind: 4}}, + aheadBehinds: []aheadBehind{{ahead: 3, behind: 4, valid: true}}, }, }, }, @@ -227,7 +228,7 @@ func TestParseAheadBehindForEachRefOutput(t *testing.T) { expected: []branchAheadBehind{ { refName: "refs/heads/feat", - aheadBehinds: []aheadBehind{}, + aheadBehinds: []aheadBehind{{valid: false}}, }, }, }, @@ -247,26 +248,35 @@ func TestParseAheadBehindForEachRefOutput(t *testing.T) { } } -func TestSelectBehindForBranch(t *testing.T) { +func TestSelectBaseForBranch(t *testing.T) { type scenario struct { - testName string - aheadBehinds []aheadBehind - expected int + testName string + aheadBehinds []aheadBehind + mainRefs []string + expectedWinner string + expectedBehind int + expectedCandidates []string } scenarios := []scenario{ { - testName: "single base, valid value", - aheadBehinds: []aheadBehind{{ahead: 3, behind: 7}}, - expected: 7, + testName: "single base, valid value", + aheadBehinds: []aheadBehind{{ahead: 3, behind: 7, valid: true}}, + mainRefs: []string{"refs/heads/master"}, + expectedWinner: "refs/heads/master", + expectedBehind: 7, + expectedCandidates: []string{"refs/heads/master"}, }, { testName: "multi-base, clear winner by ahead", aheadBehinds: []aheadBehind{ - {ahead: 50, behind: 10}, // master - {ahead: 5, behind: 2}, // develop ← smallest ahead + {ahead: 50, behind: 10, valid: true}, // master + {ahead: 5, behind: 2, valid: true}, // develop ← smallest ahead }, - expected: 2, + mainRefs: []string{"refs/heads/master", "refs/heads/develop"}, + expectedWinner: "refs/heads/develop", + expectedBehind: 2, + expectedCandidates: []string{"refs/heads/develop"}, }, { testName: "develop forked from master case (ancestor-of-each-other)", @@ -275,42 +285,66 @@ func TestSelectBehindForBranch(t *testing.T) { // ahead vs master = 5 + 50 = 55; behind vs master = 0 // ahead vs develop = 5; behind vs develop = 5 aheadBehinds: []aheadBehind{ - {ahead: 55, behind: 0}, // master - {ahead: 5, behind: 5}, // develop ← smallest ahead + {ahead: 55, behind: 0, valid: true}, // master + {ahead: 5, behind: 5, valid: true}, // develop ← smallest ahead }, - expected: 5, + mainRefs: []string{"refs/heads/master", "refs/heads/develop"}, + expectedWinner: "refs/heads/develop", + expectedBehind: 5, + expectedCandidates: []string{"refs/heads/develop"}, }, { testName: "tie on ahead - first base wins (config order)", aheadBehinds: []aheadBehind{ - {ahead: 5, behind: 10}, // first - {ahead: 5, behind: 99}, // second, same ahead + {ahead: 5, behind: 10, valid: true}, // first + {ahead: 5, behind: 99, valid: true}, // second, same ahead + }, + mainRefs: []string{"refs/heads/main", "refs/heads/develop"}, + expectedWinner: "refs/heads/main", + expectedBehind: 10, + expectedCandidates: []string{ + "refs/heads/main", + "refs/heads/develop", }, - expected: 10, }, { testName: "first base invalid, second valid", aheadBehinds: []aheadBehind{ - {ahead: 3, behind: 8}, + {valid: false}, + {ahead: 3, behind: 8, valid: true}, }, - expected: 8, + mainRefs: []string{"refs/heads/master", "refs/heads/develop"}, + expectedWinner: "refs/heads/develop", + expectedBehind: 8, + expectedCandidates: []string{"refs/heads/develop"}, }, { - testName: "all invalid - returns 0", - aheadBehinds: []aheadBehind{}, - expected: 0, + testName: "all invalid - returns empty", + aheadBehinds: []aheadBehind{ + {valid: false}, + {valid: false}, + }, + mainRefs: []string{"refs/heads/master", "refs/heads/develop"}, + expectedWinner: "", + expectedBehind: 0, + expectedCandidates: nil, }, { - testName: "empty - returns 0", - aheadBehinds: nil, - expected: 0, + testName: "empty - returns empty", + aheadBehinds: nil, + mainRefs: nil, + expectedWinner: "", + expectedBehind: 0, + expectedCandidates: nil, }, } for _, s := range scenarios { t.Run(s.testName, func(t *testing.T) { - result := selectBehindForBranch(s.aheadBehinds) - assert.Equal(t, s.expected, result) + winner, behind, candidates := selectBaseForBranch(s.aheadBehinds, s.mainRefs) + assert.Equal(t, s.expectedWinner, winner) + assert.Equal(t, s.expectedBehind, behind) + assert.Equal(t, s.expectedCandidates, candidates) }) } }