Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add remove group method for command #1997

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
24 changes: 24 additions & 0 deletions command.go
Expand Up @@ -1311,6 +1311,30 @@ func (c *Command) AddGroup(groups ...*Group) {
c.commandgroups = append(c.commandgroups, groups...)
}

// RemoveGroup removes one or more command groups to this parent command.
func (c *Command) RemoveGroup(groupIDs ...string) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i’m not sure, but I don’t think this will allow removing a group for the help or completion commands. Do we want to support removing groups for those two commands?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting use case. I guess in theory someone might want to remove those from a group that have been added. But they are also sort of the "default" commands you get. What do you think Marc?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, I would like cobra to give users as much freedom of development as possible. It is up to the developers to decide whether they want to keep the help and completion commands as existing groupies or remove them.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree and I'm 100% for freedom of use. It more just becomes a matter of not breaking long standing existing features. But this is new functionality anyways. So I'm good with it!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed as follows. If the group id passed as argument match, the group id of the help and completion command is reset in the RemoveGroup method. I would appreciate it if you check the code again.

// remove groups from commandgroups
groups := []*Group{}
main:
for _, group := range c.commandgroups {
for _, groupID := range groupIDs {
if group.ID == groupID {
continue main
}
}
groups = append(groups, group)
}
c.commandgroups = groups
// remove the groupID from the target commands
for _, command := range c.commands {
for _, groupID := range groupIDs {
if command.GroupID == groupID {
command.GroupID = ""
}
}
}
}

// RemoveCommand removes one or more commands from a parent command.
func (c *Command) RemoveCommand(cmds ...*Command) {
commands := []*Command{}
Expand Down
55 changes: 55 additions & 0 deletions command_test.go
Expand Up @@ -1862,6 +1862,61 @@ func TestAddGroup(t *testing.T) {
checkStringContains(t, output, "\nTest group\n cmd")
}

func TestRemoveSingleGroup(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding tests! Definitely makes it easier to validate!!

var rootCmd = &Command{Use: "root", Short: "test", Run: emptyRun}

rootCmd.AddGroup(
&Group{ID: "group", Title: "Test group"},
&Group{ID: "help", Title: "help"},
&Group{ID: "comp", Title: "comp"},
)

rootCmd.AddCommand(&Command{Use: "sub", GroupID: "group", Run: emptyRun})

rootCmd.SetHelpCommandGroupID("help")
rootCmd.SetCompletionCommandGroupID("comp")

rootCmd.RemoveGroup("group")

output, err := executeCommand(rootCmd, "--help")
if err != nil {
t.Errorf("Unexpected error: %v", err)
}

checkStringOmits(t, output, "\nTest group:\n sub")
checkStringContains(t, output, "\nAdditional Commands:\n sub")
}

func TestRemoveMultipleGroups(t *testing.T) {
var rootCmd = &Command{Use: "root", Short: "test", Run: emptyRun}

rootCmd.AddGroup(
&Group{ID: "group1", Title: "Test group1"},
&Group{ID: "group2", Title: "Test group2"},
&Group{ID: "help", Title: "help"},
&Group{ID: "comp", Title: "comp"},
)

rootCmd.AddCommand(
&Command{Use: "sub1", Short: "sub1", GroupID: "group1", Run: emptyRun},
&Command{Use: "sub2", Short: "sub2", GroupID: "group2", Run: emptyRun},
)

rootCmd.SetHelpCommandGroupID("help")
rootCmd.SetCompletionCommandGroupID("comp")

rootCmd.RemoveGroup("group1", "group2")

output, err := executeCommand(rootCmd, "--help")
if err != nil {
t.Errorf("Unexpected error: %v", err)
}

checkStringOmits(t, output, "\nTest group1:\n sub1")
checkStringOmits(t, output, "\nTest group2:\n sub2")
checkStringContains(t, output, "\nAdditional Commands:\n sub1 sub1\n sub2 sub2")
}

func TestWrongGroupFirstLevel(t *testing.T) {
var rootCmd = &Command{Use: "root", Short: "test", Run: emptyRun}

Expand Down
2 changes: 1 addition & 1 deletion site/content/user_guide.md
Expand Up @@ -525,7 +525,7 @@ around it. In fact, you can provide your own if you want.

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
of that subcommand. `RemoveGroup()` can be used to remove groups and initialize the GroupID of commands belonging to the removed group. The groups will appear in the help output in the same order as they are defined using different
JunNishimura marked this conversation as resolved.
Show resolved Hide resolved
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.

Expand Down