Refactor to reduce code complexity (gopasspw#542)

- action.Find
- action.Clone
- sub.Fsck
- sub.Set
- termwiz.GetSelection
- action.Generate
- action.List
- action.Insert
- action.Show
- main

Makefile

@@ -24,6 +24,7 @@ GOOS ?= $(shell go version | cut -d' ' -f4 | cut -d'/' -f1)
 GOARCH ?= $(shell go version | cut -d' ' -f4 | cut -d'/' -f2)
 
 PACKAGES ?= $(shell go list ./... | grep -v /vendor/ | grep -v /tests)
+GOFILES ?= $(shell find . -type f -name "*.go" | grep -v vendor/)
 
 TAGS ?= netgo
 
@@ -47,7 +48,7 @@ fmt:
 	$(GO) fmt $(PACKAGES)
 
 .PHONY: tests
-tests: test-cross test vet lint errcheck megacheck
+tests: test-cross test vet lint errcheck megacheck gocyclo
 
 .PHONY: vet
 vet:
@@ -74,6 +75,13 @@ megacheck:
 	fi
 	STATUS=0; for PKG in $(PACKAGES); do megacheck $$PKG || STATUS=1; done; exit $$STATUS
 
+.PHONY: gocyclo
+gocyclo:
+	@which gocyclo > /dev/null; if [ $$? -ne 0 ]; then \
+		$(GO) get -u github.com/fzipp/gocyclo; \
+	fi
+	STATUS=0; for FN in $(GOFILES); do gocyclo -over 15 $$FN || STATUS=1; done; exit $$STATUS
+
 .PHONY: test
 test:
 	STATUS=0; for PKG in $(PACKAGES); do go test -cover -coverprofile $$GOPATH/src/$$PKG/coverage.out $$PKG || STATUS=1; done; exit $$STATUS

action/clihelper_others.go

@@ -15,9 +15,9 @@ import (
 )
 
 // promptPass will prompt user's for a password by terminal.
-func (s *Action) promptPass(ctx context.Context, prompt string) (pass string, err error) {
+func (s *Action) promptPass(ctx context.Context, prompt string) (string, error) {
 	if !ctxutil.IsTerminal(ctx) {
-		return
+		return "", nil
 	}
 
 	// Make a copy of STDIN's state to restore afterward

action/clone.go

@@ -63,24 +63,15 @@ func (s *Action) clone(ctx context.Context, repo, mount, path string) error {
 	if err != nil {
 		out.Red(ctx, "Failed to ask for signing key: %s", err)
 	}
-	// for convenience, set defaults to user-selected values from available private keys
-	// NB: discarding returned error since this is merely a best-effort look-up for convenience
-	userName, userEmail, _ := s.askForGitConfigUser(ctx)
-	if userName == "" {
-		var err error
-		userName, err = s.askForString(ctx, color.CyanString("Please enter a user name for password store git config"), userName)
-		if err != nil {
-			return exitError(ctx, ExitIO, err, "Failed to read user input: %s", err)
-		}
-	}
-	if userEmail == "" {
-		var err error
-		userEmail, err = s.askForString(ctx, color.CyanString("Please enter an email address for password store git config"), userEmail)
-		if err != nil {
-			return exitError(ctx, ExitIO, err, "Failed to read user input: %s", err)
-		}
+
+	// ask for git config values
+	username, email, err := s.cloneGetGitConfig(ctx)
+	if err != nil {
+		return err
 	}
-	if err := s.Store.GitInitConfig(ctx, mount, sk, userName, userEmail); err != nil {
+
+	// initialize git config
+	if err := s.Store.GitInitConfig(ctx, mount, sk, username, email); err != nil {
 		out.Debug(ctx, "Stacktrace: %+v\n", err)
 		out.Red(ctx, "Failed to configure git: %s", err)
 	}
@@ -92,3 +83,24 @@ func (s *Action) clone(ctx context.Context, repo, mount, path string) error {
 
 	return nil
 }
+
+func (s *Action) cloneGetGitConfig(ctx context.Context) (string, string, error) {
+	// for convenience, set defaults to user-selected values from available private keys
+	// NB: discarding returned error since this is merely a best-effort look-up for convenience
+	username, email, _ := s.askForGitConfigUser(ctx)
+	if username == "" {
+		var err error
+		username, err = s.askForString(ctx, color.CyanString("Please enter a user name for password store git config"), username)
+		if err != nil {
+			return "", "", exitError(ctx, ExitIO, err, "Failed to read user input: %s", err)
+		}
+	}
+	if email == "" {
+		var err error
+		email, err = s.askForString(ctx, color.CyanString("Please enter an email address for password store git config"), email)
+		if err != nil {
+			return "", "", exitError(ctx, ExitIO, err, "Failed to read user input: %s", err)
+		}
+	}
+	return username, email, nil
+}

action/config.go

@@ -45,7 +45,7 @@ func (s *Action) printConfigValues(ctx context.Context, store string, needles ..
 
 	m := s.cfg.Root.ConfigMap()
 	if store == "" {
-		for _, k := range filter(m, needles) {
+		for _, k := range filterMap(m, needles) {
 			out.Print(ctx, "%s%s: %s\n", prefix, k, m[k])
 		}
 	}
@@ -60,7 +60,7 @@ func (s *Action) printConfigValues(ctx context.Context, store string, needles ..
 			mp += "/"
 		}
 		sm := sc.ConfigMap()
-		for _, k := range filter(sm, needles) {
+		for _, k := range filterMap(sm, needles) {
 			if sm[k] != m[k] || store != "" {
 				out.Print(ctx, "%s%s: %s\n", mp, k, sm[k])
 			}
@@ -69,7 +69,7 @@ func (s *Action) printConfigValues(ctx context.Context, store string, needles ..
 	return nil
 }
 
-func filter(haystack map[string]string, needles []string) []string {
+func filterMap(haystack map[string]string, needles []string) []string {
 	out := make([]string, 0, len(haystack))
 	for k := range haystack {
 		if !contains(needles, k) {

action/find.go

@@ -26,23 +26,22 @@ func (s *Action) Find(ctx context.Context, c *cli.Context) error {
 	}
 
 	needle := strings.ToLower(c.Args().First())
-	choices := make([]string, 0, 10)
-	for _, value := range l {
-		if strings.Contains(strings.ToLower(value), needle) {
-			choices = append(choices, value)
-		}
-	}
+	choices := filter(l, needle)
 
+	// if we have an exact match print it
 	if len(choices) == 1 {
 		out.Green(ctx, "Found exact match in '%s'", choices[0])
 		return s.show(ctx, c, choices[0], "", false)
 	}
 
+	// if we don't have a match yet try a fuzzy search
 	if len(choices) < 1 && ctxutil.IsFuzzySearch(ctx) {
 		// try fuzzy match
 		cm := closestmatch.New(l, []int{2})
 		choices = cm.ClosestN(needle, 5)
 	}
+
+	// if there are still no results we abort
 	if len(choices) < 1 {
 		return fmt.Errorf("no results found")
 	}
@@ -56,6 +55,10 @@ func (s *Action) Find(ctx context.Context, c *cli.Context) error {
 		return nil
 	}
 
+	return s.findSelection(ctx, c, choices, needle)
+}
+
+func (s *Action) findSelection(ctx context.Context, c *cli.Context, choices []string, needle string) error {
 	act, sel := termwiz.GetSelection(ctx, "Found secrets -", "", choices)
 	switch act {
 	case "default":
@@ -80,3 +83,13 @@ func (s *Action) Find(ctx context.Context, c *cli.Context) error {
 		return exitError(ctx, ExitAborted, nil, "user aborted")
 	}
 }
+
+func filter(l []string, needle string) []string {
+	choices := make([]string, 0, 10)
+	for _, value := range l {
+		if strings.Contains(strings.ToLower(value), needle) {
+			choices = append(choices, value)
+		}
+	}
+	return choices
+}

action/generate.go

@@ -31,17 +31,7 @@ func (s *Action) Generate(ctx context.Context, c *cli.Context) error {
 	}
 
 	name := c.Args().Get(0)
-	key := c.Args().Get(1)
-	length := c.Args().Get(2)
-
-	// generate can be called with one positional arg or two
-	// one - the desired length for the "master" secret itself
-	// two - the key in a YAML doc and the length for a secret generated for this
-	// key only
-	if length == "" && key != "" {
-		length = key
-		key = ""
-	}
+	key, length := keyAndLength(c)
 
 	// ask for name of the secret if it wasn't provided already
 	if name == "" {
@@ -81,6 +71,22 @@ func (s *Action) Generate(ctx context.Context, c *cli.Context) error {
 	return s.generateCopyOrPrint(ctx, c, name, key, password)
 }
 
+func keyAndLength(c *cli.Context) (string, string) {
+	key := c.Args().Get(1)
+	length := c.Args().Get(2)
+
+	// generate can be called with one positional arg or two
+	// one - the desired length for the "master" secret itself
+	// two - the key in a YAML doc and the length for a secret generated for this
+	// key only
+	if length == "" && key != "" {
+		length = key
+		key = ""
+	}
+
+	return key, length
+}
+
 func (s *Action) generateCopyOrPrint(ctx context.Context, c *cli.Context, name, key, password string) error {
 	if c.Bool("print") {
 		if key != "" {

action/insert.go

@@ -20,6 +20,7 @@ func (s *Action) Insert(ctx context.Context, c *cli.Context) error {
 	multiline := c.Bool("multiline")
 	force := c.Bool("force")
 
+	// if force mode is requested we mock the recipient func to just return anything that goes in
 	if force {
 		ctx = sub.WithRecipientFunc(ctx, func(ctx context.Context, msg string, rs []string) ([]string, error) {
 			return rs, nil
@@ -52,20 +53,12 @@ func (s *Action) Insert(ctx context.Context, c *cli.Context) error {
 	}
 
 	if ctxutil.IsStdin(ctx) {
-		sec, err := secret.Parse(content)
-		if err != nil {
-			out.Red(ctx, "WARNING: Invalid YAML: %s", err)
-		}
-		if err := s.Store.Set(sub.WithReason(ctx, "Read secret from STDIN"), name, sec); err != nil {
-			return exitError(ctx, ExitEncrypt, err, "failed to set '%s': %s", name, err)
-		}
-		return nil
+		return s.insertStdin(ctx, name, content)
 	}
 
-	if !force { // don't check if it's force anyway
-		if s.Store.Exists(ctx, name) && !s.AskForConfirmation(ctx, fmt.Sprintf("An entry already exists for %s. Overwrite it?", name)) {
-			return exitError(ctx, ExitAborted, nil, "not overwriting your current secret")
-		}
+	// don't check if it's force anyway
+	if !force && s.Store.Exists(ctx, name) && !s.AskForConfirmation(ctx, fmt.Sprintf("An entry already exists for %s. Overwrite it?", name)) {
+		return exitError(ctx, ExitAborted, nil, "not overwriting your current secret")
 	}
 
 	// if multi-line input is requested start an editor
@@ -86,6 +79,21 @@ func (s *Action) Insert(ctx context.Context, c *cli.Context) error {
 		return exitError(ctx, ExitIO, err, "failed to ask for password: %s", err)
 	}
 
+	return s.insertSingle(ctx, name, pw)
+}
+
+func (s *Action) insertStdin(ctx context.Context, name string, content []byte) error {
+	sec, err := secret.Parse(content)
+	if err != nil {
+		out.Red(ctx, "WARNING: Invalid YAML: %s", err)
+	}
+	if err := s.Store.Set(sub.WithReason(ctx, "Read secret from STDIN"), name, sec); err != nil {
+		return exitError(ctx, ExitEncrypt, err, "failed to set '%s': %s", name, err)
+	}
+	return nil
+}
+
+func (s *Action) insertSingle(ctx context.Context, name, pw string) error {
 	sec := &secret.Secret{}
 	if s.Store.Exists(ctx, name) {
 		var err error

action/jsonapi.go

@@ -61,18 +61,20 @@ func (s *Action) SetupNativeMessaging(ctx context.Context, c *cli.Context) error
 	return err
 }
 
-func (s *Action) getBrowser(ctx context.Context, c *cli.Context) (browser string, err error) {
-	browser = c.String("browser")
-	if browser == "" {
-		browser, err = s.askForString(ctx, color.BlueString("For which browser do you want to install gopass native messaging? [%s]", strings.Join(manifest.ValidBrowsers[:], ",")), manifest.DefaultBrowser)
-		if err != nil {
-			return "", errors.Wrapf(err, "failed to ask for user input")
-		}
-		if !stringInSlice(browser, manifest.ValidBrowsers) {
-			return "", errors.Errorf("%s not one of %s", browser, strings.Join(manifest.ValidBrowsers[:], ","))
-		}
+func (s *Action) getBrowser(ctx context.Context, c *cli.Context) (string, error) {
+	browser := c.String("browser")
+	if browser != "" {
+		return browser, nil
+	}
+
+	browser, err := s.askForString(ctx, color.BlueString("For which browser do you want to install gopass native messaging? [%s]", strings.Join(manifest.ValidBrowsers[:], ",")), manifest.DefaultBrowser)
+	if err != nil {
+		return "", errors.Wrapf(err, "failed to ask for user input")
 	}
-	return
+	if !stringInSlice(browser, manifest.ValidBrowsers) {
+		return "", errors.Errorf("%s not one of %s", browser, strings.Join(manifest.ValidBrowsers[:], ","))
+	}
+	return browser, nil
 }
 
 func (s *Action) getGlobalInstall(ctx context.Context, c *cli.Context) (bool, error) {
@@ -89,15 +91,16 @@ func (s *Action) getLibPath(ctx context.Context, c *cli.Context, browser string,
 	return c.String("libpath"), nil
 }
 
-func (s *Action) getWrapperPath(ctx context.Context, c *cli.Context) (path string, err error) {
-	path = c.String("path")
-	if path == "" {
-		path, err = s.askForString(ctx, color.BlueString("In which path should gopass_wrapper.sh be installed?"), config.Directory())
-		if err != nil {
-			return "", errors.Wrapf(err, "failed to ask for user input")
-		}
+func (s *Action) getWrapperPath(ctx context.Context, c *cli.Context) (string, error) {
+	path := c.String("path")
+	if path != "" {
+		return path, nil
+	}
+	path, err := s.askForString(ctx, color.BlueString("In which path should gopass_wrapper.sh be installed?"), config.Directory())
+	if err != nil {
+		return "", errors.Wrapf(err, "failed to ask for user input")
 	}
-	return
+	return path, nil
 }
 
 func stringInSlice(a string, list []string) bool {