From 9892605665e2472b5bb174bd70ba80012e206302 Mon Sep 17 00:00:00 2001 From: Andre Mueller Date: Sat, 6 Jul 2024 13:13:47 +0200 Subject: [PATCH 1/3] closes #2130 new command flag ErrorOnUnknownSubcommand --- command.go | 22 ++++++++++-- command_test.go | 89 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 2 deletions(-) diff --git a/command.go b/command.go index 54748fc67..489e0c3c8 100644 --- a/command.go +++ b/command.go @@ -44,6 +44,10 @@ type Group struct { Title string } +// ErrUnknownSubcommand is returned by Command.Execute() when the subcommand was not found. +// Hereto, the ErrorOnUnknownSubcommand flag must be set true. +var ErrUnknownSubcommand = errors.New("subcommand is unknown") + // Command is just that, a command for your application. // E.g. 'go run ...' - 'run' is the command. Cobra requires // you to define the usage and description as part of your command @@ -254,6 +258,17 @@ type Command struct { // SuggestionsMinimumDistance defines minimum levenshtein distance to display suggestions. // Must be > 0. SuggestionsMinimumDistance int + + // ErrorOnUnknownSubcommand controls the behavior of subcommand handling. + // When the flag is set true the behavior of Command.Execute() will change: + // If a sub-subcommand is not found ErrUnknownSubcommand will be returned on calling + // Command.Exec() instead of the old behavior where a nil error was sent. + // If the flag is false (default) the old behavior is performed. + // For this behavior the child subcommand must be nil. + // Example: in root/service/run - there would be an existing subcommand `run` + // in root/service/unknown - there would be an unknown subcommand `unknown` therefore returning an error + // `service` must have a nil Command.Run() function for this. + ErrorOnUnknownSubcommand bool } // Context returns underlying command context. If command was executed @@ -923,9 +938,12 @@ func (c *Command) execute(a []string) (err error) { } if !c.Runnable() { - return flag.ErrHelp + if c.ErrorOnUnknownSubcommand { + return ErrUnknownSubcommand + } else { + return flag.ErrHelp + } } - c.preRun() defer c.postRun() diff --git a/command_test.go b/command_test.go index 9ce7a529b..1012ca4ad 100644 --- a/command_test.go +++ b/command_test.go @@ -17,6 +17,7 @@ package cobra import ( "bytes" "context" + "errors" "fmt" "io" "os" @@ -174,6 +175,94 @@ func TestSubcommandExecuteC(t *testing.T) { } } +func TestSubcommandExecuteMissingSubcommand(t *testing.T) { + rootCmd := &Command{Use: "root", Run: emptyRun} + childCmd := &Command{Use: "child", Run: nil, ErrorOnUnknownSubcommand: false} + child2Cmd := &Command{Use: "child2", Run: emptyRun} + rootCmd.AddCommand(childCmd) + childCmd.AddCommand(child2Cmd) + + // test existing command + c, output, err := executeCommandC(rootCmd, "child") + if !strings.HasPrefix(output, "Usage:") { + t.Errorf("Unexpected output: %v", output) + } + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if c.Name() != "child" { + t.Errorf(`invalid command returned from ExecuteC: expected "child"', got: %q`, c.Name()) + } + + // test existing sub command + c, output, err = executeCommandC(rootCmd, "child", "child2") + if output != "" { + t.Errorf("Unexpected output: %v", output) + } + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if c.Name() != "child2" { + t.Errorf(`invalid command returned from ExecuteC: expected "child"', got: %q`, c.Name()) + } + + // now test a command which does not exist, we will get no error, just "Usage:" is printed + c, output, err = executeCommandC(rootCmd, "child", "unknownChild") + if !strings.HasPrefix(output, "Usage:") { + t.Errorf("Expected: 'Usage: ...'\nGot:\n %q\n", output) + } + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if c.Name() != "child" { + t.Errorf(`invalid command returned from ExecuteC: expected "child"', got: %q`, c.Name()) + } +} + +func TestSubcommandExecuteMissingSubcommandWithErrorOnUnknownSubcommand(t *testing.T) { + rootCmd := &Command{Use: "root", Run: emptyRun} + childCmd := &Command{Use: "child", Run: nil, ErrorOnUnknownSubcommand: true} + child2Cmd := &Command{Use: "child2", Run: emptyRun} + rootCmd.AddCommand(childCmd) + childCmd.AddCommand(child2Cmd) + + // test existing command + c, output, err := executeCommandC(rootCmd, "child") + if !strings.HasPrefix(output, "Error:") { + t.Errorf("Unexpected output: %v", output) + } + if !errors.Is(err, ErrUnknownSubcommand) { + t.Errorf("Unexpected error: %v", err) + } + if c.Name() != "child" { + t.Errorf(`invalid command returned from ExecuteC: expected "child"', got: %q`, c.Name()) + } + + // test existing sub command + c, output, err = executeCommandC(rootCmd, "child", "child2") + if output != "" { + t.Errorf("Unexpected output: %v", output) + } + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if c.Name() != "child2" { + t.Errorf(`invalid command returned from ExecuteC: expected "child"', got: %q`, c.Name()) + } + + // now test a command which does not exist, we expect an error because of the ErrorOnUnknownSubcommand flag + c, output, err = executeCommandC(rootCmd, "child", "unknownChild") + if !strings.HasPrefix(output, "Error:") { + t.Errorf("Unexpected output: %v", output) + } + if !errors.Is(err, ErrUnknownSubcommand) { + t.Errorf("Unexpected error: %v", err) + } + if c.Name() != "child" { + t.Errorf(`invalid command returned from ExecuteC: expected "child"', got: %q`, c.Name()) + } +} + func TestExecuteContext(t *testing.T) { ctx := context.TODO() From 397434967568d7b2977c62a962aff80f8bb36c26 Mon Sep 17 00:00:00 2001 From: Andre Mueller Date: Mon, 8 Jul 2024 16:17:02 +0200 Subject: [PATCH 2/3] fix linting issue --- command_test.go | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/command_test.go b/command_test.go index 1012ca4ad..3d3afe01d 100644 --- a/command_test.go +++ b/command_test.go @@ -177,88 +177,92 @@ func TestSubcommandExecuteC(t *testing.T) { func TestSubcommandExecuteMissingSubcommand(t *testing.T) { rootCmd := &Command{Use: "root", Run: emptyRun} - childCmd := &Command{Use: "child", Run: nil, ErrorOnUnknownSubcommand: false} - child2Cmd := &Command{Use: "child2", Run: emptyRun} + const childName = "child" + const grandchildName = "grandchild" + childCmd := &Command{Use: childName, Run: nil, ErrorOnUnknownSubcommand: false} + child2Cmd := &Command{Use: grandchildName, Run: emptyRun} rootCmd.AddCommand(childCmd) childCmd.AddCommand(child2Cmd) // test existing command - c, output, err := executeCommandC(rootCmd, "child") + c, output, err := executeCommandC(rootCmd, childName) if !strings.HasPrefix(output, "Usage:") { t.Errorf("Unexpected output: %v", output) } if err != nil { t.Errorf("Unexpected error: %v", err) } - if c.Name() != "child" { + if c.Name() != childName { t.Errorf(`invalid command returned from ExecuteC: expected "child"', got: %q`, c.Name()) } // test existing sub command - c, output, err = executeCommandC(rootCmd, "child", "child2") + c, output, err = executeCommandC(rootCmd, childName, grandchildName) if output != "" { t.Errorf("Unexpected output: %v", output) } if err != nil { t.Errorf("Unexpected error: %v", err) } - if c.Name() != "child2" { - t.Errorf(`invalid command returned from ExecuteC: expected "child"', got: %q`, c.Name()) + if c.Name() != grandchildName { + t.Errorf(`invalid command returned from ExecuteC: expected "grandchild"', got: %q`, c.Name()) } // now test a command which does not exist, we will get no error, just "Usage:" is printed - c, output, err = executeCommandC(rootCmd, "child", "unknownChild") + c, output, err = executeCommandC(rootCmd, childName, "unknownChild") if !strings.HasPrefix(output, "Usage:") { t.Errorf("Expected: 'Usage: ...'\nGot:\n %q\n", output) } if err != nil { t.Errorf("Unexpected error: %v", err) } - if c.Name() != "child" { + if c.Name() != childName { t.Errorf(`invalid command returned from ExecuteC: expected "child"', got: %q`, c.Name()) } } func TestSubcommandExecuteMissingSubcommandWithErrorOnUnknownSubcommand(t *testing.T) { rootCmd := &Command{Use: "root", Run: emptyRun} - childCmd := &Command{Use: "child", Run: nil, ErrorOnUnknownSubcommand: true} - child2Cmd := &Command{Use: "child2", Run: emptyRun} + const childName = "child" + const grandchildName = "grandchild" + childCmd := &Command{Use: childName, Run: nil, ErrorOnUnknownSubcommand: true} + child2Cmd := &Command{Use: grandchildName, Run: emptyRun} rootCmd.AddCommand(childCmd) childCmd.AddCommand(child2Cmd) // test existing command - c, output, err := executeCommandC(rootCmd, "child") + c, output, err := executeCommandC(rootCmd, childName) if !strings.HasPrefix(output, "Error:") { t.Errorf("Unexpected output: %v", output) } if !errors.Is(err, ErrUnknownSubcommand) { t.Errorf("Unexpected error: %v", err) } - if c.Name() != "child" { + if c.Name() != childName { t.Errorf(`invalid command returned from ExecuteC: expected "child"', got: %q`, c.Name()) } // test existing sub command - c, output, err = executeCommandC(rootCmd, "child", "child2") + c, output, err = executeCommandC(rootCmd, childName, grandchildName) if output != "" { t.Errorf("Unexpected output: %v", output) } if err != nil { t.Errorf("Unexpected error: %v", err) } - if c.Name() != "child2" { + if c.Name() != grandchildName { t.Errorf(`invalid command returned from ExecuteC: expected "child"', got: %q`, c.Name()) } // now test a command which does not exist, we expect an error because of the ErrorOnUnknownSubcommand flag - c, output, err = executeCommandC(rootCmd, "child", "unknownChild") + c, output, err = executeCommandC(rootCmd, childName, "unknownChild") if !strings.HasPrefix(output, "Error:") { t.Errorf("Unexpected output: %v", output) } if !errors.Is(err, ErrUnknownSubcommand) { t.Errorf("Unexpected error: %v", err) } - if c.Name() != "child" { + if c.Name() != childName { t.Errorf(`invalid command returned from ExecuteC: expected "child"', got: %q`, c.Name()) } } From 133c6493b02a28abeb85993766ed8a6246c97133 Mon Sep 17 00:00:00 2001 From: Andre Mueller Date: Tue, 9 Jul 2024 07:40:03 +0200 Subject: [PATCH 3/3] moved to global EnableErrorOnUnknownSubcommand option --- cobra.go | 12 ++++++++++++ command.go | 13 +------------ command_test.go | 8 ++++++-- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/cobra.go b/cobra.go index e0b0947b0..67c5799e0 100644 --- a/cobra.go +++ b/cobra.go @@ -47,6 +47,7 @@ const ( defaultCommandSorting = true defaultCaseInsensitive = false defaultTraverseRunHooks = false + defaultErrorOnUnknownSubcommand = false ) // EnablePrefixMatching allows setting automatic prefix matching. Automatic prefix matching can be a dangerous thing @@ -65,6 +66,17 @@ var EnableCaseInsensitive = defaultCaseInsensitive // By default this is disabled, which means only the first run hook to be found is executed. var EnableTraverseRunHooks = defaultTraverseRunHooks +// EnableErrorOnUnknownSubcommand controls the behavior of subcommand handling. +// When the flag is set true the behavior of Command.Execute() will change: +// If a sub-subcommand is not found ErrUnknownSubcommand will be returned on calling +// Command.Exec() instead of the old behavior where a nil error was sent. +// If the flag is false (default) the old behavior is performed. +// For this behavior the child subcommand must be nil. +// Example: in root/service/run - there would be an existing subcommand `run` +// in root/service/unknown - there would be an unknown subcommand `unknown` therefore returning an error +// `service` must have a nil Command.Run() function for this. +var EnableErrorOnUnknownSubcommand = defaultErrorOnUnknownSubcommand + // MousetrapHelpText enables an information splash screen on Windows // if the CLI is started from explorer.exe. // To disable the mousetrap, just set this variable to blank string (""). diff --git a/command.go b/command.go index 489e0c3c8..7eaaedc8d 100644 --- a/command.go +++ b/command.go @@ -258,17 +258,6 @@ type Command struct { // SuggestionsMinimumDistance defines minimum levenshtein distance to display suggestions. // Must be > 0. SuggestionsMinimumDistance int - - // ErrorOnUnknownSubcommand controls the behavior of subcommand handling. - // When the flag is set true the behavior of Command.Execute() will change: - // If a sub-subcommand is not found ErrUnknownSubcommand will be returned on calling - // Command.Exec() instead of the old behavior where a nil error was sent. - // If the flag is false (default) the old behavior is performed. - // For this behavior the child subcommand must be nil. - // Example: in root/service/run - there would be an existing subcommand `run` - // in root/service/unknown - there would be an unknown subcommand `unknown` therefore returning an error - // `service` must have a nil Command.Run() function for this. - ErrorOnUnknownSubcommand bool } // Context returns underlying command context. If command was executed @@ -938,7 +927,7 @@ func (c *Command) execute(a []string) (err error) { } if !c.Runnable() { - if c.ErrorOnUnknownSubcommand { + if EnableErrorOnUnknownSubcommand { return ErrUnknownSubcommand } else { return flag.ErrHelp diff --git a/command_test.go b/command_test.go index 3d3afe01d..0bba7af57 100644 --- a/command_test.go +++ b/command_test.go @@ -179,7 +179,9 @@ func TestSubcommandExecuteMissingSubcommand(t *testing.T) { rootCmd := &Command{Use: "root", Run: emptyRun} const childName = "child" const grandchildName = "grandchild" - childCmd := &Command{Use: childName, Run: nil, ErrorOnUnknownSubcommand: false} + EnableErrorOnUnknownSubcommand = false + defer func() { EnableErrorOnUnknownSubcommand = defaultErrorOnUnknownSubcommand }() + childCmd := &Command{Use: childName, Run: nil} child2Cmd := &Command{Use: grandchildName, Run: emptyRun} rootCmd.AddCommand(childCmd) childCmd.AddCommand(child2Cmd) @@ -225,7 +227,9 @@ func TestSubcommandExecuteMissingSubcommandWithErrorOnUnknownSubcommand(t *testi rootCmd := &Command{Use: "root", Run: emptyRun} const childName = "child" const grandchildName = "grandchild" - childCmd := &Command{Use: childName, Run: nil, ErrorOnUnknownSubcommand: true} + EnableErrorOnUnknownSubcommand = true + defer func() { EnableErrorOnUnknownSubcommand = defaultErrorOnUnknownSubcommand }() + childCmd := &Command{Use: childName, Run: nil} child2Cmd := &Command{Use: grandchildName, Run: emptyRun} rootCmd.AddCommand(childCmd) childCmd.AddCommand(child2Cmd)