diff --git a/command.go b/command.go index 9d5e9cf5e..6ff47dd5c 100644 --- a/command.go +++ b/command.go @@ -998,6 +998,10 @@ func (c *Command) ExecuteC() (cmd *Command, err error) { // initialize completion at the last point to allow for user overriding c.InitDefaultCompletionCmd() + // Now that all commands have been created, let's make sure all groups + // are properly created also + c.checkCommandGroups() + args := c.args // Workaround FAIL with "go test -v" or "cobra.test -test.v", see #155 @@ -1092,6 +1096,19 @@ func (c *Command) ValidateRequiredFlags() error { return nil } +// checkCommandGroups checks if a command has been added to a group that does not exists. +// If so, we panic because it indicates a coding error that should be corrected. +func (c *Command) checkCommandGroups() { + for _, sub := range c.commands { + // if Group is not defined let the developer know right away + if sub.GroupID != "" && !c.ContainsGroup(sub.GroupID) { + panic(fmt.Sprintf("group id '%s' is not defined for subcommand '%s'", sub.GroupID, sub.CommandPath())) + } + + sub.checkCommandGroups() + } +} + // InitDefaultHelpFlag adds default help flag to c. // It is called automatically by executing the c or by calling help and usage. // If c already has help flag, it will do nothing. @@ -1218,10 +1235,6 @@ func (c *Command) AddCommand(cmds ...*Command) { panic("Command can't be a child of itself") } cmds[i].parent = c - // if Group is not defined let the developer know right away - if x.GroupID != "" && !c.ContainsGroup(x.GroupID) { - panic(fmt.Sprintf("Group id '%s' is not defined for subcommand '%s'", x.GroupID, cmds[i].CommandPath())) - } // update max lengths usageLen := len(x.Use) if usageLen > c.commandsMaxUseLen { diff --git a/command_test.go b/command_test.go index 0adec93de..c023bd6ed 100644 --- a/command_test.go +++ b/command_test.go @@ -1862,6 +1862,84 @@ func TestAddGroup(t *testing.T) { checkStringContains(t, output, "\nTest group\n cmd") } +func TestWrongGroupFirstLevel(t *testing.T) { + var rootCmd = &Command{Use: "root", Short: "test", Run: emptyRun} + + rootCmd.AddGroup(&Group{ID: "group", Title: "Test group"}) + // Use the wrong group ID + rootCmd.AddCommand(&Command{Use: "cmd", GroupID: "wrong", Run: emptyRun}) + + defer func() { + if recover() == nil { + t.Errorf("The code should have panicked due to a missing group") + } + }() + _, err := executeCommand(rootCmd, "--help") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } +} + +func TestWrongGroupNestedLevel(t *testing.T) { + var rootCmd = &Command{Use: "root", Short: "test", Run: emptyRun} + var childCmd = &Command{Use: "child", Run: emptyRun} + rootCmd.AddCommand(childCmd) + + childCmd.AddGroup(&Group{ID: "group", Title: "Test group"}) + // Use the wrong group ID + childCmd.AddCommand(&Command{Use: "cmd", GroupID: "wrong", Run: emptyRun}) + + defer func() { + if recover() == nil { + t.Errorf("The code should have panicked due to a missing group") + } + }() + _, err := executeCommand(rootCmd, "child", "--help") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } +} + +func TestWrongGroupForHelp(t *testing.T) { + var rootCmd = &Command{Use: "root", Short: "test", Run: emptyRun} + var childCmd = &Command{Use: "child", Run: emptyRun} + rootCmd.AddCommand(childCmd) + + rootCmd.AddGroup(&Group{ID: "group", Title: "Test group"}) + // Use the wrong group ID + rootCmd.SetHelpCommandGroupID("wrong") + + defer func() { + if recover() == nil { + t.Errorf("The code should have panicked due to a missing group") + } + }() + _, err := executeCommand(rootCmd, "--help") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } +} + +func TestWrongGroupForCompletion(t *testing.T) { + var rootCmd = &Command{Use: "root", Short: "test", Run: emptyRun} + var childCmd = &Command{Use: "child", Run: emptyRun} + rootCmd.AddCommand(childCmd) + + rootCmd.AddGroup(&Group{ID: "group", Title: "Test group"}) + // Use the wrong group ID + rootCmd.SetCompletionCommandGroupID("wrong") + + defer func() { + if recover() == nil { + t.Errorf("The code should have panicked due to a missing group") + } + }() + _, err := executeCommand(rootCmd, "--help") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } +} + func TestSetOutput(t *testing.T) { c := &Command{} c.SetOutput(nil) diff --git a/user_guide.md b/user_guide.md index 977306aa8..e55367e85 100644 --- a/user_guide.md +++ b/user_guide.md @@ -492,10 +492,11 @@ around it. In fact, you can provide your own if you want. ### Grouping commands in help -Cobra supports grouping of available commands. Groups must be explicitly defined by `AddGroup` and set by -the `GroupId` element of a subcommand. The groups will appear in the same order as they are defined. -If you use the generated `help` or `completion` commands, you can set the group ids by `SetHelpCommandGroupId` -and `SetCompletionCommandGroupId`, respectively. +Cobra supports grouping of available commands in the help output. To group commands, each group must be explicitly +defined using `AddGroup()` on the parent command. Then a subcommand can be added to a group using the `GroupID` element +of that subcommand. The groups will appear in the help output in the same order as they are defined using different +calls to `AddGroup()`. If you use the generated `help` or `completion` commands, you can set their group ids using +`SetHelpCommandGroupId()` and `SetCompletionCommandGroupId()` on the root command, respectively. ### Defining your own help