Repository: spf13/cobra
Pull request #84 (2015-04-03 23:12:41)

Stop special casing runnable root commands
The special case code to handle a runnable root command had some
problems.  It was noticed that if you created a runnable root and a
subcommand.  And the subcommand was then executed with both a valid and
invalid flag, the error message was about the valid flag being invalid.
For example

./command subcommand --goodflag=10 --badflag=10

Would fail and tell you that --goodflag was an invalid flag. Instead if
we just do away with the special Command.execute() for the root command
the parser for subcommand is what prints the error and it gets it
right...

eparis: @deads2k

https://github.com/spf13/cobra/pull/79

What do you think about solving the above like this?
deads2k: No objection.  I was just trying for minimal change.
deads2k: It's difficult to read the diff and determine complete fidelity of error reporting with the existing code, but the new version is more readable.

lgtm.

Patch:
diff --git a/command.go b/command.go
index 5e882d0..7e52d34 100644
--- a/command.go
+++ b/command.go
@@ -305,6 +305,8 @@ func stripFlags(args []string, c *Command) []string {
 				inFlag = true
 			case inFlag:
 				inFlag = false
+			case y == "":
+				// strip empty commands, as the go tests expect this to be ok....
 			case !strings.HasPrefix(y, "-"):
 				commands = append(commands, y)
 				inFlag = false
@@ -375,9 +377,8 @@ func (c *Command) Find(arrs []string) (*Command, []string, error) {
 
 	commandFound, a := innerfind(c, arrs)
 
-	// if commander returned and the first argument (if it exists) doesn't
-	// match the command name, return nil & error
-	if commandFound.Name() == c.Name() && len(arrs[0]) > 0 && commandFound.Name() != arrs[0] {
+	// If we matched on the root, but we asked for a subcommand, return an error
+	if commandFound.Name() == c.Name() && len(stripFlags(arrs, c)) > 0 && commandFound.Name() != arrs[0] {
 		return nil, a, fmt.Errorf("unknown command %q", a[0])
 	}
 
@@ -398,16 +399,6 @@ func (c *Command) Root() *Command {
 	return findRoot(c)
 }
 
-// execute the command determined by args and the command tree
-func (c *Command) findAndExecute(args []string) (err error) {
-
-	cmd, a, e := c.Find(args)
-	if e != nil {
-		return e
-	}
-	return cmd.execute(a)
-}
-
 func (c *Command) execute(a []string) (err error) {
 	if c == nil {
 		return fmt.Errorf("Called Execute() on a nil Command")
@@ -494,45 +485,11 @@ func (c *Command) Execute() (err error) {
 			c.Help()
 		}
 	} else {
-		err = c.findAndExecute(args)
-	}
-
-	// Now handle the case where the root is runnable and only flags are provided
-	if err != nil && c.Runnable() {
-		// This is pretty much a custom version of the *Command.execute method
-		// with a few differences because it's the final command (no fall back)
-		e := c.ParseFlags(args)
+		cmd, flags, e := c.Find(args)
 		if e != nil {
-			// Flags parsing had an error.
-			// If an error happens here, we have to report it to the user
-			c.Println(e.Error())
-			// If an error happens search also for subcommand info about that
-			if c.cmdErrorBuf != nil && c.cmdErrorBuf.Len() > 0 {
-				c.Println(c.cmdErrorBuf.String())
-			} else {
-				c.Usage()
-			}
 			err = e
-			return
 		} else {
-			// If help is called, regardless of other flags, we print that
-			if c.helpFlagVal {
-				c.Help()
-				return nil
-			}
-
-			argWoFlags := c.Flags().Args()
-			if len(argWoFlags) > 0 {
-				// If there are arguments (not flags) one of the earlier
-				// cases should have caught it.. It means invalid usage
-				// print the usage
-				c.Usage()
-			} else {
-				// Only flags left... Call root.Run
-				c.preRun()
-				c.Run(c, argWoFlags)
-				err = nil
-			}
+			err = cmd.execute(flags)
 		}
 	}
 


Test patch:
diff --git a/cobra_test.go b/cobra_test.go
index 68e55ef..716a9fe 100644
--- a/cobra_test.go
+++ b/cobra_test.go
@@ -3,11 +3,13 @@ package cobra
 import (
 	"bytes"
 	"fmt"
+	"os"
 	"strings"
 	"testing"
 )
 
 var _ = fmt.Println
+var _ = os.Stderr
 
 var tp, te, tt, t1 []string
 var flagb1, flagb2, flagb3, flagbr, flagbp bool
@@ -22,7 +24,7 @@ const strtwoChildHelp = "help message for child flag strtwo"
 var cmdPrint = &Command{
 	Use:   "print [string to print]",
 	Short: "Print anything to the screen",
-	Long:  `an utterly useless command for testing.`,
+	Long:  `an absolutely utterly useless command for testing.`,
 	Run: func(cmd *Command, args []string) {
 		tp = args
 	},
@@ -41,7 +43,7 @@ var cmdEcho = &Command{
 var cmdTimes = &Command{
 	Use:   "times [# times] [string to echo]",
 	Short: "Echo anything to the screen more times",
-	Long:  `an slightly useless command for testing.`,
+	Long:  `a slightly useless command for testing.`,
 	Run: func(cmd *Command, args []string) {
 		tt = args
 	},
@@ -417,6 +419,20 @@ func TestTrailingCommandFlags(t *testing.T) {
 	}
 }
 
+func TestInvalidSubCommandFlags(t *testing.T) {
+	cmd := initializeWithRootCmd()
+	cmd.AddCommand(cmdTimes)
+
+	result := simpleTester(cmd, "times --inttwo=2 --badflag=bar")
+
+	checkResultContains(t, result, "unknown flag: --badflag")
+
+	if strings.Contains(result.Output, "unknown flag: --inttwo") {
+		t.Errorf("invalid --badflag flag shouldn't fail on 'unknown' --inttwo flag")
+	}
+
+}
+
 func TestPersistentFlags(t *testing.T) {
 	fullSetupTest("echo -s something -p more here")
 
@@ -473,6 +489,53 @@ func TestRunnableRootCommand(t *testing.T) {
 	}
 }
 
+func TestRunnableRootCommandNilInput(t *testing.T) {
+	empty_arg := make([]string, 0)
+	c := initializeWithRootCmd()
+
+	buf := new(bytes.Buffer)
+	// Testing flag with invalid input
+	c.SetOutput(buf)
+	cmdEcho.AddCommand(cmdTimes)
+	c.AddCommand(cmdPrint, cmdEcho)
+	c.SetArgs(empty_arg)
+
+	c.Execute()
+
+	if rootcalled != true {
+		t.Errorf("Root Function was not called")
+	}
+}
+
+func TestRunnableRootCommandEmptyInput(t *testing.T) {
+	args := make([]string, 3)
+	args[0] = ""
+	args[1] = "--introot=12"
+	args[2] = ""
+	c := initializeWithRootCmd()
+
+	buf := new(bytes.Buffer)
+	// Testing flag with invalid input
+	c.SetOutput(buf)
+	cmdEcho.AddCommand(cmdTimes)
+	c.AddCommand(cmdPrint, cmdEcho)
+	c.SetArgs(args)
+
+	c.Execute()
+
+	if rootcalled != true {
+		t.Errorf("Root Function was not called.\n\nOutput was:\n\n%s\n", buf)
+	}
+}
+
+func TestInvalidSubcommandWhenArgsAllowed(t *testing.T) {
+	fullSetupTest("echo invalid-sub")
+
+	if te[0] != "invalid-sub" {
+		t.Errorf("Subcommand didn't work...")
+	}
+}
+
 func TestRootFlags(t *testing.T) {
 	fullSetupTest("-i 17 -b")
 
@@ -534,6 +597,24 @@ func TestFlagAccess(t *testing.T) {
 	}
 }
 
+func TestNoNRunnableRootCommandNilInput(t *testing.T) {
+	args := make([]string, 0)
+	c := initialize()
+
+	buf := new(bytes.Buffer)
+	// Testing flag with invalid input
+	c.SetOutput(buf)
+	cmdEcho.AddCommand(cmdTimes)
+	c.AddCommand(cmdPrint, cmdEcho)
+	c.SetArgs(args)
+
+	c.Execute()
+
+	if !strings.Contains(buf.String(), cmdRootNoRun.Long) {
+		t.Errorf("Expected to get help output, Got: \n %s", buf)
+	}
+}
+
 func TestRootNoCommandHelp(t *testing.T) {
 	x := rootOnlySetupTest("--help")