refactor: apply code review improvements across scanner, ui, and report

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

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=

main.go

@@ -7,21 +7,41 @@ import (
 	"os"
 	"path/filepath"
 
-	"github.com/mitchellh/go-homedir"
 	"github.com/urfave/cli/v3"
 
 	"github.com/boyvinall/dirtygit/scanner"
 	"github.com/boyvinall/dirtygit/ui"
 )
 
 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)
 	}
 }

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"))
 		},
 	}
 }

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)
+	}
+}