From 46f41fce471dd4c6c50f7cc81d35618e1ebbd2e0 Mon Sep 17 00:00:00 2001
From: Daniel Nephin <dnephin@gmail.com>
Date: Wed, 17 Jan 2018 13:46:54 -0500
Subject: [PATCH] Refactor to re-use more code between Traverse and
 stripFlags()

Signed-off-by: Daniel Nephin <dnephin@gmail.com>
---
 command.go      | 56 ++++++++++++++++++++++++-------------------------
 command_test.go | 21 +++++++++++++++++++
 2 files changed, 49 insertions(+), 28 deletions(-)

diff --git a/command.go b/command.go
index 232649028..72f5422e9 100644
--- a/command.go
+++ b/command.go
@@ -436,30 +436,7 @@ func (c *Command) VersionTemplate() string {
 `
 }
 
-func hasNoOptDefVal(name string, fs *flag.FlagSet) bool {
-	flag := fs.Lookup(name)
-	if flag == nil {
-		return false
-	}
-	return flag.NoOptDefVal != ""
-}
-
-func shortHasNoOptDefVal(name string, fs *flag.FlagSet) bool {
-	if len(name) == 0 {
-		return false
-	}
-
-	flag := fs.ShorthandLookup(name[:1])
-	if flag == nil {
-		return false
-	}
-	return flag.NoOptDefVal != ""
-}
-
 func stripFlags(args []string, c *Command) []string {
-	if len(args) == 0 {
-		return args
-	}
 	c.mergePersistentFlags()
 
 	commands := []string{}
@@ -470,11 +447,11 @@ Loop:
 		s := args[0]
 		args = args[1:]
 		switch {
-		case strings.HasPrefix(s, "--") && !strings.Contains(s, "=") && !hasNoOptDefVal(s[2:], flags):
+		case strings.HasPrefix(s, "--") && !strings.Contains(s, "=") && !hasDefaultValue(s[2:], flags):
 			// If '--flag arg' then
 			// delete arg from args.
 			fallthrough // (do the same as below)
-		case strings.HasPrefix(s, "-") && !strings.Contains(s, "=") && len(s) == 2 && !shortHasNoOptDefVal(s[1:], flags):
+		case isShortFlagWithNoDefaultValue(s, flags):
 			// If '-f arg' then
 			// delete 'arg' from args or break the loop if len(args) <= 1.
 			if len(args) <= 1 {
@@ -491,6 +468,30 @@ Loop:
 	return commands
 }
 
+func hasDefaultValue(name string, fs *flag.FlagSet) bool {
+	flag := fs.Lookup(name)
+	if flag == nil {
+		return false
+	}
+	return flag.NoOptDefVal != ""
+}
+
+func isShortFlagWithNoDefaultValue(arg string, flags *flag.FlagSet) bool {
+	return strings.HasPrefix(arg, "-") && !strings.Contains(arg, "=") && len(arg) == 2 && !shortFlagHasDefaultValue(arg[1:], flags)
+}
+
+func shortFlagHasDefaultValue(name string, fs *flag.FlagSet) bool {
+	if len(name) == 0 {
+		return false
+	}
+
+	flag := fs.ShorthandLookup(name[:1])
+	if flag == nil {
+		return false
+	}
+	return flag.NoOptDefVal != ""
+}
+
 // argsMinusFirstX removes only the first x from args.  Otherwise, commands that look like
 // openshift admin policy add-role-to-user admin my-user, lose the admin argument (arg[4]).
 func argsMinusFirstX(args []string, x string) []string {
@@ -582,12 +583,11 @@ func (c *Command) Traverse(args []string) (*Command, []string, error) {
 		switch {
 		// A long flag with a space separated value
 		case strings.HasPrefix(arg, "--") && !strings.Contains(arg, "="):
-			// TODO: this isn't quite right, we should really check ahead for 'true' or 'false'
-			inFlag = !hasNoOptDefVal(arg[2:], c.Flags())
+			inFlag = !hasDefaultValue(arg[2:], c.Flags())
 			flags = append(flags, arg)
 			continue
 		// A short flag with a space separated value
-		case strings.HasPrefix(arg, "-") && !strings.Contains(arg, "=") && len(arg) == 2 && !shortHasNoOptDefVal(arg[1:], c.Flags()):
+		case isShortFlagWithNoDefaultValue(arg, c.Flags()):
 			inFlag = true
 			flags = append(flags, arg)
 			continue
diff --git a/command_test.go b/command_test.go
index d3dde1525..f04881026 100644
--- a/command_test.go
+++ b/command_test.go
@@ -1552,6 +1552,27 @@ func TestTraverseWithTwoSubcommands(t *testing.T) {
 	}
 }
 
+func TestTraversalWithPersistentFlags(t *testing.T) {
+	rootCmd := &Command{Use: "root", TraverseChildren: true}
+	rootCmd.PersistentFlags().BoolP("toggle", "t", false, "Help message for toggle")
+	subCmd := &Command{
+		Use:                "sub",
+		DisableFlagParsing: true,
+	}
+	rootCmd.AddCommand(subCmd)
+
+	c, args, err := rootCmd.Traverse([]string{"-t", "sub", "abc"})
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+	if len(args) != 1 && args[0] != "abc" {
+		t.Errorf("Wrong args: %v", args)
+	}
+	if c.Name() != subCmd.Name() {
+		t.Errorf("Expected command: %q, got: %q", subCmd.Name(), c.Name())
+	}
+}
+
 // TestUpdateName checks if c.Name() updates on changed c.Use.
 // Related to https://github.com/spf13/cobra/pull/422#discussion_r143918343.
 func TestUpdateName(t *testing.T) {