Merge pull request #11 from grdl/pkg_structure

Improve packages structure

pkg/git/config_test.go

@@ -1,8 +1,8 @@
 package git
 
 import (
+	"git-get/pkg/git/test"
 	"git-get/pkg/run"
-	"git-get/pkg/test"
 	"testing"
 )
 

pkg/git/finder.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 	"syscall"
 
@@ -36,10 +37,11 @@ func Exists(path string) (bool, error) {
 	return true, errDirectoryAccess
 }
 
-// RepoFinder finds paths to git repos inside given path.
+// RepoFinder finds git repositories inside a given path.
 type RepoFinder struct {
-	root  string
+	root   string
-	repos []string
+	repos  []*Repo
+	errors []error
 }
 
 // NewRepoFinder returns a RepoFinder pointed at given root path.
@@ -49,51 +51,95 @@ func NewRepoFinder(root string) *RepoFinder {
 	}
 }
 
-// Find returns a sorted list of paths to git repos found inside a given root path.
+// Find finds git repositories inside a given root path.
+// It doesn't add repositories nested inside other git repos.
 // Returns error if root repo path can't be found or accessed.
-func (r *RepoFinder) Find() ([]string, error) {
+func (f *RepoFinder) Find() error {
-	if _, err := Exists(r.root); err != nil {
+	if _, err := Exists(f.root); err != nil {
-		return nil, err
+		return err
 	}
 
 	walkOpts := &godirwalk.Options{
-		ErrorCallback: r.errorCb,
+		ErrorCallback: f.errorCb,
-		Callback:      r.walkCb,
+		Callback:      f.walkCb,
 		// Use Unsorted to improve speed because repos will be processed by goroutines in a random order anyway.
 		Unsorted: true,
 	}
 
-	err := godirwalk.Walk(r.root, walkOpts)
+	err := godirwalk.Walk(f.root, walkOpts)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
-	if len(r.repos) == 0 {
+	if len(f.repos) == 0 {
-		return nil, fmt.Errorf("no git repos found in root path %s", r.root)
+		return fmt.Errorf("no git repos found in root path %s", f.root)
 	}
 
-	return r.repos, nil
+	return nil
 }
 
-func (r *RepoFinder) walkCb(path string, ent *godirwalk.Dirent) error {
+// LoadAll loads and returns sorted slice of statuses of all repositories found by RepoFinder.
+// If fetch equals true, it first fetches from the remote repo before loading the status.
+// Each repo is loaded concurrently in its own goroutine, with max 100 repos being loaded at the same time.
+func (f *RepoFinder) LoadAll(fetch bool) []*Status {
+	var ss []*Status
+
+	loadedChan := make(chan *Status)
+
+	for _, repo := range f.repos {
+		go func(repo *Repo) {
+			loadedChan <- repo.LoadStatus(fetch)
+		}(repo)
+	}
+
+	for l := range loadedChan {
+		ss = append(ss, l)
+
+		// Close the channel when all repos are loaded.
+		if len(ss) == len(f.repos) {
+			close(loadedChan)
+		}
+	}
+
+	// Sort the status slice by path
+	sort.Slice(ss, func(i, j int) bool {
+		return strings.Compare(ss[i].path, ss[j].path) < 0
+	})
+
+	return ss
+}
+
+func (f *RepoFinder) walkCb(path string, ent *godirwalk.Dirent) error {
 	// Do not traverse .git directories
-	if ent.IsDir() && ent.Name() == ".git" {
+	if ent.IsDir() && ent.Name() == dotgit {
-		r.repos = append(r.repos, strings.TrimSuffix(path, ".git"))
+		f.addIfOk(path)
 		return errSkipNode
 	}
 
 	// Do not traverse directories containing a .git directory
 	if ent.IsDir() {
-		_, err := os.Stat(filepath.Join(path, ".git"))
+		_, err := os.Stat(filepath.Join(path, dotgit))
 		if err == nil {
-			r.repos = append(r.repos, strings.TrimSuffix(path, ".git"))
+			f.addIfOk(path)
 			return errSkipNode
 		}
 	}
 	return nil
 }
 
-func (r *RepoFinder) errorCb(_ string, err error) godirwalk.ErrorAction {
+// 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
+	}
+
+	f.repos = append(f.repos, repo)
+}
+
+func (f *RepoFinder) errorCb(_ string, err error) godirwalk.ErrorAction {
 	// Skip .git directory and directories we don't have permissions to access
 	// TODO: Will syscall.EACCES work on windows?
 	if errors.Is(err, errSkipNode) || errors.Is(err, syscall.EACCES) {

pkg/git/finder_test.go

@@ -1,7 +1,7 @@
 package git
 
 import (
-	"git-get/pkg/test"
+	"git-get/pkg/git/test"
 	"testing"
 )
 
@@ -38,10 +38,10 @@ func TestFinder(t *testing.T) {
 			root := test.reposMaker(t)
 
 			finder := NewRepoFinder(root)
-			paths, _ := finder.Find()
+			finder.Find()
 
-			if len(paths) != test.want {
+			if len(finder.repos) != test.want {
-				t.Errorf("expected %d; got %d", test.want, len(paths))
+				t.Errorf("expected %d; got %d", test.want, len(finder.repos))
 			}
 		})
 	}

pkg/git/repo.go

@@ -15,24 +15,12 @@ const (
 	head      = "HEAD"
 )
 
-// Repo represents a git repository on disk.
+// Repo represents a git Repository cloned or initialized on disk.
-type Repo interface {
+type Repo struct {
-	Path() string
-	Branches() ([]string, error)
-	CurrentBranch() (string, error)
-	Fetch() error
-	Remote() (string, error)
-	Uncommitted() (int, error)
-	Untracked() (int, error)
-	Upstream(string) (string, error)
-	AheadBehind(string, string) (int, int, error)
-}
-
-type repo struct {
 	path string
 }
 
-// CloneOpts specify detail about repository to clone.
+// CloneOpts specify detail about Repository to clone.
 type CloneOpts struct {
 	URL    *url.URL
 	Path   string // TODO: should Path be a part of clone opts?
@@ -41,18 +29,18 @@ type CloneOpts struct {
 }
 
 // Open checks if given path can be accessed and returns a Repo instance pointing to it.
-func Open(path string) (Repo, error) {
+func Open(path string) (*Repo, error) {
 	if _, err := Exists(path); err != nil {
 		return nil, err
 	}
 
-	return &repo{
+	return &Repo{
 		path: path,
 	}, nil
 }
 
-// Clone clones repository specified with CloneOpts.
+// Clone clones Repository specified with CloneOpts.
-func Clone(opts *CloneOpts) (Repo, error) {
+func Clone(opts *CloneOpts) (*Repo, error) {
 	runGit := run.Git("clone", opts.URL.String(), opts.Path)
 	if opts.Branch != "" {
 		runGit = run.Git("clone", "--branch", opts.Branch, "--single-branch", opts.URL.String(), opts.Path)
@@ -69,19 +57,19 @@ func Clone(opts *CloneOpts) (Repo, error) {
 		return nil, err
 	}
 
-	repo, err := Open(opts.Path)
+	Repo, err := Open(opts.Path)
-	return repo, err
+	return Repo, err
 }
 
 // Fetch preforms a git fetch on all remotes
-func (r *repo) Fetch() error {
+func (r *Repo) Fetch() error {
 	err := run.Git("fetch", "--all").OnRepo(r.path).AndShutUp()
 	return err
 }
 
-// Uncommitted returns the number of uncommitted files in the repository.
+// Uncommitted returns the number of uncommitted files in the Repository.
 // Only tracked files are not counted.
-func (r *repo) Uncommitted() (int, error) {
+func (r *Repo) Uncommitted() (int, error) {
 	out, err := run.Git("status", "--ignore-submodules", "--porcelain").OnRepo(r.path).AndCaptureLines()
 	if err != nil {
 		return 0, err
@@ -98,8 +86,8 @@ func (r *repo) Uncommitted() (int, error) {
 	return count, nil
 }
 
-// Untracked returns the number of untracked files in the repository.
+// Untracked returns the number of untracked files in the Repository.
-func (r *repo) Untracked() (int, error) {
+func (r *Repo) Untracked() (int, error) {
 	out, err := run.Git("status", "--ignore-submodules", "--untracked-files=all", "--porcelain").OnRepo(r.path).AndCaptureLines()
 	if err != nil {
 		return 0, err
@@ -115,9 +103,9 @@ func (r *repo) Untracked() (int, error) {
 	return count, nil
 }
 
-// CurrentBranch returns the short name currently checked-out branch for the repository.
+// CurrentBranch returns the short name currently checked-out branch for the Repository.
-// If repo is in a detached head state, it will return "HEAD".
+// If Repo is in a detached head state, it will return "HEAD".
-func (r *repo) CurrentBranch() (string, error) {
+func (r *Repo) CurrentBranch() (string, error) {
 	out, err := run.Git("rev-parse", "--symbolic-full-name", "--abbrev-ref", "HEAD").OnRepo(r.path).AndCaptureLine()
 	if err != nil {
 		return "", err
@@ -126,8 +114,8 @@ func (r *repo) CurrentBranch() (string, error) {
 	return out, nil
 }
 
-// Branches returns a list of local branches in the repository.
+// Branches returns a list of local branches in the Repository.
-func (r *repo) Branches() ([]string, error) {
+func (r *Repo) Branches() ([]string, error) {
 	out, err := run.Git("branch", "--format=%(refname:short)").OnRepo(r.path).AndCaptureLines()
 	if err != nil {
 		return nil, err
@@ -146,7 +134,7 @@ func (r *repo) Branches() ([]string, error) {
 
 // Upstream returns the name of an upstream branch if a given branch is tracking one.
 // Otherwise it returns an empty string.
-func (r *repo) Upstream(branch string) (string, error) {
+func (r *Repo) Upstream(branch string) (string, error) {
 	out, err := run.Git("rev-parse", "--abbrev-ref", "--symbolic-full-name", fmt.Sprintf("%s@{upstream}", branch)).OnRepo(r.path).AndCaptureLine()
 	if err != nil {
 		// TODO: no upstream will also throw an error.
@@ -157,7 +145,7 @@ func (r *repo) Upstream(branch string) (string, error) {
 }
 
 // AheadBehind returns the number of commits a given branch is ahead and/or behind the upstream.
-func (r *repo) AheadBehind(branch string, upstream string) (int, int, error) {
+func (r *Repo) AheadBehind(branch string, upstream string) (int, int, error) {
 	out, err := run.Git("rev-list", "--left-right", "--count", fmt.Sprintf("%s...%s", branch, upstream)).OnRepo(r.path).AndCaptureLine()
 	if err != nil {
 		return 0, 0, err
@@ -179,8 +167,8 @@ func (r *repo) AheadBehind(branch string, upstream string) (int, int, error) {
 	return ahead, behind, nil
 }
 
-// Remote returns URL of remote repository.
+// Remote returns URL of remote Repository.
-func (r *repo) Remote() (string, error) {
+func (r *Repo) Remote() (string, error) {
 	// https://stackoverflow.com/a/16880000/1085632
 	out, err := run.Git("ls-remote", "--get-url").OnRepo(r.path).AndCaptureLine()
 	if err != nil {
@@ -191,7 +179,7 @@ func (r *repo) Remote() (string, error) {
 	return out, nil
 }
 
-// Path returns path to the repository.
+// Path returns path to the Repository.
-func (r *repo) Path() string {
+func (r *Repo) Path() string {
 	return r.path
 }

pkg/git/repo_test.go

@@ -1,7 +1,7 @@
 package git
 
 import (
-	"git-get/pkg/test"
+	"git-get/pkg/git/test"
 	"reflect"
 	"testing"
 )
@@ -31,7 +31,7 @@ func TestUncommitted(t *testing.T) {
 			want:      0,
 		},
 		{
-			name:      "single tracked ",
+			name:      "single tracked",
 			repoMaker: test.RepoWithStaged,
 			want:      1,
 		},

pkg/git/status.go

@@ -1,67 +1,62 @@
-package pkg
+package git
 
 import (
 	"fmt"
-	"git-get/pkg/git"
 	"strings"
 )
 
-// Loaded represents a repository which status is Loaded from disk and stored for printing.
+// Status contains human readable (and printable) representation of a git repository status.
-type Loaded struct {
+type Status struct {
 	path     string
 	current  string
 	branches map[string]string // key: branch name, value: branch status
 	worktree string
 	remote   string
-	errors   []string
+	errors   []string // Slice of errors which occurred when loading the status.
 }
 
-// Load reads status of a repository at a given path.
+// LoadStatus reads status of a repository.
-func Load(path string, fetch bool) *Loaded {
+// If fetch equals true, it first fetches from the remote repo before loading the status.
-	loaded := &Loaded{
+// If errors occur during loading, they are stored in Status.errors slice.
-		path:     path,
+func (r *Repo) LoadStatus(fetch bool) *Status {
+	status := &Status{
+		path:     r.path,
 		branches: make(map[string]string),
 		errors:   make([]string, 0),
 	}
 
-	repo, err := git.Open(path)
-	if err != nil {
-		loaded.errors = append(loaded.errors, err.Error())
-		return loaded
-	}
-
 	if fetch {
-		err = repo.Fetch()
+		if err := r.Fetch(); err != nil {
-		if err != nil {
+			status.errors = append(status.errors, err.Error())
-			loaded.errors = append(loaded.errors, err.Error())
 		}
 	}
 
-	loaded.current, err = repo.CurrentBranch()
+	var err error
+	status.current, err = r.CurrentBranch()
 	if err != nil {
-		loaded.errors = append(loaded.errors, err.Error())
+		status.errors = append(status.errors, err.Error())
 	}
 
 	var errs []error
-	loaded.branches, errs = loadBranches(repo)
+	status.branches, errs = r.loadBranches()
 	for _, err := range errs {
-		loaded.errors = append(loaded.errors, err.Error())
+		status.errors = append(status.errors, err.Error())
 	}
 
-	loaded.worktree, err = loadWorkTree(repo)
+	status.worktree, err = r.loadWorkTree()
 	if err != nil {
-		loaded.errors = append(loaded.errors, err.Error())
+		status.errors = append(status.errors, err.Error())
 	}
 
-	loaded.remote, err = repo.Remote()
+	status.remote, err = r.Remote()
 	if err != nil {
-		loaded.errors = append(loaded.errors, err.Error())
+		status.errors = append(status.errors, err.Error())
 	}
 
-	return loaded
+	return status
 }
 
-func loadBranches(r git.Repo) (map[string]string, []error) {
+func (r *Repo) loadBranches() (map[string]string, []error) {
 	statuses := make(map[string]string)
 	errors := make([]error, 0)
 
@@ -72,7 +67,7 @@ func loadBranches(r git.Repo) (map[string]string, []error) {
 	}
 
 	for _, branch := range branches {
-		status, err := loadBranchStatus(r, branch)
+		status, err := r.loadBranchStatus(branch)
 		statuses[branch] = status
 		if err != nil {
 			errors = append(errors, err)
@@ -82,7 +77,7 @@ func loadBranches(r git.Repo) (map[string]string, []error) {
 	return statuses, errors
 }
 
-func loadBranchStatus(r git.Repo, branch string) (string, error) {
+func (r *Repo) loadBranchStatus(branch string) (string, error) {
 	upstream, err := r.Upstream(branch)
 	if err != nil {
 		return "", err
@@ -112,7 +107,7 @@ func loadBranchStatus(r git.Repo, branch string) (string, error) {
 	return strings.Join(res, " "), nil
 }
 
-func loadWorkTree(r git.Repo) (string, error) {
+func (r *Repo) loadWorkTree() (string, error) {
 	uncommitted, err := r.Uncommitted()
 	if err != nil {
 		return "", err
@@ -139,20 +134,20 @@ func loadWorkTree(r git.Repo) (string, error) {
 }
 
 // Path returns path to a repository.
-func (r *Loaded) Path() string {
+func (s *Status) Path() string {
-	return r.path
+	return s.path
 }
 
 // Current returns the name of currently checked out branch (or tag or detached HEAD).
-func (r *Loaded) Current() string {
+func (s *Status) Current() string {
-	return r.current
+	return s.current
 }
 
 // Branches returns a list of all branches names except the currently checked out one. Use Current() to get its name.
-func (r *Loaded) Branches() []string {
+func (s *Status) Branches() []string {
 	var branches []string
-	for b := range r.branches {
+	for b := range s.branches {
-		if b != r.current {
+		if b != s.current {
 			branches = append(branches, b)
 		}
 	}
@@ -160,21 +155,21 @@ func (r *Loaded) Branches() []string {
 }
 
 // BranchStatus returns status of a given branch
-func (r *Loaded) BranchStatus(branch string) string {
+func (s *Status) BranchStatus(branch string) string {
-	return r.branches[branch]
+	return s.branches[branch]
 }
 
 // WorkTreeStatus returns status of a worktree
-func (r *Loaded) WorkTreeStatus() string {
+func (s *Status) WorkTreeStatus() string {
-	return r.worktree
+	return s.worktree
 }
 
 // Remote returns URL to remote repository
-func (r *Loaded) Remote() string {
+func (s *Status) Remote() string {
-	return r.remote
+	return s.remote
 }
 
 // Errors is a slice of errors that occurred when loading repo status
-func (r *Loaded) Errors() []string {
+func (s *Status) Errors() []string {
-	return r.errors
+	return s.errors
 }

pkg/list.go

@@ -5,7 +5,6 @@ import (
 	"git-get/pkg/cfg"
 	"git-get/pkg/git"
 	"git-get/pkg/print"
-	"sort"
 	"strings"
 )
 
@@ -18,16 +17,15 @@ type ListCfg struct {
 
 // List executes the "git list" command.
 func List(c *ListCfg) error {
-	paths, err := git.NewRepoFinder(c.Root).Find()
+	finder := git.NewRepoFinder(c.Root)
-	if err != nil {
+	if err := finder.Find(); err != nil {
 		return err
 	}
 
-	loaded := loadAll(paths, c.Fetch)
+	statuses := finder.LoadAll(c.Fetch)
-
+	printables := make([]print.Printable, len(statuses))
-	printables := make([]print.Printable, len(loaded))
+	for i := range statuses {
-	for i := range loaded {
+		printables[i] = statuses[i]
-		printables[i] = loaded[i]
 	}
 
 	switch c.Output {
@@ -43,33 +41,3 @@ func List(c *ListCfg) error {
 
 	return nil
 }
-
-// loadAll runs a separate goroutine to open, fetch (if asked to) and load status of git repo
-func loadAll(paths []string, fetch bool) []*Loaded {
-	var ll []*Loaded
-
-	loadedChan := make(chan *Loaded)
-
-	for _, path := range paths {
-		go func(path string) {
-
-			loadedChan <- Load(path, fetch)
-		}(path)
-	}
-
-	for l := range loadedChan {
-		ll = append(ll, l)
-
-		// Close the channell when loaded all paths
-		if len(ll) == len(paths) {
-			close(loadedChan)
-		}
-	}
-
-	// sort the loaded slice by path
-	sort.Slice(ll, func(i, j int) bool {
-		return strings.Compare(ll[i].path, ll[j].path) < 0
-	})
-
-	return ll
-}