From 1441359ccc3d2ecb227cfc29cce6f00e7065e2fa Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 19 Sep 2026 16:48:35 +0200 Subject: [PATCH] Keep the positions of malformed ahead-behind fields parseAheadBehindForEachRefOutput dropped fields it couldn't parse, which left the remaining ones of that line pointing at the wrong bases. The caller that picks the closest base doesn't care, but sorting refs by ancestry has to know which base a pair of numbers belongs to. Return an entry per base and mark the ones that were malformed, as the function's comment has claimed all along. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/commands/git_commands/ahead_behind.go | 19 +++++---- .../git_commands/ahead_behind_test.go | 42 ++++++++++--------- 2 files changed, 34 insertions(+), 27 deletions(-) diff --git a/pkg/commands/git_commands/ahead_behind.go b/pkg/commands/git_commands/ahead_behind.go index f142bf9f4..de8dcbbce 100644 --- a/pkg/commands/git_commands/ahead_behind.go +++ b/pkg/commands/git_commands/ahead_behind.go @@ -10,6 +10,7 @@ import ( // Holds parsed values from a single %(ahead-behind:) field. type aheadBehind struct { ahead, behind int + valid bool } type branchAheadBehind struct { @@ -23,7 +24,8 @@ 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, so +// that the entries of a line stay aligned with the bases. func parseAheadBehindForEachRefOutput( output string, numBases int, // number of %(ahead-behind:...) tokens @@ -39,7 +41,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{ @@ -51,17 +53,17 @@ 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 @@ -69,7 +71,10 @@ func parseAheadBehindField(s string) (aheadBehind, bool) { // its behind value. // Ties are broken by index order func selectBehindForBranch(aheadBehinds []aheadBehind) int { - return lo.MinBy(aheadBehinds, func(a, b aheadBehind) bool { + validOnes := lo.Filter(aheadBehinds, func(ab aheadBehind, _ int) bool { + return ab.valid + }) + return lo.MinBy(validOnes, func(a, b aheadBehind) bool { return a.ahead < b.ahead }).behind } diff --git a/pkg/commands/git_commands/ahead_behind_test.go b/pkg/commands/git_commands/ahead_behind_test.go index e963dc975..2fe040633 100644 --- a/pkg/commands/git_commands/ahead_behind_test.go +++ b/pkg/commands/git_commands/ahead_behind_test.go @@ -22,7 +22,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}}, }, }, }, @@ -35,15 +35,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}, }, }, }, @@ -56,7 +56,8 @@ func TestParseAheadBehindForEachRefOutput(t *testing.T) { { refName: "refs/heads/feat", aheadBehinds: []aheadBehind{ - {ahead: 2, behind: 5}, + {}, + {ahead: 2, behind: 5, valid: true}, }, }, }, @@ -68,7 +69,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}}, }, }, }, @@ -79,7 +80,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}}, }, }, }, @@ -92,11 +93,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}}, }, }, }, @@ -107,7 +108,7 @@ func TestParseAheadBehindForEachRefOutput(t *testing.T) { expected: []branchAheadBehind{ { refName: "refs/heads/feat", - aheadBehinds: []aheadBehind{}, + aheadBehinds: []aheadBehind{{}}, }, }, }, @@ -137,14 +138,14 @@ func TestSelectBehindForBranch(t *testing.T) { scenarios := []scenario{ { testName: "single base, valid value", - aheadBehinds: []aheadBehind{{ahead: 3, behind: 7}}, + aheadBehinds: []aheadBehind{{ahead: 3, behind: 7, valid: true}}, expected: 7, }, { 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, }, @@ -155,29 +156,30 @@ 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, }, { 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 }, expected: 10, }, { testName: "first base invalid, second valid", aheadBehinds: []aheadBehind{ - {ahead: 3, behind: 8}, + {}, + {ahead: 3, behind: 8, valid: true}, }, expected: 8, }, { testName: "all invalid - returns 0", - aheadBehinds: []aheadBehind{}, + aheadBehinds: []aheadBehind{{}, {}}, expected: 0, }, {