From 1a67134bc83f5eb4b611f49e63bc8b78bfa1ea12 Mon Sep 17 00:00:00 2001 From: Matt Vinall Date: Sat, 2 May 2026 07:22:48 +0100 Subject: [PATCH] chore: refactor, remove redundant field members --- scanner/branch_status.go | 118 ++++++++++--------------- scanner/branch_status_internal_test.go | 9 +- scanner/scan.go | 8 +- scanner/types.go | 21 ++--- ui/status_layout_test.go | 28 +++--- ui/view_test.go | 5 +- 6 files changed, 76 insertions(+), 113 deletions(-) diff --git a/scanner/branch_status.go b/scanner/branch_status.go index 426aa95..b6a2feb 100644 --- a/scanner/branch_status.go +++ b/scanner/branch_status.go @@ -111,59 +111,46 @@ func refTip(d, ref string) (hash string, unix int64, exists bool, err error) { return parts[0], unix, true, nil } -func uniqueCommitInfo(d, ref string, otherRefs []string) (count int, newestUnix int64, err error) { +// branchLocationRef returns the full ref for a location row (local heads vs +// refs/remotes) for use with git commands; it is not stored on [BranchLocation]. +func branchLocationRef(locationName, branchName string) string { + if locationName == "local" { + return "refs/heads/" + branchName + } + return "refs/remotes/" + locationName + "/" + branchName +} + +func uniqueCommitCount(d, ref string, otherRefs []string) (count int, err error) { if len(otherRefs) == 0 { - return 0, 0, nil + return 0, nil } args := []string{"rev-list", "--count", ref, "--not"} args = append(args, otherRefs...) out, err := runGit(d, args...) if err != nil { - return 0, 0, err + return 0, err } count, err = strconv.Atoi(strings.TrimSpace(out)) if err != nil { - return 0, 0, err - } - if count == 0 { - return 0, 0, nil - } - - args = []string{"log", "-1", "--format=%ct", ref, "--not"} - args = append(args, otherRefs...) - out, err = runGit(d, args...) - if err != nil { - return count, 0, err + return 0, err } - if strings.TrimSpace(out) == "" { - return count, 0, nil - } - newestUnix, err = strconv.ParseInt(strings.TrimSpace(out), 10, 64) - if err != nil { - return count, 0, err - } - return count, newestUnix, nil + return count, nil } // computeBranchLocations compares refs/heads/ to refs/remotes// -// for each configured remote and fills UniqueCount / NewestUniqueUnix per location. -func computeBranchLocations(d, branchName string, remotes []string) ([]BranchLocation, string, error) { +// for each configured remote and fills UniqueCount per location. +func computeBranchLocations(d, branchName string, remotes []string) ([]BranchLocation, error) { locations := make([]BranchLocation, 0, 1+len(remotes)) - locations = append(locations, BranchLocation{ - Name: "local", - Ref: "refs/heads/" + branchName, - }) + locations = append(locations, BranchLocation{Name: "local"}) for _, remote := range remotes { - locations = append(locations, BranchLocation{ - Name: remote, - Ref: "refs/remotes/" + remote + "/" + branchName, - }) + locations = append(locations, BranchLocation{Name: remote}) } for i := range locations { - hash, unix, exists, err := refTip(d, locations[i].Ref) + ref := branchLocationRef(locations[i].Name, branchName) + hash, unix, exists, err := refTip(d, ref) if err != nil { - return nil, "", err + return nil, err } locations[i].Exists = exists locations[i].TipHash = hash @@ -173,63 +160,56 @@ func computeBranchLocations(d, branchName string, remotes []string) ([]BranchLoc existingRefs := make([]string, 0, len(locations)) for _, loc := range locations { if loc.Exists { - existingRefs = append(existingRefs, loc.Ref) + existingRefs = append(existingRefs, branchLocationRef(loc.Name, branchName)) } } for i := range locations { if !locations[i].Exists { continue } + ref := branchLocationRef(locations[i].Name, branchName) others := make([]string, 0, len(existingRefs)-1) - for _, ref := range existingRefs { - if ref != locations[i].Ref { - others = append(others, ref) + for _, otherRef := range existingRefs { + if otherRef != ref { + others = append(others, otherRef) } } - count, newestUnix, err := uniqueCommitInfo(d, locations[i].Ref, others) + count, err := uniqueCommitCount(d, ref, others) if err != nil { - return nil, "", err + return nil, err } locations[i].UniqueCount = count - locations[i].NewestUniqueUnix = newestUnix } if len(locations) > 0 && locations[0].Exists { - localRef := locations[0].Ref + localRef := branchLocationRef("local", branchName) for i := 1; i < len(locations); i++ { if !locations[i].Exists { continue } related, err := haveMergeBase(d, locations[0].TipHash, locations[i].TipHash) if err != nil { - return nil, "", err + return nil, err } if !related { locations[i].HistoriesUnrelated = true continue } - incoming, _, err := uniqueCommitInfo(d, locations[i].Ref, []string{localRef}) + remoteRef := branchLocationRef(locations[i].Name, branchName) + incoming, err := uniqueCommitCount(d, remoteRef, []string{localRef}) if err != nil { - return nil, "", err + return nil, err } - outgoing, _, err := uniqueCommitInfo(d, localRef, []string{locations[i].Ref}) + outgoing, err := uniqueCommitCount(d, localRef, []string{remoteRef}) if err != nil { - return nil, "", err + return nil, err } locations[i].Incoming = incoming locations[i].Outgoing = outgoing } } - newestLocation := "" - var newestUnix int64 - for _, loc := range locations { - if loc.NewestUniqueUnix > newestUnix { - newestUnix = loc.NewestUniqueUnix - newestLocation = loc.Name - } - } - return locations, newestLocation, nil + return locations, nil } func GitBranchStatus(d string) (BranchStatus, error) { @@ -256,46 +236,42 @@ func GitBranchStatus(d string) (BranchStatus, error) { } if len(locals) == 0 { - locations, newestLocation, err := computeBranchLocations(d, branch, remotes) + locations, err := computeBranchLocations(d, branch, remotes) if err != nil { return BranchStatus{}, err } return BranchStatus{ - Branch: branch, - Detached: false, - Locations: locations, - NewestLocation: newestLocation, - LocalBranches: nil, + Branch: branch, + Detached: false, + Locations: locations, + LocalBranches: nil, }, nil } var topLocations []BranchLocation - var topNewest string for i := range locals { - locs, newest, err := computeBranchLocations(d, locals[i].Name, remotes) + locs, err := computeBranchLocations(d, locals[i].Name, remotes) if err != nil { return BranchStatus{}, err } locals[i].Locations = locs if locals[i].Current { topLocations = locs - topNewest = newest } } if topLocations == nil { var err2 error - topLocations, topNewest, err2 = computeBranchLocations(d, branch, remotes) + topLocations, err2 = computeBranchLocations(d, branch, remotes) if err2 != nil { return BranchStatus{}, err2 } } return BranchStatus{ - Branch: branch, - Detached: false, - Locations: topLocations, - NewestLocation: topNewest, - LocalBranches: locals, + Branch: branch, + Detached: false, + Locations: topLocations, + LocalBranches: locals, }, nil } diff --git a/scanner/branch_status_internal_test.go b/scanner/branch_status_internal_test.go index 0bae3fc..f3b5cfc 100644 --- a/scanner/branch_status_internal_test.go +++ b/scanner/branch_status_internal_test.go @@ -44,7 +44,7 @@ func TestHaveMergeBaseUnrelatedHistories(t *testing.T) { } } -func TestUniqueCommitInfoAheadOnBranch(t *testing.T) { +func TestUniqueCommitCountAheadOnBranch(t *testing.T) { dir := t.TempDir() gitMinimalInit(t, dir) gitCommitFile(t, dir, "f.txt", "v0\n", "base") @@ -52,16 +52,13 @@ func TestUniqueCommitInfoAheadOnBranch(t *testing.T) { gitCommitFile(t, dir, "f.txt", "v0\nv1\n", "ahead-only") execGit(t, dir, "checkout", "main") - n, newest, err := uniqueCommitInfo(dir, "refs/heads/ahead", []string{"refs/heads/main"}) + n, err := uniqueCommitCount(dir, "refs/heads/ahead", []string{"refs/heads/main"}) if err != nil { - t.Fatalf("uniqueCommitInfo: %v", err) + t.Fatalf("uniqueCommitCount: %v", err) } if n != 1 { t.Fatalf("unique commit count = %d, want 1", n) } - if newest == 0 { - t.Fatal("expected positive unix time for newest unique commit") - } } func execGit(t *testing.T, dir string, args ...string) { diff --git a/scanner/scan.go b/scanner/scan.go index dd48b06..a8c0ce8 100644 --- a/scanner/scan.go +++ b/scanner/scan.go @@ -65,7 +65,9 @@ func ScanWithProgress(config *Config, onProgress func(ScanProgress)) (*MultiGitS CurrentPath: d, }) + statusStart := time.Now() rs, include, err := StatusForRepo(config, d) + statusDur := time.Since(statusStart) if err != nil { return err } @@ -78,10 +80,10 @@ func ScanWithProgress(config *Config, onProgress func(ScanProgress)) (*MultiGitS ReposChecked: int(n), CurrentPath: d, }) - log.Println(d, rs.ScanTime) + log.Println(d, statusDur) if include { - atomic.AddInt64(&totalStatusDuration, rs.ScanTime.Nanoseconds()) + atomic.AddInt64(&totalStatusDuration, statusDur.Nanoseconds()) results.AddResult(d, rs) } return nil @@ -106,7 +108,6 @@ func ScanWithProgress(config *Config, onProgress func(ScanProgress)) (*MultiGitS // branch metadata is collected (used by [ScanWithProgress] for progress timing). func StatusForRepo(config *Config, dir string) (RepoStatus, bool, error) { ex := NewExcluder(config.GitIgnore.FileGlob, config.GitIgnore.DirGlob) - start := time.Now() porcelain, err := GitStatus(dir) if err != nil { return RepoStatus{}, false, err @@ -124,7 +125,6 @@ func StatusForRepo(config *Config, dir string) (RepoStatus, bool, error) { Status: st, Porcelain: porcelain, Branches: branches, - ScanTime: time.Since(start), } include := !st.IsClean() || branches.HasLocalRemoteMismatch() return rs, include, nil diff --git a/scanner/types.go b/scanner/types.go index 283b57a..ebf12dd 100644 --- a/scanner/types.go +++ b/scanner/types.go @@ -5,7 +5,6 @@ import ( "os" "regexp" "strings" - "time" "github.com/go-git/go-git/v5" ) @@ -15,14 +14,12 @@ type RepoStatus struct { Porcelain PorcelainStatus Branches BranchStatus - ScanTime time.Duration } type BranchStatus struct { - Branch string - Detached bool - Locations []BranchLocation - NewestLocation string + Branch string + Detached bool + Locations []BranchLocation // LocalBranches lists refs/heads in name order (from git for-each-ref). LocalBranches []LocalBranchRef } @@ -92,13 +89,11 @@ func (lb LocalBranchRef) IsLocalOnly() bool { } type BranchLocation struct { - Name string - Ref string - Exists bool - TipHash string - TipUnix int64 - UniqueCount int - NewestUniqueUnix int64 + Name string + Exists bool + TipHash string + TipUnix int64 + UniqueCount int // Incoming/Outgoing compare this ref to the local branch ref only (remote // rows). Incoming is commits reachable from this remote but not local (+N); // Outgoing is commits on local not reachable from this remote (UI: -M). diff --git a/ui/status_layout_test.go b/ui/status_layout_test.go index 674d6bb..b426163 100644 --- a/ui/status_layout_test.go +++ b/ui/status_layout_test.go @@ -329,18 +329,17 @@ func TestRefreshBranchContentOneRowPerBranch(t *testing.T) { m.repoList = []string{"/repo"} m.repositories.Set("/repo", scanner.RepoStatus{ Branches: scanner.BranchStatus{ - Branch: "aaa", - NewestLocation: "origin", + Branch: "aaa", Locations: []scanner.BranchLocation{ {Name: "local", Exists: true, TipHash: "aaaaaaaaaaaaaaaa", TipUnix: 1_700_000_000, UniqueCount: 2}, - {Name: "origin", Exists: true, TipHash: "bbbbbbbbbbbbbbbb", TipUnix: 1_700_000_001, UniqueCount: 1, NewestUniqueUnix: 1_700_000_001, Incoming: 1, Outgoing: 2}, + {Name: "origin", Exists: true, TipHash: "bbbbbbbbbbbbbbbb", TipUnix: 1_700_000_001, UniqueCount: 1, Incoming: 1, Outgoing: 2}, {Name: "upstream", Exists: false}, }, // Names sort opposite to recency so the test proves UI order is by tip time, not name. LocalBranches: []scanner.LocalBranchRef{ {Name: "aaa", TipHash: "aaaaaaaaaaaaaaaa", TipUnix: 1_700_000_000, Current: true, Locations: []scanner.BranchLocation{ {Name: "local", Exists: true, TipHash: "aaaaaaaaaaaaaaaa", TipUnix: 1_700_000_000, UniqueCount: 2}, - {Name: "origin", Exists: true, TipHash: "bbbbbbbbbbbbbbbb", TipUnix: 1_700_000_001, UniqueCount: 1, NewestUniqueUnix: 1_700_000_001, Incoming: 1, Outgoing: 2}, + {Name: "origin", Exists: true, TipHash: "bbbbbbbbbbbbbbbb", TipUnix: 1_700_000_001, UniqueCount: 1, Incoming: 1, Outgoing: 2}, {Name: "upstream", Exists: false}, }}, {Name: "zzz", TipHash: "cccccccccccccccc", TipUnix: 1_700_000_002, Current: false, Locations: []scanner.BranchLocation{ @@ -394,16 +393,15 @@ branches: m.repoList = []string{"/repo"} m.repositories.Set("/repo", scanner.RepoStatus{ Branches: scanner.BranchStatus{ - Branch: "aaa", - NewestLocation: "origin", + Branch: "aaa", Locations: []scanner.BranchLocation{ {Name: "local", Exists: true, TipHash: "aaaaaaaaaaaaaaaa", TipUnix: 1_700_000_000, UniqueCount: 2}, - {Name: "origin", Exists: true, TipHash: "bbbbbbbbbbbbbbbb", TipUnix: 1_700_000_001, UniqueCount: 1, NewestUniqueUnix: 1_700_000_001, Incoming: 1, Outgoing: 2}, + {Name: "origin", Exists: true, TipHash: "bbbbbbbbbbbbbbbb", TipUnix: 1_700_000_001, UniqueCount: 1, Incoming: 1, Outgoing: 2}, }, LocalBranches: []scanner.LocalBranchRef{ {Name: "aaa", TipHash: "aaaaaaaaaaaaaaaa", TipUnix: 1_700_000_000, Current: true, Locations: []scanner.BranchLocation{ {Name: "local", Exists: true, TipHash: "aaaaaaaaaaaaaaaa", TipUnix: 1_700_000_000, UniqueCount: 2}, - {Name: "origin", Exists: true, TipHash: "bbbbbbbbbbbbbbbb", TipUnix: 1_700_000_001, UniqueCount: 1, NewestUniqueUnix: 1_700_000_001, Incoming: 1, Outgoing: 2}, + {Name: "origin", Exists: true, TipHash: "bbbbbbbbbbbbbbbb", TipUnix: 1_700_000_001, UniqueCount: 1, Incoming: 1, Outgoing: 2}, }}, {Name: "wip/hidden", TipHash: "dddddddddddddddd", TipUnix: 1_700_000_003, Current: false, Locations: []scanner.BranchLocation{ {Name: "local", Exists: true, TipHash: "dddddddddddddddd", TipUnix: 1_700_000_003}, @@ -457,16 +455,15 @@ branches: m.repoList = []string{"/repo"} m.repositories.Set("/repo", scanner.RepoStatus{ Branches: scanner.BranchStatus{ - Branch: "aaa", - NewestLocation: "origin", + Branch: "aaa", Locations: []scanner.BranchLocation{ {Name: "local", Exists: true, TipHash: "aaaaaaaaaaaaaaaa", TipUnix: 1_700_000_000, UniqueCount: 2}, - {Name: "origin", Exists: true, TipHash: "bbbbbbbbbbbbbbbb", TipUnix: 1_700_000_001, UniqueCount: 1, NewestUniqueUnix: 1_700_000_001, Incoming: 1, Outgoing: 2}, + {Name: "origin", Exists: true, TipHash: "bbbbbbbbbbbbbbbb", TipUnix: 1_700_000_001, UniqueCount: 1, Incoming: 1, Outgoing: 2}, }, LocalBranches: []scanner.LocalBranchRef{ {Name: "aaa", TipHash: "aaaaaaaaaaaaaaaa", TipUnix: 1_700_000_000, Current: true, Locations: []scanner.BranchLocation{ {Name: "local", Exists: true, TipHash: "aaaaaaaaaaaaaaaa", TipUnix: 1_700_000_000, UniqueCount: 2}, - {Name: "origin", Exists: true, TipHash: "bbbbbbbbbbbbbbbb", TipUnix: 1_700_000_001, UniqueCount: 1, NewestUniqueUnix: 1_700_000_001, Incoming: 1, Outgoing: 2}, + {Name: "origin", Exists: true, TipHash: "bbbbbbbbbbbbbbbb", TipUnix: 1_700_000_001, UniqueCount: 1, Incoming: 1, Outgoing: 2}, }}, {Name: "main", TipHash: "cccccccccccccccc", TipUnix: 1_700_000_002, Current: false, Locations: []scanner.BranchLocation{ {Name: "local", Exists: true, TipHash: "cccccccccccccccc", TipUnix: 1_700_000_002}, @@ -513,17 +510,16 @@ branches: m.repoList = []string{"/repo"} m.repositories.Set("/repo", scanner.RepoStatus{ Branches: scanner.BranchStatus{ - Branch: "aaa", - NewestLocation: "origin", + Branch: "aaa", Locations: []scanner.BranchLocation{ {Name: "local", Exists: true, TipHash: "aaaaaaaaaaaaaaaa", TipUnix: 1_700_000_000, UniqueCount: 2}, - {Name: "origin", Exists: true, TipHash: "bbbbbbbbbbbbbbbb", TipUnix: 1_700_000_001, UniqueCount: 1, NewestUniqueUnix: 1_700_000_001, Incoming: 1, Outgoing: 2}, + {Name: "origin", Exists: true, TipHash: "bbbbbbbbbbbbbbbb", TipUnix: 1_700_000_001, UniqueCount: 1, Incoming: 1, Outgoing: 2}, {Name: "upstream", Exists: false}, }, LocalBranches: []scanner.LocalBranchRef{ {Name: "aaa", TipHash: "aaaaaaaaaaaaaaaa", TipUnix: 1_700_000_000, Current: true, Locations: []scanner.BranchLocation{ {Name: "local", Exists: true, TipHash: "aaaaaaaaaaaaaaaa", TipUnix: 1_700_000_000, UniqueCount: 2}, - {Name: "origin", Exists: true, TipHash: "bbbbbbbbbbbbbbbb", TipUnix: 1_700_000_001, UniqueCount: 1, NewestUniqueUnix: 1_700_000_001, Incoming: 1, Outgoing: 2}, + {Name: "origin", Exists: true, TipHash: "bbbbbbbbbbbbbbbb", TipUnix: 1_700_000_001, UniqueCount: 1, Incoming: 1, Outgoing: 2}, {Name: "upstream", Exists: false}, }}, {Name: "main", TipHash: "cccccccccccccccc", TipUnix: 1_700_000_002, Current: false, Locations: []scanner.BranchLocation{ diff --git a/ui/view_test.go b/ui/view_test.go index 93076a6..0b4a932 100644 --- a/ui/view_test.go +++ b/ui/view_test.go @@ -66,11 +66,10 @@ func TestBranchTableViewFitsInnerWidth(t *testing.T) { m.repoList = []string{"/repo"} m.repositories.Set("/repo", scanner.RepoStatus{ Branches: scanner.BranchStatus{ - Branch: "main", - NewestLocation: "origin", + Branch: "main", Locations: []scanner.BranchLocation{ {Name: "local", Exists: true, TipHash: "aaaaaaaaaaaaaaaa", TipUnix: 1_700_000_000, UniqueCount: 2}, - {Name: "origin", Exists: true, TipHash: "bbbbbbbbbbbbbbbb", TipUnix: 1_700_000_001, UniqueCount: 1, NewestUniqueUnix: 1_700_000_001, Incoming: 1, Outgoing: 2}, + {Name: "origin", Exists: true, TipHash: "bbbbbbbbbbbbbbbb", TipUnix: 1_700_000_001, UniqueCount: 1, Incoming: 1, Outgoing: 2}, {Name: "upstream", Exists: false}, }, },