From 5452222cb43ffc98952e61dfbbb325c6518947f2 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Mon, 6 Apr 2015 14:22:27 -0400 Subject: [PATCH] Require commands to explicitly state they take arbitrary arguments The code is riddled with inconsistencies between the root command and subcommands. One of those is that the root command cannot take arbitrary arguments and will complain about invalid subcommands. But subcommands will always take arbitrary arguments and will never complain about invalid sub-sub-commands. I make arbitrary arguments explicitly listed. You now need to set cmd.TakesArgs = true if you need to be able to accept arguments tha aren't flags. This is a pretty major change. --- cobra_test.go | 40 +++++++++++++++++++++++++++++----------- command.go | 11 ++++++++--- 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/cobra_test.go b/cobra_test.go index f42d3cb1..9cef8fb7 100644 --- a/cobra_test.go +++ b/cobra_test.go @@ -23,33 +23,44 @@ const strtwoParentHelp = "help message for parent flag strtwo" const strtwoChildHelp = "help message for child flag strtwo" var cmdPrint = &Command{ - Use: "print [string to print]", - Short: "Print anything to the screen", - Long: `an absolutely utterly useless command for testing.`, + Use: "print [string to print]", + Short: "Print anything to the screen", + Long: `an absolutely utterly useless command for testing.`, + TakesArgs: true, Run: func(cmd *Command, args []string) { tp = args }, } var cmdEcho = &Command{ - Use: "echo [string to echo]", - Aliases: []string{"say"}, - Short: "Echo anything to the screen", - Long: `an utterly useless command for testing.`, + Use: "echo [string to echo]", + Aliases: []string{"say"}, + Short: "Echo anything to the screen", + Long: `an utterly useless command for testing.`, + TakesArgs: true, Run: func(cmd *Command, args []string) { te = args }, } var cmdTimes = &Command{ - Use: "times [# times] [string to echo]", - Short: "Echo anything to the screen more times", - Long: `a slightly useless command for testing.`, + Use: "times [# times] [string to echo]", + Short: "Echo anything to the screen more times", + Long: `a slightly useless command for testing.`, + TakesArgs: true, Run: func(cmd *Command, args []string) { tt = args }, } +var cmdNoArgs = &Command{ + Use: "noargs [because they don't work!!]", + Short: "Runnable, but won't take non-flag args", + Long: `something in long form, just because.`, + Run: func(cmd *Command, args []string) { + }, +} + var cmdRootNoRun = &Command{ Use: "cobra-test", Short: "The root can run it's own function", @@ -93,6 +104,7 @@ func flagInit() { cmdEcho.ResetFlags() cmdPrint.ResetFlags() cmdTimes.ResetFlags() + cmdNoArgs.ResetFlags() cmdRootNoRun.ResetFlags() cmdRootSameName.ResetFlags() cmdRootWithRun.ResetFlags() @@ -187,7 +199,7 @@ func fullTester(c *Command, input string) resulter { // Testing flag with invalid input c.SetOutput(buf) cmdEcho.AddCommand(cmdTimes) - c.AddCommand(cmdPrint, cmdEcho) + c.AddCommand(cmdPrint, cmdEcho, cmdNoArgs) c.SetArgs(strings.Split(input, " ")) err := c.Execute() @@ -557,6 +569,12 @@ func TestInvalidSubcommandWhenArgsAllowed(t *testing.T) { } } +func TestInvalidSubcommandWhenArgsDisallowed(t *testing.T) { + x := fullSetupTest("noargs invalid-sub") + + checkResultContains(t, x, "Error: unknown command \"invalid-sub\"\n") +} + func TestRootFlags(t *testing.T) { fullSetupTest("-i 17 -b") diff --git a/command.go b/command.go index 4abd93cf..311eb501 100644 --- a/command.go +++ b/command.go @@ -42,6 +42,10 @@ type Command struct { Long string // Examples of how to use the command Example string + // Accepts arguments that are not defined in flags. This makes subcommand checking and + // error reporting less strict as we can cannot tell if `command [arg]` is an incorrect + // subcommand or just an argument this command will process. + TakesArgs bool // Full set of flags flags *flag.FlagSet // Set of flags childrens of this command will inherit @@ -377,8 +381,8 @@ func (c *Command) Find(arrs []string) (*Command, []string, error) { commandFound, a := innerfind(c, arrs) - // 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] { + // Reject if there are args left and the command doesn't take args + if commandFound.TakesArgs == false && len(stripFlags(a, commandFound)) > 0 { return nil, a, fmt.Errorf("unknown command %q", a[0]) } @@ -517,7 +521,8 @@ func (c *Command) initHelp() { Short: "Help about any command", Long: `Help provides help for any command in the application. Simply type ` + c.Name() + ` help [path to command] for full details.`, - Run: c.HelpFunc(), + TakesArgs: true, + Run: c.HelpFunc(), } } c.AddCommand(c.helpCommand)