diff --git a/go.mod b/go.mod index 474a14c..72c3e4e 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,6 @@ require ( github.com/charmbracelet/bubbletea v1.3.10 github.com/charmbracelet/lipgloss v1.1.0 github.com/go-git/go-git/v5 v5.18.0 - github.com/mitchellh/go-homedir v1.1.0 github.com/muesli/termenv v0.16.0 github.com/urfave/cli/v3 v3.8.0 golang.org/x/sync v0.20.0 diff --git a/go.sum b/go.sum index e2e6082..8bfdaec 100644 --- a/go.sum +++ b/go.sum @@ -81,8 +81,6 @@ github.com/mattn/go-localereader v0.0.1 h1:ygSAOl7ZXTx4RdPYinUpg6W99U8jWvWi9Ye2J github.com/mattn/go-localereader v0.0.1/go.mod h1:8fBrzywKY7BI3czFoHkuzRoWE9C+EiG4R1k4Cjx5p88= github.com/mattn/go-runewidth v0.0.23 h1:7ykA0T0jkPpzSvMS5i9uoNn2Xy3R383f9HDx3RybWcw= github.com/mattn/go-runewidth v0.0.23/go.mod h1:XBkDxAl56ILZc9knddidhrOlY5R/pDhgLpndooCuJAs= -github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y= -github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 h1:ZK8zHtRHOkbHy6Mmr5D264iyp3TiX5OmNcI5cIARiQI= github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6/go.mod h1:CJlz5H+gyd6CUWT45Oy4q24RdLyn7Md9Vj2/ldJBSIo= github.com/muesli/cancelreader v0.2.2 h1:3I4Kt4BQjOR54NavqnDogx/MIoWBFa0StPA8ELUXHmA= diff --git a/main.go b/main.go index 09dfc45..1e2e522 100644 --- a/main.go +++ b/main.go @@ -7,7 +7,6 @@ import ( "os" "path/filepath" - "github.com/mitchellh/go-homedir" "github.com/urfave/cli/v3" "github.com/boyvinall/dirtygit/scanner" @@ -15,13 +14,34 @@ import ( ) func getDefaultConfigPath() string { - home, err := homedir.Dir() + home, err := os.UserHomeDir() if err != nil { - fmt.Println("WARNING: Unable to determine home directory for default config path") + fmt.Fprintln(os.Stderr, "WARNING: Unable to determine home directory for default config path:", err) + return ".dirtygit.yml" } return filepath.Join(home, ".dirtygit.yml") } +// loadConfig parses the config file named by the --config flag and expands +// environment variables in ScanDirs. If positional args are provided they +// replace ScanDirs.Include. +func loadConfig(cmd *cli.Command, defaultConfig string) (*scanner.Config, error) { + config, err := scanner.ParseConfigFile(cmd.Root().String("config"), defaultConfig) + if err != nil { + return nil, err + } + if cmd.Args().Len() > 0 { + config.ScanDirs.Include = cmd.Args().Slice() + } + for i := range config.ScanDirs.Include { + config.ScanDirs.Include[i] = os.ExpandEnv(config.ScanDirs.Include[i]) + } + for i := range config.ScanDirs.Exclude { + config.ScanDirs.Exclude[i] = os.ExpandEnv(config.ScanDirs.Exclude[i]) + } + return config, nil +} + //go:embed .dirtygit.yml var defaultConfig string @@ -49,25 +69,15 @@ func main() { }, }, Action: func(ctx context.Context, cmd *cli.Command) error { - config, err := scanner.ParseConfigFile(cmd.String("config"), defaultConfig) + config, err := loadConfig(cmd, defaultConfig) if err != nil { return err } - if cmd.Args().Len() > 0 { - config.ScanDirs.Include = cmd.Args().Slice() - } - for i := range config.ScanDirs.Include { - config.ScanDirs.Include[i] = os.ExpandEnv(config.ScanDirs.Include[i]) - } - for i := range config.ScanDirs.Exclude { - config.ScanDirs.Exclude[i] = os.ExpandEnv(config.ScanDirs.Exclude[i]) - } - return ui.Run(config) }, } err := app.Run(context.Background(), os.Args) if err != nil { - fmt.Printf("%+v\n", err) + fmt.Fprintln(os.Stderr, err) } } diff --git a/report.go b/report.go index f78038d..6b7ee32 100644 --- a/report.go +++ b/report.go @@ -100,8 +100,8 @@ func buildReport(mgs *scanner.MultiGitStatus) report { return report{Repos: repos} } -func runReport(config *scanner.Config, outputFile string) error { - mgs, err := scanner.Scan(config) +func runReport(ctx context.Context, config *scanner.Config, outputFile string) error { + mgs, err := scanner.Scan(ctx, config) if err != nil { return err } @@ -134,7 +134,7 @@ func printReportSummary(r report) { fmt.Println(repo.Path) for _, b := range repo.Branches { if b.ShownInTUI { - fmt.Printf(" %s\n", b.GetDisplayName()) + fmt.Printf(" %s\n", b.DisplayName()) } } } @@ -152,20 +152,11 @@ func reportCommand() *cli.Command { }, }, Action: func(ctx context.Context, cmd *cli.Command) error { - config, err := scanner.ParseConfigFile(cmd.Root().String("config"), defaultConfig) + config, err := loadConfig(cmd, defaultConfig) if err != nil { return err } - if cmd.Args().Len() > 0 { - config.ScanDirs.Include = cmd.Args().Slice() - } - for i := range config.ScanDirs.Include { - config.ScanDirs.Include[i] = os.ExpandEnv(config.ScanDirs.Include[i]) - } - for i := range config.ScanDirs.Exclude { - config.ScanDirs.Exclude[i] = os.ExpandEnv(config.ScanDirs.Exclude[i]) - } - return runReport(config, cmd.String("output-file")) + return runReport(ctx, config, cmd.String("output-file")) }, } } diff --git a/report_test.go b/report_test.go new file mode 100644 index 0000000..c3ddfd3 --- /dev/null +++ b/report_test.go @@ -0,0 +1,167 @@ +package main + +import ( + "testing" + + "github.com/go-git/go-git/v5" + + "github.com/boyvinall/dirtygit/scanner" +) + +func TestBuildReportEmpty(t *testing.T) { + mgs := scanner.NewMultiGitStatus() + r := buildReport(mgs) + if len(r.Repos) != 0 { + t.Fatalf("expected 0 repos, got %d", len(r.Repos)) + } +} + +func TestBuildReportFileEntries(t *testing.T) { + mgs := scanner.NewMultiGitStatus() + mgs.AddResult("/repo/a", scanner.RepoStatus{ + Branch: "main", + Porcelain: scanner.PorcelainStatus{ + Entries: []scanner.PorcelainEntry{ + {Staging: 'M', Worktree: ' ', Path: "foo.go"}, + {Staging: ' ', Worktree: 'D', Path: "bar.go"}, + }, + }, + }) + + r := buildReport(mgs) + if len(r.Repos) != 1 { + t.Fatalf("expected 1 repo, got %d", len(r.Repos)) + } + repo := r.Repos[0] + if repo.Path != "/repo/a" { + t.Errorf("path = %q, want /repo/a", repo.Path) + } + if repo.IsClean { + t.Error("expected IsClean=false for dirty repo") + } + if repo.CurrentBranch != "main" { + t.Errorf("CurrentBranch = %q, want main", repo.CurrentBranch) + } + if len(repo.Files) != 2 { + t.Fatalf("files = %d, want 2", len(repo.Files)) + } + if repo.Files[0].Staging != string(git.StatusCode('M')) || repo.Files[0].Path != "foo.go" { + t.Errorf("unexpected file[0]: %+v", repo.Files[0]) + } + if repo.Files[1].Worktree != string(git.StatusCode('D')) || repo.Files[1].Path != "bar.go" { + t.Errorf("unexpected file[1]: %+v", repo.Files[1]) + } +} + +func TestBuildReportBranchShownInTUI(t *testing.T) { + local := scanner.BranchLocation{Name: "local", Exists: true, TipHash: "abc"} + remote := scanner.BranchLocation{Name: "origin", Exists: true, TipHash: "abc"} + + branches := []scanner.LocalBranchRef{ + {Name: "main", Current: true, Locations: []scanner.BranchLocation{local, remote}}, + {Name: "wip", Current: false, Locations: []scanner.BranchLocation{ + {Name: "local", Exists: true, TipHash: "def"}, + {Name: "origin", Exists: false}, + }}, + } + // FilteredBranches only contains "main" (wip is hidden by config) + filtered := []scanner.LocalBranchRef{branches[0]} + + mgs := scanner.NewMultiGitStatus() + mgs.AddResult("/repo/b", scanner.RepoStatus{ + Branch: "main", + Branches: branches, + FilteredBranches: filtered, + }) + + r := buildReport(mgs) + if len(r.Repos) != 1 { + t.Fatalf("expected 1 repo, got %d", len(r.Repos)) + } + if len(r.Repos[0].Branches) != 2 { + t.Fatalf("expected 2 branch entries, got %d", len(r.Repos[0].Branches)) + } + + byName := make(map[string]reportBranchEntry) + for _, b := range r.Repos[0].Branches { + byName[b.Name] = b + } + + if !byName["main"].ShownInTUI { + t.Error("main should be ShownInTUI=true (in FilteredBranches)") + } + if byName["wip"].ShownInTUI { + t.Error("wip should be ShownInTUI=false (not in FilteredBranches)") + } +} + +func TestBuildReportIsLocalOnly(t *testing.T) { + localOnly := scanner.LocalBranchRef{ + Name: "local-feature", + Locations: []scanner.BranchLocation{ + {Name: "local", Exists: true, TipHash: "111"}, + {Name: "origin", Exists: false}, + }, + } + withRemote := scanner.LocalBranchRef{ + Name: "main", + Locations: []scanner.BranchLocation{ + {Name: "local", Exists: true, TipHash: "222"}, + {Name: "origin", Exists: true, TipHash: "222"}, + }, + } + + mgs := scanner.NewMultiGitStatus() + mgs.AddResult("/repo/c", scanner.RepoStatus{ + Branch: "main", + Branches: []scanner.LocalBranchRef{localOnly, withRemote}, + FilteredBranches: []scanner.LocalBranchRef{localOnly, withRemote}, + }) + + r := buildReport(mgs) + byName := make(map[string]reportBranchEntry) + for _, b := range r.Repos[0].Branches { + byName[b.Name] = b + } + + if !byName["local-feature"].IsLocalOnly { + t.Error("local-feature should be IsLocalOnly=true") + } + if byName["main"].IsLocalOnly { + t.Error("main should be IsLocalOnly=false") + } +} + +func TestBuildReportSortedPaths(t *testing.T) { + mgs := scanner.NewMultiGitStatus() + mgs.AddResult("/z/repo", scanner.RepoStatus{Branch: "main"}) + mgs.AddResult("/a/repo", scanner.RepoStatus{Branch: "main"}) + mgs.AddResult("/m/repo", scanner.RepoStatus{Branch: "main"}) + + r := buildReport(mgs) + if len(r.Repos) != 3 { + t.Fatalf("expected 3 repos, got %d", len(r.Repos)) + } + if r.Repos[0].Path != "/a/repo" || r.Repos[1].Path != "/m/repo" || r.Repos[2].Path != "/z/repo" { + t.Errorf("repos not sorted: %v", []string{r.Repos[0].Path, r.Repos[1].Path, r.Repos[2].Path}) + } +} + +func TestBuildReportDetachedHead(t *testing.T) { + mgs := scanner.NewMultiGitStatus() + mgs.AddResult("/repo/d", scanner.RepoStatus{ + Branch: "abc1234", + Detached: true, + }) + + r := buildReport(mgs) + if len(r.Repos) != 1 { + t.Fatalf("expected 1 repo, got %d", len(r.Repos)) + } + if !r.Repos[0].Detached { + t.Error("expected Detached=true") + } + if r.Repos[0].CurrentBranch != "abc1234" { + t.Errorf("CurrentBranch = %q, want abc1234", r.Repos[0].CurrentBranch) + } +} diff --git a/scanner/branch_status.go b/scanner/branch_status.go index 6f52d37..b0678b0 100644 --- a/scanner/branch_status.go +++ b/scanner/branch_status.go @@ -1,6 +1,7 @@ package scanner import ( + "errors" "fmt" "os/exec" "sort" @@ -16,7 +17,8 @@ func haveMergeBase(dir, commitA, commitB string) (bool, error) { if err == nil { return true, nil } - if exitErr, ok := err.(*exec.ExitError); ok && exitErr.ExitCode() == 1 { + var exitErr *exec.ExitError + if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 { return false, nil } return false, fmt.Errorf("git merge-base: %w", err) @@ -71,15 +73,21 @@ func runGit(dir string, args ...string) (string, error) { } func currentBranch(dir string) (name string, detached bool, err error) { + // First try symbolic-ref to get the branch name name, err = runGit(dir, "symbolic-ref", "--quiet", "--short", "HEAD") if err == nil { return name, false, nil } - head, headErr := runGit(dir, "rev-parse", "--short", "HEAD") - if headErr != nil { - return "", false, headErr + + // If that fails, try rev-parse to see if we are in a detached HEAD state + // head, err := runGit(dir, "rev-parse", "--short", "HEAD") + head, err := runGit(dir, "rev-parse", "HEAD") + if err == nil { + return head, true, nil } - return head, true, nil + + // If both fail, return error + return "", false, err } func listRemotes(dir string) ([]string, error) { @@ -98,7 +106,13 @@ func listRemotes(dir string) ([]string, error) { func refTip(dir, ref string) (hash string, unix int64, exists bool, err error) { out, err := runGit(dir, "show", "-s", "--format=%H %ct", ref) if err != nil { - return "", 0, false, nil + // git show exits 128 for unknown refs; treat that as "doesn't exist" rather + // than an error so callers correctly see Exists=false for missing remotes. + var exitErr *exec.ExitError + if errors.As(err, &exitErr) && exitErr.ExitCode() == 128 { + return "", 0, false, nil + } + return "", 0, false, err } parts := strings.Fields(out) if len(parts) != 2 { @@ -226,10 +240,6 @@ func GitBranchStatus(dir string) (branch string, detached bool, locals []LocalBr if err != nil { return } - if detached { - locals, err = listLocalBranches(dir, branch, true) - return - } var remotes []string remotes, err = listRemotes(dir) @@ -237,11 +247,12 @@ func GitBranchStatus(dir string) (branch string, detached bool, locals []LocalBr return } - locals, err = listLocalBranches(dir, branch, false) + locals, err = listLocalBranches(dir, branch, detached) if err != nil { return } + // if !detached && len(locals) == 0 { if len(locals) == 0 { var locations []BranchLocation locations, err = computeBranchLocations(dir, branch, remotes) @@ -256,7 +267,6 @@ func GitBranchStatus(dir string) (branch string, detached bool, locals []LocalBr Current: true, Locations: locations, }} - return } for i := range locals { @@ -268,40 +278,27 @@ func GitBranchStatus(dir string) (branch string, detached bool, locals []LocalBr locals[i].Locations = locs } - // TODO: This fallback is likely unreachable: the only scenario (detached HEAD) is already handled above. - // This dead path adds computeBranchLocations calls and complexity for no gain. - var foundCurrent bool - for i := range locals { - if locals[i].Current { - foundCurrent = true - break - } - } - if !foundCurrent { - locs, err2 := computeBranchLocations(dir, branch, remotes) - if err2 != nil { - err = err2 - return - } - matched := false - for i := range locals { - if locals[i].Name == branch { - locals[i].Locations = locs - locals[i].Current = true - matched = true - break - } - } - if !matched { - tipHash, tipUnix := tipFromLocalBranchLocation(locs) - locals = append([]LocalBranchRef{{ - Name: branch, - TipHash: tipHash, - TipUnix: tipUnix, - Current: true, - Locations: locs, - }}, locals...) + if detached { + unix := int64(0) + if raw, e := runGit(dir, "log", "-1", "--format=%ct", "HEAD"); e == nil { + unix, _ = strconv.ParseInt(raw, 10, 64) } + locals = append([]LocalBranchRef{ + { + Name: "HEAD", + TipHash: branch, + TipUnix: unix, + Current: true, + Locations: []BranchLocation{ + { + Name: "local", + Exists: true, + TipHash: branch, + TipUnix: unix, + }, + }, + }, + }, locals...) // avoid mutating the original slice since it may be shared with the caller } return diff --git a/scanner/find.go b/scanner/find.go index 81f4196..64e9bec 100644 --- a/scanner/find.go +++ b/scanner/find.go @@ -124,10 +124,7 @@ func Walk(ctx context.Context, config *Config, results chan string, onRepoFound var eg errgroup.Group for i := range config.ScanDirs.Include { eg.Go(func() error { - err := walkone(ctx, config.ScanDirs.Include[i], config, results, onRepoFound) - if err == filepath.SkipDir { - cancel() - } else if err != nil { + if err := walkone(ctx, config.ScanDirs.Include[i], config, results, onRepoFound); err != nil { return err } return nil diff --git a/scanner/git_status.go b/scanner/git_status.go index fdf4549..1ed3df7 100644 --- a/scanner/git_status.go +++ b/scanner/git_status.go @@ -1,77 +1,73 @@ package scanner import ( - "bufio" + "bytes" "fmt" "io" "os/exec" - "strings" "github.com/go-git/go-git/v5" ) +// ParsePorcelainStatus parses NUL-delimited output from git status --porcelain -z. +// Rename/copy entries (staging code 'R' or 'C') consume an extra NUL-delimited +// token for the original path; all other entries are single-token records. func ParsePorcelainStatus(r io.Reader) (PorcelainStatus, error) { + data, err := io.ReadAll(r) + if err != nil { + return PorcelainStatus{}, err + } st := PorcelainStatus{} - lineScanner := bufio.NewScanner(r) - for lineScanner.Scan() { - entry, err := parsePorcelainLine(lineScanner.Text()) - if err != nil { - return PorcelainStatus{}, err + i := 0 + for i < len(data) { + j := bytes.IndexByte(data[i:], 0) + var token []byte + if j < 0 { + token = data[i:] + i = len(data) + } else { + token = data[i : i+j] + i += j + 1 + } + if len(token) == 0 { + continue + } + if len(token) < 4 || token[2] != ' ' { + return PorcelainStatus{}, fmt.Errorf("unable to parse status line: %q", token) + } + entry := PorcelainEntry{ + Staging: git.StatusCode(token[0]), + Worktree: git.StatusCode(token[1]), + Path: string(token[3:]), + } + if entry.Path == "" { + return PorcelainStatus{}, fmt.Errorf("unable to parse file path from status line: %q", token) + } + if entry.Staging == 'R' || entry.Staging == 'C' { + // Original path follows as the next NUL-delimited token. + k := bytes.IndexByte(data[i:], 0) + var orig []byte + if k < 0 { + orig = data[i:] + i = len(data) + } else { + orig = data[i : i+k] + i += k + 1 + } + entry.OriginalPath = string(orig) } st.Entries = append(st.Entries, entry) } - if err := lineScanner.Err(); err != nil { - return PorcelainStatus{}, err - } return st, nil } -func parsePorcelainLine(s string) (PorcelainEntry, error) { - if len(s) < 4 || s[2] != ' ' { - return PorcelainEntry{}, fmt.Errorf("unable to parse status line: %q", s) - } - - entry := PorcelainEntry{ - Staging: git.StatusCode(s[0]), - Worktree: git.StatusCode(s[1]), - } - - payload := s[3:] - if oldPath, newPath, ok := strings.Cut(payload, " -> "); ok { - entry.OriginalPath = oldPath - entry.Path = newPath - } else { - entry.Path = payload - } - - if entry.Path == "" { - return PorcelainEntry{}, fmt.Errorf("unable to parse file path from status line: %q", s) - } - - return entry, nil -} - -// GitStatus invokes git executable to determine the git status for a directory. +// GitStatus invokes git to return porcelain status for a directory. func GitStatus(d string) (PorcelainStatus, error) { - cmd := exec.Command("git", "status", "--porcelain") + cmd := exec.Command("git", "status", "--porcelain", "-z") cmd.Dir = d - stdout, err := cmd.StdoutPipe() - if err != nil { - return PorcelainStatus{}, fmt.Errorf("%s: %w", d, err) - } - - if err := cmd.Start(); err != nil { - return PorcelainStatus{}, fmt.Errorf("%s: %w", d, err) - } - - st, err := ParsePorcelainStatus(stdout) + out, err := cmd.Output() if err != nil { return PorcelainStatus{}, fmt.Errorf("%s: %w", d, err) } - - if err := cmd.Wait(); err != nil { - return PorcelainStatus{}, fmt.Errorf("%s: %w", d, err) - } - - return st, nil + return ParsePorcelainStatus(bytes.NewReader(out)) } diff --git a/scanner/multi_git_status.go b/scanner/multi_git_status.go index 0b714ad..6e7f0dc 100644 --- a/scanner/multi_git_status.go +++ b/scanner/multi_git_status.go @@ -7,9 +7,9 @@ import ( // MultiGitStatus holds per-repository scan results. The zero value is usable: // reads treat a nil receiver as empty; the first AddResult or Set allocates -// the inner map. Do not copy a non-zero MultiGitStatus (it contains a sync.Mutex). +// the inner map. Do not copy a non-zero MultiGitStatus (it contains a sync.RWMutex). type MultiGitStatus struct { - mu sync.Mutex + mu sync.RWMutex m map[string]RepoStatus } @@ -46,8 +46,8 @@ func (m *MultiGitStatus) Get(path string) (RepoStatus, bool) { if m == nil { return RepoStatus{}, false } - m.mu.Lock() - defer m.mu.Unlock() + m.mu.RLock() + defer m.mu.RUnlock() rs, ok := m.m[path] return rs, ok } @@ -57,8 +57,8 @@ func (m *MultiGitStatus) Len() int { if m == nil { return 0 } - m.mu.Lock() - defer m.mu.Unlock() + m.mu.RLock() + defer m.mu.RUnlock() return len(m.m) } @@ -67,12 +67,12 @@ func (m *MultiGitStatus) SortedRepoPaths() []string { if m == nil { return nil } - m.mu.Lock() + m.mu.RLock() paths := make([]string, 0, len(m.m)) for r := range m.m { paths = append(paths, r) } - m.mu.Unlock() + m.mu.RUnlock() sort.Strings(paths) return paths } diff --git a/scanner/scan.go b/scanner/scan.go index 9418a8d..e3e3cba 100644 --- a/scanner/scan.go +++ b/scanner/scan.go @@ -2,7 +2,7 @@ package scanner import ( "context" - "log" + "log/slog" "sync/atomic" "time" @@ -16,14 +16,13 @@ func reportProgress(onProgress func(ScanProgress), p ScanProgress) { } // Scan finds all "dirty" git repositories specified by config. -func Scan(config *Config) (*MultiGitStatus, error) { - return ScanWithProgress(config, nil) +func Scan(ctx context.Context, config *Config) (*MultiGitStatus, error) { + return ScanWithProgress(ctx, config, nil) } // ScanWithProgress runs the same scan as [Scan] and invokes onProgress from concurrent // discovery and the status loop. Callbacks should be non-blocking (e.g. small channel send). -func ScanWithProgress(config *Config, onProgress func(ScanProgress)) (*MultiGitStatus, error) { - ctx := context.Background() +func ScanWithProgress(ctx context.Context, config *Config, onProgress func(ScanProgress)) (*MultiGitStatus, error) { repositories := make(chan string, 1000) var found, checked atomic.Uint64 @@ -52,6 +51,7 @@ func ScanWithProgress(config *Config, onProgress func(ScanProgress)) (*MultiGitS results := NewMultiGitStatus() var eg errgroup.Group + ex := NewExcluder(config.GitIgnore.FileGlob, config.GitIgnore.DirGlob) for d := range repositories { eg.Go(func() error { @@ -63,7 +63,7 @@ func ScanWithProgress(config *Config, onProgress func(ScanProgress)) (*MultiGitS CurrentPath: d, }) - rs, include, err := StatusForRepo(config, d) + rs, include, err := statusForRepoWithExcluder(config, ex, d) if err != nil { return err } @@ -94,11 +94,14 @@ func ScanWithProgress(config *Config, onProgress func(ScanProgress)) (*MultiGitS // StatusForRepo returns fresh status for a single repository directory using the // same porcelain filtering and branch metadata as [ScanWithProgress]. The bool // is whether this repo should appear in the dirty list (!clean or remote mismatch). -// If ex is nil, a new excluder is built from config (for single-repo refresh paths). -// afterPorcelain, if non-nil, is invoked after porcelain is resolved and before -// 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) + return statusForRepoWithExcluder(config, ex, dir) +} + +// statusForRepoWithExcluder is the shared implementation used by [StatusForRepo] +// and [ScanWithProgress] (which builds the excluder once for all repos). +func statusForRepoWithExcluder(config *Config, ex Excluder, dir string) (RepoStatus, bool, error) { porcelain, err := GitStatus(dir) if err != nil { return RepoStatus{}, false, err @@ -106,7 +109,10 @@ func StatusForRepo(config *Config, dir string) (RepoStatus, bool, error) { porcelain = ex.FilterPorcelainStatus(porcelain) branch, detached, branches, err := GitBranchStatus(dir) if err != nil { - log.Printf("branch status scan failed for %s: %v", dir, err) + // Best-effort: a single repo's branch metadata failure should not abort the + // whole scan. The repo will still appear if it has uncommitted working-tree + // changes; it will just show no branch divergence information. + slog.Warn("branch status scan failed", "dir", dir, "err", err) } rs := RepoStatus{ diff --git a/scanner/scan_integration_test.go b/scanner/scan_integration_test.go index 990f5c1..9e75cce 100644 --- a/scanner/scan_integration_test.go +++ b/scanner/scan_integration_test.go @@ -1,6 +1,7 @@ package scanner import ( + "context" "os" "path/filepath" "sync" @@ -26,7 +27,7 @@ func TestScanFindsDirtyRepos(t *testing.T) { cfg := &Config{} cfg.ScanDirs.Include = []string{root} - mgs, err := Scan(cfg) + mgs, err := Scan(context.Background(), cfg) if err != nil { t.Fatalf("Scan: %v", err) } @@ -46,7 +47,7 @@ func TestScanEmptyTree(t *testing.T) { cfg := &Config{} cfg.ScanDirs.Include = []string{root} - mgs, err := Scan(cfg) + mgs, err := Scan(context.Background(), cfg) if err != nil { t.Fatalf("Scan: %v", err) } @@ -82,7 +83,7 @@ func TestScanWithProgressReportsProgress(t *testing.T) { } } - mgs, err := ScanWithProgress(cfg, callback) + mgs, err := ScanWithProgress(context.Background(), cfg, callback) if err != nil { t.Fatalf("ScanWithProgress: %v", err) } diff --git a/scanner/scan_test.go b/scanner/scan_test.go index abf35d7..06756f5 100644 --- a/scanner/scan_test.go +++ b/scanner/scan_test.go @@ -9,11 +9,9 @@ import ( ) func TestParsePorcelainStatus(t *testing.T) { - input := strings.Join([]string{ - " M scanner/scan.go", - "R old/name.go -> new/name.go", - "?? scanner/scan_test.go", - }, "\n") + // NUL-delimited format (git status --porcelain -z): + // renames: first token is "R new-path", second token is "old-path" + input := " M scanner/scan.go\x00R new/name.go\x00old/name.go\x00?? scanner/scan_test.go\x00" st, err := ParsePorcelainStatus(strings.NewReader(input)) if err != nil { @@ -38,7 +36,7 @@ func TestParsePorcelainStatus(t *testing.T) { } func TestParsePorcelainStatusRejectsMalformedLine(t *testing.T) { - _, err := ParsePorcelainStatus(strings.NewReader("bad")) + _, err := ParsePorcelainStatus(strings.NewReader("bad\x00")) if err == nil { t.Fatal("ParsePorcelainStatus() expected error for malformed line") } diff --git a/scanner/types.go b/scanner/types.go index e3d50ca..dd52d7c 100644 --- a/scanner/types.go +++ b/scanner/types.go @@ -29,9 +29,10 @@ type RepoStatus struct { Branches []LocalBranchRef // FilteredBranches is the subset of Branches that pass config filtering - // (e.g. hide local-only branches matching config patterns). It is what the - // UI branch pane shows and is used for mismatch detection and unpushed - // change status. It is always a subset of Branches and has the same order. + // (e.g. hide local-only branches matching config patterns). It drives the + // UI branch pane display and report inclusion decisions. HasUnpushedChanges + // iterates Branches directly with inline config filtering, so it does not + // use FilteredBranches. It is always a subset of Branches with the same order. FilteredBranches []LocalBranchRef } @@ -52,11 +53,11 @@ type LocalBranchRef struct { Locations []BranchLocation } -func (lbr *LocalBranchRef) GetDisplayName() string { - if lbr.Current { - return "*" + lbr.Name +func (lb *LocalBranchRef) DisplayName() string { + if lb.Current { + return "*" + lb.Name } - return lbr.Name + return lb.Name } // BranchLocation is one side of a local branch compared to same-named remotes: @@ -117,7 +118,7 @@ func (lb LocalBranchRef) IsLocalOnly() bool { // checked-out branch (the [LocalBranchRef] with Current: true). Returns nil when // detached or when no current row exists. func (rs *RepoStatus) CurrentBranchLocations() []BranchLocation { - if rs == nil || rs.Detached { + if rs == nil { return nil } for i := range rs.Branches { @@ -186,7 +187,7 @@ func (lb *LocalBranchRef) HasUnpushedChanges() bool { // [Config.ShouldHideLocalOnlyBranch] matches. // The checked-out branch is never removed so HEAD remote comparison stays available. func (rs *RepoStatus) Filter(c *Config) []LocalBranchRef { - out := make([]LocalBranchRef, 0) + out := make([]LocalBranchRef, 0, len(rs.Branches)) for _, lb := range rs.Branches { if lb.Current { out = append(out, lb) @@ -207,14 +208,12 @@ func (rs *RepoStatus) Filter(c *Config) []LocalBranchRef { return out } -// LocalRemoteMismatchReasons returns a short line explaining why -func (rs *RepoStatus) LocalRemoteMismatchReasons() []string { - if rs.Detached { - return nil - } +// LocalRemoteMismatchReason returns a human-readable explanation of why the +// current branch diverges from its remotes, and whether a mismatch was found. +func (rs *RepoStatus) LocalRemoteMismatchReason() (reason string, ok bool) { locs := rs.CurrentBranchLocations() if len(locs) == 0 { - return nil + return "", false } var local *BranchLocation for i := range locs { @@ -224,7 +223,7 @@ func (rs *RepoStatus) LocalRemoteMismatchReasons() []string { } } if local == nil || !local.Exists { - return nil + return "", false } branchName := rs.Branch if branchName == "" { @@ -238,31 +237,31 @@ func (rs *RepoStatus) LocalRemoteMismatchReasons() []string { } hasRemote = true if !loc.Exists { - return []string{fmt.Sprintf("On remote %q, there is no same-named branch to compare with your local %q (refs/remotes/…/… missing).", loc.Name, branchName)} + return fmt.Sprintf("On remote %q, there is no same-named branch to compare with your local %q (refs/remotes/…/… missing).", loc.Name, branchName), true } if loc.TipHash != local.TipHash { if !loc.HistoriesUnrelated && loc.Incoming > 0 && loc.Outgoing == 0 { continue } if loc.HistoriesUnrelated { - return []string{fmt.Sprintf("On remote %q, %q has unrelated history to your local tip (no merge base).", loc.Name, branchName)} + return fmt.Sprintf("On remote %q, %q has unrelated history to your local tip (no merge base).", loc.Name, branchName), true } if loc.Incoming > 0 && loc.Outgoing > 0 { - return []string{fmt.Sprintf("On remote %q, %q diverged from the local tip (incoming +%d, outgoing %d).", loc.Name, branchName, loc.Incoming, loc.Outgoing)} + return fmt.Sprintf("On remote %q, %q diverged from the local tip (incoming +%d, outgoing %d).", loc.Name, branchName, loc.Incoming, loc.Outgoing), true } if loc.Outgoing > 0 { - return []string{fmt.Sprintf("On remote %q, your local %q is ahead: %d commit(s) not on that remote (tips differ or unpushed).", loc.Name, branchName, loc.Outgoing)} + return fmt.Sprintf("On remote %q, your local %q is ahead: %d commit(s) not on that remote (tips differ or unpushed).", loc.Name, branchName, loc.Outgoing), true } if loc.Incoming > 0 { - return []string{fmt.Sprintf("On remote %q, the same-named ref tip differs from your local %q (and it is not the \"only behind the remote\" case).", loc.Name, branchName)} + return fmt.Sprintf("On remote %q, the same-named ref tip differs from your local %q (and it is not the \"only behind the remote\" case).", loc.Name, branchName), true } - return []string{fmt.Sprintf("On remote %q, the same-named ref tip differs from your local branch %q (not the \"only behind the remote\" case).", loc.Name, branchName)} + return fmt.Sprintf("On remote %q, the same-named ref tip differs from your local branch %q (not the \"only behind the remote\" case).", loc.Name, branchName), true } } if hasRemote && local.UniqueCount > 0 { - return []string{fmt.Sprintf("Branch %q: %d commit(s) on local are not on any of the other refs this scan compared (e.g. same-named remotes) — see the Branches pane for detail.", branchName, local.UniqueCount)} + return fmt.Sprintf("Branch %q: %d commit(s) on local are not on any of the other refs this scan compared (e.g. same-named remotes) — see the Branches pane for detail.", branchName, local.UniqueCount), true } - return nil + return "", false } // PorcelainEntry is one parsed line of git status --porcelain (short format). diff --git a/ui/app.go b/ui/app.go index d4aae02..5b45a70 100644 --- a/ui/app.go +++ b/ui/app.go @@ -1,6 +1,7 @@ package ui import ( + "context" "log" "time" @@ -44,10 +45,8 @@ func Run(config *scanner.Config) error { log.SetOutput(m.logBuf) p := tea.NewProgram(m, tea.WithAltScreen(), tea.WithMouseCellMotion()) - if _, err := p.Run(); err != nil { - return err - } - return nil + _, err := p.Run() + return err } // Init starts the initial repository scan when the app launches. @@ -72,7 +71,7 @@ func (m *model) beginScan() tea.Cmd { progCh := m.scanProgressCh m.scanSpinner = newScanSpinner() go func() { - mgs, err := scanner.ScanWithProgress(m.config, func(p scanner.ScanProgress) { + mgs, err := scanner.ScanWithProgress(context.Background(), m.config, func(p scanner.ScanProgress) { select { case progCh <- p: default: diff --git a/ui/model.go b/ui/model.go index 0f79985..d8f2e1a 100644 --- a/ui/model.go +++ b/ui/model.go @@ -137,8 +137,8 @@ type model struct { layoutBranchBody int layoutLogBody int - // layoutBranchesOuter is the framed Diff (right) column outer width in the middle row (0 = default pct). - layoutBranchesOuter int + // layoutDiffColumnOuter is the framed Diff (right) column outer width in the middle row (0 = default pct). + layoutDiffColumnOuter int resizeDrag resizeSplit } diff --git a/ui/mouse_line_select_test.go b/ui/mouse_line_select_test.go index c789cae..46ca491 100644 --- a/ui/mouse_line_select_test.go +++ b/ui/mouse_line_select_test.go @@ -13,6 +13,8 @@ import ( // TestBubblesTableInternalFields guards against upstream field renames that // would cause bubblesTableSlice and bubblesTableViewportYOffset to silently // return zeros. Field names are verified against bubbles v1.0.0 (go.mod). +// It also calls the functions on a populated table to confirm non-zero values +// are returned — a silent zero return on rename would be caught here too. func TestBubblesTableInternalFields(t *testing.T) { typ := reflect.TypeOf(table.Model{}) for _, name := range []string{"start", "end", "viewport"} { @@ -24,6 +26,28 @@ func TestBubblesTableInternalFields(t *testing.T) { if _, ok := vpField.Type.FieldByName("YOffset"); !ok { t.Error("bubbles viewport.Model missing field \"YOffset\" — update bubblesTableViewportYOffset in mouse_line_select.go") } + + // Confirm non-zero values are returned for a populated, scrolled table so + // a silent zero-return from a field rename is caught at runtime, not just + // at field-existence time above. + rows := make([]table.Row, 20) + for i := range rows { + rows[i] = table.Row{"x"} + } + tbl := table.New( + table.WithColumns([]table.Column{{Title: "C", Width: 10}}), + table.WithRows(rows), + table.WithHeight(5), + table.WithFocused(true), + ) + // Move cursor well into the list so the start/end window is non-zero. + for i := 0; i < 15; i++ { + tbl.MoveDown(1) + } + start, end := bubblesTableSlice(tbl) + if start == 0 && end == 0 { + t.Error("bubblesTableSlice returned (0,0) on a scrolled table — field names may have changed") + } } func TestMouseRepoLineSelect(t *testing.T) { diff --git a/ui/mouse_resize.go b/ui/mouse_resize.go index 51593ed..2eaa799 100644 --- a/ui/mouse_resize.go +++ b/ui/mouse_resize.go @@ -201,7 +201,7 @@ func (m *model) applyResizeDrag(x, y int) { if !ok { return } - m.layoutBranchesOuter = rightOuter + m.layoutDiffColumnOuter = rightOuter default: return diff --git a/ui/status_layout.go b/ui/status_layout.go index 52e8855..7476bcf 100644 --- a/ui/status_layout.go +++ b/ui/status_layout.go @@ -146,8 +146,8 @@ func (m *model) innerWidth() int { // middleRowColumnOuterWidths splits the middle row: left (Status+Branches stack) vs right (Diff). func (m *model) middleRowColumnOuterWidths(total int) (leftOuter, rightOuter int) { - if m.layoutBranchesOuter > 0 { - right := m.layoutBranchesOuter + if m.layoutDiffColumnOuter > 0 { + right := m.layoutDiffColumnOuter right = max(layoutMinStatusBranchesColumn, min(right, total-layoutMinStatusBranchesColumn)) return total - right, right } @@ -380,26 +380,6 @@ func (m *model) refreshBranchContent(totalWidth int) { return } - // TODO - // branch := st.Branches - // if branch.Detached { - // locals := append([]scanner.LocalBranchRef(nil), branch.LocalBranches...) - // sortLocalBranchesByTipNewestFirst(locals) - // rows := make([]table.Row, 0, 1+len(locals)) - // rows = append(rows, table.Row{"(detached HEAD)", shortHash(branch.Branch), "-", "-"}) - // for _, lb := range locals { - // rows = append(rows, table.Row{ - // lb.Name, - // shortHash(lb.TipHash), - // relativeTime(lb.TipUnix), - // "-", - // }) - // } - // m.branchTable.SetRows(rows) - // m.branchTable.SetHeight(max(4, len(rows)+1)) - // return - // } - // create a deep copy and sort it locals := append([]scanner.LocalBranchRef(nil), st.FilteredBranches...) sortLocalBranchesByTipNewestFirst(locals) @@ -407,11 +387,10 @@ func (m *model) refreshBranchContent(totalWidth int) { // show only the dirty branches in the UI rows := make([]table.Row, 0, len(locals)) for _, lb := range locals { - // TODO: check detached head remote := branchRemoteSummaryFromLocations(lb.Locations) rows = append(rows, table.Row{ - lb.GetDisplayName(), + lb.DisplayName(), shortHash(lb.TipHash), relativeTime(lb.TipUnix), remote, diff --git a/ui/update.go b/ui/update.go index 2682893..d61a117 100644 --- a/ui/update.go +++ b/ui/update.go @@ -28,11 +28,11 @@ func (m *model) handleWindowSize(msg tea.WindowSizeMsg) (tea.Model, tea.Cmd) { m.height = msg.Height if m.logVP.Height == 0 { inner := max(layoutMinInnerContentWidth, msg.Width-4) - m.logVP = viewport.New(inner, layoutMinInnerContentWidth) + m.logVP = viewport.New(inner, layoutDefaultTableViewRows) } if m.diffVP.Height == 0 { inner := max(layoutMinInnerContentWidth, msg.Width-4) - m.diffVP = viewport.New(inner, layoutMinInnerContentWidth) + m.diffVP = viewport.New(inner, layoutDefaultTableViewRows) } m.syncViewports() return m, nil @@ -99,6 +99,17 @@ func (m *model) handleHelpOverlayKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { } } +// handleConfirmNavKey handles the shared n/y/left/h/right/l navigation inside +// any yes/no confirmation dialog. +func (m *model) handleConfirmNavKey(msg tea.KeyMsg) { + switch msg.String() { + case "n", "left", "h": + m.deleteConfirmYes = false + case "y", "right", "l": + m.deleteConfirmYes = true + } +} + // handleDeleteRepoConfirmKey processes keys while the delete-directory confirmation is open. func (m *model) handleDeleteRepoConfirmKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { switch msg.String() { @@ -107,18 +118,6 @@ func (m *model) handleDeleteRepoConfirmKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) case "esc": m.deleteRepoConfirmOpen = false return m, nil - case "n": - m.deleteConfirmYes = false - return m, nil - case "y": - m.deleteConfirmYes = true - return m, nil - case "left", "h": - m.deleteConfirmYes = false - return m, nil - case "right", "l": - m.deleteConfirmYes = true - return m, nil case "enter": m.deleteRepoConfirmOpen = false if !m.deleteConfirmYes { @@ -127,6 +126,7 @@ func (m *model) handleDeleteRepoConfirmKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) m.deleteSelectedRepoFromDisk() return m, nil default: + m.handleConfirmNavKey(msg) return m, nil } } @@ -140,18 +140,6 @@ func (m *model) handleCheckoutStatusFileConfirmKey(msg tea.KeyMsg) (tea.Model, t m.checkoutStatusFileConfirmOpen = false m.checkoutStatusFilePendingRel = "" return m, nil - case "n": - m.deleteConfirmYes = false - return m, nil - case "y": - m.deleteConfirmYes = true - return m, nil - case "left", "h": - m.deleteConfirmYes = false - return m, nil - case "right", "l": - m.deleteConfirmYes = true - return m, nil case "enter": m.checkoutStatusFileConfirmOpen = false pending := m.checkoutStatusFilePendingRel @@ -169,6 +157,7 @@ func (m *model) handleCheckoutStatusFileConfirmKey(msg tea.KeyMsg) (tea.Model, t m.syncViewports() return m, nil default: + m.handleConfirmNavKey(msg) return m, nil } } @@ -182,18 +171,6 @@ func (m *model) handleDeleteStatusFileConfirmKey(msg tea.KeyMsg) (tea.Model, tea m.deleteStatusFileConfirmOpen = false m.deleteStatusFilePendingRel = "" return m, nil - case "n": - m.deleteConfirmYes = false - return m, nil - case "y": - m.deleteConfirmYes = true - return m, nil - case "left", "h": - m.deleteConfirmYes = false - return m, nil - case "right", "l": - m.deleteConfirmYes = true - return m, nil case "enter": m.deleteStatusFileConfirmOpen = false pending := m.deleteStatusFilePendingRel @@ -204,6 +181,7 @@ func (m *model) handleDeleteStatusFileConfirmKey(msg tea.KeyMsg) (tea.Model, tea m.deletePendingStatusFileFromDisk(pending) return m, nil default: + m.handleConfirmNavKey(msg) return m, nil } }