refactor according to good practices

6 files changed, 89 insertions, 73 deletions

diff --git a/pkg/app/app.go b/pkg/app/app.go
index 9c9e9a7..7d51abb 100644
--- a/pkg/app/app.go
+++ b/pkg/app/app.go
@@ -45,18 +45,18 @@ func Setup(setupConfig *SetupConfig) (*App, error) {
 		x := appConfig.Mode == "fetch"
 		y := appConfig.Mode == "pull"
 		if x == y {
-			log.Fatal("Unrecognized quick mode: " + appConfig.Mode)
+			log.Error("Unrecognized quick mode: " + appConfig.Mode)
+			os.Exit(1)
 		}
 		quick(directories, appConfig.Depth, appConfig.Mode)
-		log.Fatal("Finished")
+		os.Exit(0)
 	}
 
 	// create a gui.Gui struct and set it as App's gui
 	app.Gui, err = gui.NewGui(appConfig.Mode, directories)
 	if err != nil {
-		// the error types and handling is not considered yer
-		log.Error(err)
-		return app, err
+		// the error types and handling is not considered yet
+		return nil, err
 	}
 	// hopefull everything went smooth as butter
 	log.Trace("App configuration completed")
diff --git a/pkg/app/files.go b/pkg/app/files.go
index 25e5074..1ec05cd 100644
--- a/pkg/app/files.go
+++ b/pkg/app/files.go
@@ -4,20 +4,20 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
-	"strings"
 
 	log "github.com/sirupsen/logrus"
 )
 
 // generateDirectories returns poosible git repositories to pipe into git pkg's
 // load function
-func generateDirectories(directories []string, depth int) (gitDirectories []string) {
+func generateDirectories(dirs []string, depth int) []string {
+	gitDirs := make([]string, 0)
 	for i := 0; i <= depth; i++ {
-		nonrepos, repos := walkRecursive(directories, gitDirectories)
-		directories = nonrepos
-		gitDirectories = repos
+		nonrepos, repos := walkRecursive(dirs, gitDirs)
+		dirs = nonrepos
+		gitDirs = repos
 	}
-	return gitDirectories
+	return gitDirs
 }
 
 // returns given values, first search directories and second stands for possible
@@ -33,7 +33,7 @@ func walkRecursive(search, appendant []string) ([]string, []string) {
 		if err != nil {
 			log.WithFields(log.Fields{
 				"directory": search[i],
-			}).Trace("Can't read directory")
+			}).WithError(err).Trace("Can't read directory")
 			continue
 		}
 		// since we started to search let's get rid of it and remove from search
@@ -49,14 +49,16 @@ func walkRecursive(search, appendant []string) ([]string, []string) {
 
 // seperateDirectories is to find all the files in given path. This method
 // does not check if the given file is a valid git repositories
-func seperateDirectories(directory string) (directories, gitDirectories []string, err error) {
+func seperateDirectories(directory string) ([]string, []string, error) {
+	dirs := make([]string, 0)
+	gitDirs := make([]string, 0)
 	files, err := ioutil.ReadDir(directory)
 	// can we read the directory?
 	if err != nil {
 		log.WithFields(log.Fields{
 			"directory": directory,
 		}).Trace("Can't read directory")
-		return directories, gitDirectories, nil
+		return nil, nil, nil
 	}
 	for _, f := range files {
 		repo := directory + string(os.PathSeparator) + f.Name()
@@ -66,38 +68,25 @@ func seperateDirectories(directory string) (directories, gitDirectories []string
 			log.WithFields(log.Fields{
 				"file":      file,
 				"directory": repo,
-			}).Trace("Failed to open file in the directory")
+			}).WithError(err).Trace("Failed to open file in the directory")
+			file.Close()
 			continue
 		}
 		dir, err := filepath.Abs(file.Name())
 		if err != nil {
-			return nil, nil, err
+			file.Close()
+			continue
 		}
 		// with this approach, we ignore submodule or sub repositoreis in a git repository
 		ff, err := os.Open(dir + string(os.PathSeparator) + ".git")
 		if err != nil {
-			directories = append(directories, dir)
+			dirs = append(dirs, dir)
 		} else {
-			gitDirectories = append(gitDirectories, dir)
+			gitDirs = append(gitDirs, dir)
 		}
 		ff.Close()
 		file.Close()
 
 	}
-	return directories, gitDirectories, nil
-}
-
-// takes a fileInfo slice and returns it with the ones matches with the
-// pattern string
-func filterDirectories(files []os.FileInfo, pattern string) []os.FileInfo {
-	var filteredRepos []os.FileInfo
-	for _, f := range files {
-		// it is just a simple filter
-		if strings.Contains(f.Name(), pattern) && f.Name() != ".git" {
-			filteredRepos = append(filteredRepos, f)
-		} else {
-			continue
-		}
-	}
-	return filteredRepos
+	return dirs, gitDirs, nil
 }
