diff --git a/go.mod b/go.mod index 36aa2d4..4a88e33 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/pkg/errors v0.9.1 github.com/spf13/cobra v1.0.0 github.com/spf13/viper v1.7.0 - github.com/stretchr/testify v1.4.0 // indirect + github.com/stretchr/testify v1.4.0 github.com/xlab/treeprint v1.0.0 golang.org/x/sys v0.0.0-20200302150141-5c8b2ff67527 // indirect gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect diff --git a/pkg/git/finder.go b/pkg/git/finder.go index d8eb9a0..d997988 100644 --- a/pkg/git/finder.go +++ b/pkg/git/finder.go @@ -16,8 +16,8 @@ import ( // It's handled by ErrorsCallback to tell the WalkCallback to skip this dir. var errSkipNode = errors.New(".git directory found, skipping this node") -// errDirectoryAccess indicates a directory doesn't exists or can't be accessed -var errDirectoryAccess = errors.New("directory doesn't exist or can't be accessed") +var errDirNoAccess = errors.New("directory can't be accessed") +var errDirNotExist = errors.New("directory doesn't exist") // Exists returns true if a directory exists. If it doesn't or the directory can't be accessed it returns an error. func Exists(path string) (bool, error) { @@ -29,19 +29,18 @@ func Exists(path string) (bool, error) { if err != nil { if os.IsNotExist(err) { - return false, errDirectoryAccess + return false, errors.Wrapf(errDirNotExist, "can't access %s", path) } } // Directory exists but can't be accessed - return true, errDirectoryAccess + return true, errors.Wrapf(errDirNoAccess, "can't access %s", path) } // RepoFinder finds git repositories inside a given path. type RepoFinder struct { - root string - repos []*Repo - errors []error + root string + repos []*Repo } // NewRepoFinder returns a RepoFinder pointed at given root path. @@ -128,15 +127,15 @@ func (f *RepoFinder) walkCb(path string, ent *godirwalk.Dirent) error { } // addIfOk adds the found repo to the repos slice if it can be opened. -// If repo path can't be accessed it will add an error to the errors slice. func (f *RepoFinder) addIfOk(path string) { - repo, err := Open(strings.TrimSuffix(path, dotgit)) - if err != nil { - f.errors = append(f.errors, err) - return - } + // TODO: is the case below really correct? What if there's a race condition and the dir becomes unaccessible between finding it and opening? - f.repos = append(f.repos, repo) + // Open() should never return an error here. If a finder found a .git inside this dir, it means it could open and access it. + // If the dir was unaccessible, then it would have been skipped by the check in errorCb(). + repo, err := Open(strings.TrimSuffix(path, dotgit)) + if err == nil { + f.repos = append(f.repos, repo) + } } func (f *RepoFinder) errorCb(_ string, err error) godirwalk.ErrorAction { diff --git a/pkg/git/finder_test.go b/pkg/git/finder_test.go index 24205ae..1ab2078 100644 --- a/pkg/git/finder_test.go +++ b/pkg/git/finder_test.go @@ -1,8 +1,11 @@ package git import ( + "errors" "git-get/pkg/git/test" "testing" + + "github.com/stretchr/testify/assert" ) func TestFinder(t *testing.T) { @@ -15,18 +18,15 @@ func TestFinder(t *testing.T) { name: "no repos", reposMaker: makeNoRepos, want: 0, - }, - { + }, { name: "single repos", reposMaker: makeSingleRepo, want: 1, - }, - { + }, { name: "single nested repo", reposMaker: makeNestedRepo, want: 1, - }, - { + }, { name: "multiple nested repo", reposMaker: makeMultipleNestedRepos, want: 2, @@ -40,9 +40,38 @@ func TestFinder(t *testing.T) { finder := NewRepoFinder(root) finder.Find() - if len(finder.repos) != test.want { - t.Errorf("expected %d; got %d", test.want, len(finder.repos)) - } + assert.Len(t, finder.repos, test.want) + }) + } +} + +// TODO: this test will only work on Linux +func TestExists(t *testing.T) { + tests := []struct { + name string + path string + want error + }{ + { + name: "dir does not exist", + path: "/this/directory/does/not/exist", + want: errDirNotExist, + }, { + name: "dir cant be accessed", + path: "/root/some/directory", + want: errDirNoAccess, + }, { + name: "dir exists", + path: "/tmp/", + want: nil, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + _, err := Exists(test.path) + + assert.True(t, errors.Is(err, test.want)) }) } } diff --git a/pkg/git/repo_test.go b/pkg/git/repo_test.go index 38f65c3..24c4e45 100644 --- a/pkg/git/repo_test.go +++ b/pkg/git/repo_test.go @@ -6,14 +6,6 @@ import ( "testing" ) -func TestOpen(t *testing.T) { - _, err := Open("/paththatdoesnotexist/repo") - - if err != errDirectoryAccess { - t.Errorf("Opening a repo in non existing path should throw an error") - } -} - func TestUncommitted(t *testing.T) { tests := []struct { name string diff --git a/pkg/list.go b/pkg/list.go index 7060907..3550026 100644 --- a/pkg/list.go +++ b/pkg/list.go @@ -30,11 +30,11 @@ func List(c *ListCfg) error { switch c.Output { case cfg.OutFlat: - fmt.Println(print.NewFlatPrinter().Print(printables)) + fmt.Print(print.NewFlatPrinter().Print(printables)) case cfg.OutTree: - fmt.Println(print.NewTreePrinter().Print(c.Root, printables)) + fmt.Print(print.NewTreePrinter().Print(c.Root, printables)) case cfg.OutDump: - fmt.Println(print.NewDumpPrinter().Print(printables)) + fmt.Print(print.NewDumpPrinter().Print(printables)) default: return fmt.Errorf("invalid --out flag; allowed values: [%s]", strings.Join(cfg.AllowedOut, ", ")) } diff --git a/pkg/print/dump.go b/pkg/print/dump.go index 63ca454..293b3f3 100644 --- a/pkg/print/dump.go +++ b/pkg/print/dump.go @@ -17,7 +17,7 @@ func NewDumpPrinter() *DumpPrinter { func (p *DumpPrinter) Print(repos []Printable) string { var str strings.Builder - for i, r := range repos { + for _, r := range repos { str.WriteString(r.Remote()) // TODO: if head is detached maybe we should get the revision it points to in case it's a tag @@ -25,9 +25,7 @@ func (p *DumpPrinter) Print(repos []Printable) string { str.WriteString(" " + current) } - if i < len(repos)-1 { - str.WriteString("\n") - } + str.WriteString("\n") } return str.String() diff --git a/pkg/print/flat.go b/pkg/print/flat.go index ca1fb66..4bafb56 100644 --- a/pkg/print/flat.go +++ b/pkg/print/flat.go @@ -18,8 +18,14 @@ func NewFlatPrinter() *FlatPrinter { func (p *FlatPrinter) Print(repos []Printable) string { var str strings.Builder - for i, r := range repos { + for _, r := range repos { str.WriteString(strings.TrimSuffix(r.Path(), string(os.PathSeparator))) + + if len(r.Errors()) > 0 { + str.WriteString(" " + red("error") + "\n") + continue + } + str.WriteString(" " + blue(r.Current())) current := r.BranchStatus(r.Current()) @@ -45,10 +51,8 @@ func (p *FlatPrinter) Print(repos []Printable) string { str.WriteString(fmt.Sprintf("\n%s %s %s", indent, blue(branch), yellow(status))) } - if i < len(repos)-1 { - str.WriteString("\n") - } + str.WriteString("\n") } - return str.String() + return str.String() + Errors(repos) } diff --git a/pkg/print/print.go b/pkg/print/print.go index 18ee341..58d673a 100644 --- a/pkg/print/print.go +++ b/pkg/print/print.go @@ -1,6 +1,9 @@ package print -import "fmt" +import ( + "fmt" + "strings" +) const ( head = "HEAD" @@ -17,6 +20,28 @@ type Printable interface { Errors() []string } +// Errors returns a printable list of errors from the slice of Printables or an empty string if there are no errors. +// It's meant to be appended at the end of Print() result. +func Errors(repos []Printable) string { + errors := []string{} + + for _, repo := range repos { + for _, err := range repo.Errors() { + errors = append(errors, err) + } + } + + if len(errors) == 0 { + return "" + } + + var str strings.Builder + str.WriteString(red("\nOops, errors happened when loading repository status:\n")) + str.WriteString(strings.Join(errors, "\n")) + + return str.String() +} + // TODO: not sure if this works on windows. See https://github.com/mattn/go-colorable func red(str string) string { return fmt.Sprintf("\033[1;31m%s\033[0m", str) diff --git a/pkg/print/tree.go b/pkg/print/tree.go index 0526bb7..fc327a3 100644 --- a/pkg/print/tree.go +++ b/pkg/print/tree.go @@ -29,7 +29,7 @@ func (p *TreePrinter) Print(root string, repos []Printable) string { p.printTree(tree, tp) - return tp.String() + return tp.String() + Errors(repos) } // Node represents a path fragment in repos tree. @@ -114,34 +114,11 @@ func buildTree(root string, repos []Printable) *Node { return tree } +// printTree renders the repo tree by recursively traversing the tree nodes. +// If a node doesn't have any children, it's a leaf node containing the repo status. func (p *TreePrinter) printTree(node *Node, tp treeprint.Tree) { if node.children == nil { - r := node.repo - current := r.BranchStatus(r.Current()) - worktree := r.WorkTreeStatus() - - if worktree != "" { - worktree = fmt.Sprintf("[ %s ]", worktree) - } - - var str strings.Builder - - if worktree == "" && current == "" { - str.WriteString(fmt.Sprintf("%s %s %s", node.val, blue(r.Current()), green("ok"))) - } else { - str.WriteString(fmt.Sprintf("%s %s %s", node.val, blue(r.Current()), strings.Join([]string{yellow(current), red(worktree)}, " "))) - } - - for _, branch := range r.Branches() { - status := r.BranchStatus(branch) - if status == "" { - status = green("ok") - } - - str.WriteString(fmt.Sprintf("\n%s%s %s", indentation(node), blue(branch), yellow(status))) - } - - tp.SetValue(str.String()) + tp.SetValue(printLeaf(node)) } for _, child := range node.children { @@ -150,6 +127,42 @@ func (p *TreePrinter) printTree(node *Node, tp treeprint.Tree) { } } +func printLeaf(node *Node) string { + r := node.repo + + // If any errors happened during status loading, don't print the status but "error" instead. + // Actual error messages are printed in bulk below the tree. + if len(r.Errors()) > 0 { + return fmt.Sprintf("%s %s", node.val, red("error")) + } + + current := r.BranchStatus(r.Current()) + worktree := r.WorkTreeStatus() + + if worktree != "" { + worktree = fmt.Sprintf("[ %s ]", worktree) + } + + var str strings.Builder + + if worktree == "" && current == "" { + str.WriteString(fmt.Sprintf("%s %s %s", node.val, blue(r.Current()), green("ok"))) + } else { + str.WriteString(fmt.Sprintf("%s %s %s", node.val, blue(r.Current()), strings.Join([]string{yellow(current), red(worktree)}, " "))) + } + + for _, branch := range r.Branches() { + status := r.BranchStatus(branch) + if status == "" { + status = green("ok") + } + + str.WriteString(fmt.Sprintf("\n%s%s %s", indentation(node), blue(branch), yellow(status))) + } + + return str.String() +} + // indentation generates a correct indentation for the branches row to match the links to lower rows. // It traverses the tree "upwards" and checks if a parent node is the youngest one (ie, there are no more sibling at the same level). // If it is, it means that level should be indented with empty spaces because there is nothing to link to anymore. diff --git a/pkg/run/run.go b/pkg/run/run.go index cb3eadd..238b28e 100644 --- a/pkg/run/run.go +++ b/pkg/run/run.go @@ -25,13 +25,16 @@ import ( // - run.Git("pull").OnRepo().AndShutUp() // means running "git pull" inside and not printing any output type Cmd struct { - cmd *exec.Cmd + cmd *exec.Cmd + args string + path string } // Git creates a git command with given arguments. func Git(args ...string) *Cmd { return &Cmd{ - cmd: exec.Command("git", args...), + cmd: exec.Command("git", args...), + args: strings.Join(args, " "), } } @@ -46,6 +49,8 @@ func (c *Cmd) OnRepo(path string) *Cmd { // Insert into the args slice after the 1st element (https://github.com/golang/go/wiki/SliceTricks#insert) c.cmd.Args = append(c.cmd.Args[:1], append(insert, c.cmd.Args[1:]...)...) + c.path = path + return c } @@ -56,7 +61,7 @@ func (c *Cmd) AndCaptureLines() ([]string, error) { out, err := c.cmd.Output() if err != nil { - return nil, &GitError{errStream, c.cmd.Args, err} + return nil, &GitError{errStream, c.args, c.path, err} } lines := lines(out) @@ -83,7 +88,7 @@ func (c *Cmd) AndShow() error { err := c.cmd.Run() if err != nil { - return &GitError{&bytes.Buffer{}, c.cmd.Args, err} + return &GitError{&bytes.Buffer{}, c.args, c.path, err} } return nil } @@ -97,7 +102,7 @@ func (c *Cmd) AndShutUp() error { err := c.cmd.Run() if err != nil { - return &GitError{errStream, c.cmd.Args, err} + return &GitError{errStream, c.args, c.path, err} } return nil } @@ -105,16 +110,20 @@ func (c *Cmd) AndShutUp() error { // GitError provides more visibility into why an git command had failed. type GitError struct { Stderr *bytes.Buffer - Args []string + Args string + Path string Err error } func (e GitError) Error() string { msg := e.Stderr.String() - if msg != "" && !strings.HasSuffix(msg, "\n") { - msg += "\n" + + if e.Path == "" { + return fmt.Sprintf("git %s failed: %s", e.Args, msg) } - return fmt.Sprintf("%s%q: %s", msg, strings.Join(e.Args, " "), e.Err) + + return fmt.Sprintf("git %s failed on %s: %s", e.Args, e.Path, msg) + } func lines(output []byte) []string {