diff --git a/pkg/app/quick.go b/pkg/app/quick.go
index 488e1d6..90a2283 100644
--- a/pkg/app/quick.go
+++ b/pkg/app/quick.go
@@ -37,10 +37,12 @@ func operate(directory, mode string) error {
 	case "fetch":
 		return git.Fetch(r, git.FetchOptions{
 			RemoteName: "origin",
+			Progress:   true,
 		})
 	case "pull":
 		return git.Pull(r, git.PullOptions{
 			RemoteName: "origin",
+			Progress:   true,
 		})
 	}
 	return nil
diff --git a/pkg/git/branch.go b/pkg/git/branch.go
index 94ad578..084288a 100644
--- a/pkg/git/branch.go
+++ b/pkg/git/branch.go
@@ -38,40 +38,43 @@ func (e *RepoEntity) loadLocalBranches() error {
 	}
 	var branchFound bool
 	bs.ForEach(func(b *plumbing.Reference) error {
-		if b.Type() == plumbing.HashReference {
-			var push, pull string
-			pushables, err := RevList(e, RevListOptions{
-				Ref1: "@{u}",
-				Ref2: "HEAD",
-			})
-			if err != nil {
-				push = pushables[0]
-			} else {
-				push = strconv.Itoa(len(pushables))
-			}
-			pullables, err := RevList(e, RevListOptions{
-				Ref1: "HEAD",
-				Ref2: "@{u}",
-			})
-			if err != nil {
-				pull = pullables[0]
-			} else {
-				pull = strconv.Itoa(len(pullables))
-			}
-			clean := e.isClean()
-			branch := &Branch{
-				Name:      b.Name().Short(),
-				Reference: b,
-				Pushables: push,
-				Pullables: pull,
-				Clean:     clean,
-			}
-			if b.Name() == headRef.Name() {
-				e.Branch = branch
-				branchFound = true
-			}
-			lbs = append(lbs, branch)
+		if b.Type() != plumbing.HashReference {
+			return nil
 		}
+
+		var push, pull string
+		pushables, err := RevList(e, RevListOptions{
+			Ref1: "@{u}",
+			Ref2: "HEAD",
+		})
+		if err != nil {
+			push = pushables[0]
+		} else {
+			push = strconv.Itoa(len(pushables))
+		}
+		pullables, err := RevList(e, RevListOptions{
+			Ref1: "HEAD",
+			Ref2: "@{u}",
+		})
+		if err != nil {
+			pull = pullables[0]
+		} else {
+			pull = strconv.Itoa(len(pullables))
+		}
+		clean := e.isClean()
+		branch := &Branch{
+			Name:      b.Name().Short(),
+			Reference: b,
+			Pushables: push,
+			Pullables: pull,
+			Clean:     clean,
+		}
+		if b.Name() == headRef.Name() {
+			e.Branch = branch
+			branchFound = true
+		}
+		lbs = append(lbs, branch)
+
 		return nil
 	})
 	if !branchFound {
@@ -112,8 +115,8 @@ func (e *RepoEntity) currentBranchIndex() int {
 
 // Checkout to given branch. If any errors occur, the method returns it instead
 // of returning nil
-func (e *RepoEntity) Checkout(branch *Branch) error {
-	if branch.Name == e.Branch.Name {
+func (e *RepoEntity) Checkout(b *Branch) error {
+	if b.Name == e.Branch.Name {
 		return nil
 	}
 
@@ -123,7 +126,7 @@ func (e *RepoEntity) Checkout(branch *Branch) error {
 		return err
 	}
 	if err = w.Checkout(&git.CheckoutOptions{
-		Branch: branch.Reference.Name(),
+		Branch: b.Reference.Name(),
 	}); err != nil {
 		log.Warn("Cannot checkout " + err.Error())
 		return err
@@ -131,7 +134,7 @@ func (e *RepoEntity) Checkout(branch *Branch) error {
 
 	// make this conditional on global scale
 	// we don't care if this function returns an error
-	e.Remote.SyncBranches(branch.Name)
+	e.Remote.SyncBranches(b.Name)
 
 	return e.Refresh()
 }
diff --git a/pkg/git/cmd-fetch.go b/pkg/git/cmd-fetch.go
index 0830105..2c66c1c 100644
--- a/pkg/git/cmd-fetch.go
+++ b/pkg/git/cmd-fetch.go
@@ -1,10 +1,11 @@
 package git
 
 import (
+	"os"
 	"strings"
 
 	log "github.com/sirupsen/logrus"
-	"gopkg.in/src-d/go-git.v4"
+	git "gopkg.in/src-d/go-git.v4"
 	"gopkg.in/src-d/go-git.v4/config"
 	"gopkg.in/src-d/go-git.v4/plumbing/transport"
 	"gopkg.in/src-d/go-git.v4/plumbing/transport/http"
@@ -31,6 +32,8 @@ type FetchOptions struct {
 	Prune bool
 	// Show what would be done, without making any changes.
 	DryRun bool
+	// Process logs the output to stdout
+	Progress bool
 	// Force allows the fetch to update a local branch even when the remote
 	// branch does not descend from it.
 	Force bool
@@ -122,6 +125,9 @@ func fetchWithGoGit(e *RepoEntity, options FetchOptions, refspec string) (err er
 			return ErrInvalidAuthMethod
 		}
 	}
+	if options.Progress {
+		opt.Progress = os.Stdout
+	}
 
 	if err := e.Repository.Fetch(opt); err != nil {
 		if err == git.NoErrAlreadyUpToDate {
@@ -137,6 +143,10 @@ func fetchWithGoGit(e *RepoEntity, options FetchOptions, refspec string) (err er
 			} else {
 				return err
 			}
+			// TODO: submit a PR for this kind of error, this type of catch is lame
+		} else if strings.Contains(err.Error(), "SSH_AUTH_SOCK") {
+			// The env variable SSH_AUTH_SOCK is not defined, maybe git can handle this
+			return fetchWithGit(e, options)
 		} else if err == transport.ErrAuthenticationRequired {
 			log.Warn(err.Error())
 			return ErrAuthenticationRequired
diff --git a/pkg/git/cmd-pull.go b/pkg/git/cmd-pull.go
index f620925..8f2cf92 100644
--- a/pkg/git/cmd-pull.go
+++ b/pkg/git/cmd-pull.go
@@ -1,8 +1,11 @@
 package git
 
 import (
+	"os"
+	"strings"
+
 	log "github.com/sirupsen/logrus"
-	"gopkg.in/src-d/go-git.v4"
+	git "gopkg.in/src-d/go-git.v4"
 	"gopkg.in/src-d/go-git.v4/plumbing"
 	"gopkg.in/src-d/go-git.v4/plumbing/transport"
 	"gopkg.in/src-d/go-git.v4/plumbing/transport/http"
@@ -28,6 +31,8 @@ type PullOptions struct {
 	SingleBranch bool
 	// Credentials holds the user and pswd information
 	Credentials Credentials
+	// Process logs the output to stdout
+	Progress bool
 	// Force allows the pull to update a local branch even when the remote
 	// branch does not descend from it.
 	Force bool
@@ -94,6 +99,9 @@ func pullWithGoGit(e *RepoEntity, options PullOptions) (err error) {
 			return ErrInvalidAuthMethod
 		}
 	}
+	if options.Progress {
+		opt.Progress = os.Stdout
+	}
 	w, err := e.Repository.Worktree()
 	if err != nil {
 		return err
@@ -103,6 +111,10 @@ func pullWithGoGit(e *RepoEntity, options PullOptions) (err error) {
 		if err == git.NoErrAlreadyUpToDate {
 			// Already up-to-date
 			log.Warn(err.Error())
+			// TODO: submit a PR for this kind of error, this type of catch is lame
+		} else if strings.Contains(err.Error(), "SSH_AUTH_SOCK") {
+			// The env variable SSH_AUTH_SOCK is not defined, maybe git can handle this
+			return pullWithGit(e, options)
 		} else if err == transport.ErrAuthenticationRequired {
 			log.Warn(err.Error())
 			return ErrAuthenticationRequired