Add groups for commands in help

[aawsome]

Adds the possibility to group the commands in the help message.

closes #836
closes #1271

[jharshman]

@aawsome I think this is a good addition. It's something I can put on the roadmap after 1.0.0 is released.

[CLAassistant]

All committers have signed the CLA.

[mhewedy]

Any updates on this? or any workarounds exist?

[github-actions]

This PR is being marked as stale due to a long period of inactivity

[aawsome]

I rebased this PR.
Now that 1.0.0 is out, is there anything I can do to get this PR merged?

[itskingori]

@aawsome Maybe we can nudge @jharshman for some visibility. I think should be able to provide some direction.

[umarcor]

@aawsome would you mind adding some tests for this feature?

[aawsome]

Added some tests. Almost all new code is covered by these tests.

The exception is AddCommandGroup when helpCommand is manually set.
(In this case most people would anyway directly specify the Group within the Command definition)

[aawsome]

Is there anything I can do to help getting this PR merged?

[nacx]

This has been open for a while and would be pretty nice to have this merged. Any chance anyone could have a look at it?

[itskingori]

@aawsome Branch has conflicts. Could you rebase? So that if it's ready by the time @jharshman has a another look?

[aawsome]

@aawsome Branch has conflicts. Could you rebase? So that if it's ready by the time @jharshman has a another look?

is rebased.

[yuvalsosman]

Hey, Any updates regarding this feature?

[umarcor]

@yuvalsosman, see #1496.

[MichaelMure]

Also close #1271

[marckhouzam]

Some small suggestions to cleanup a bit.

I haven't tested yet.

[marckhouzam]

I rebased this PR to the current master and made the mentioned changes.
I now get a linting error Error: struct of size 720 bytes could be of size 712 bytes (maligned) which I don't know how to solve.

If you move the new completionCommandGroup declaration slightly higher right under the declaration of helpCommandGroup, the linter error goes away. It has to do with memory alignment, although I haven't looked exactly at what was not well aligned.

And IMO I will add a test of SetCompletionCommandGroup.

Yes please.

[marckhouzam]

Ok, I like this a lot. Nice job @aawsome. I put a bunch of minor suggestions about improving comments and such, but you should just be able to accept them in Github which should be very easy.

I believe the only thing remaining is when a command is not part of a group. I have created the "Administration Commands:" and "System Commands:" groups but "forgot" to assign the login command to any group.
Currently we see this:

My CLI

Usage:
  prog [command]

Available Commands:
  login             Login to the platform

System Commands:
  config            Configuration for the CLI
  init              Initialize the CLI
  update            Update the CLI
  version           Version information

Administration Commands:
  completion        Generate the autocompletion script for the specified shell
  help              Help about any command

Flags:
  -h, --help   help for prog

I think the section "Available Commands:" looks bad.
However, I would personally be ok with leaving it as is and saying that it is up to developers to properly define their groups and not leave commands without a group.

@jpmcb Thoughts?

@aawsome I'm finished with the review. Whenever you find time, if you can update, and then we'll be pretty much ready to merge 🎉 .

[aawsome]

Thanks a lot for your review, @marckhouzam
I resolved all comments, added a test for the completion command and rebased this PR.
I hope now everything is fine 😉

[jpmcb]

I would personally be ok with leaving it as is and saying that it is up to developers to properly define their groups and not leave commands without a group.

I think this is should be the desired behavior since it essentially makes this feature backwards compatible: if someone doesn't define any groups, I'd expect this the help section to continue to work like it does today.

---

I've pondered the design decisions this PR carries and while I think this implementation is good, I wonder if this would be due for a refactor down the road since people may want a way to do more interesting things with groups beyond matching on strings (i.e., a group struct with a superset of cobra commands). But we'd need to see if people would even want that.

TLDR: I think this is good for now and we can merge it to get this behavior in

[marckhouzam]

I tested the latest version and it looked great.
But I kept being bothered by the "Additional commands:" section, mostly when it was empty.
So I went ahead and pushed a new commit that fixed it. This is the result.

If no groups are defined at all we remain backwards-compatible and get:

$ prog -h
My CLI

Usage:
  prog [command]

Available Commands:
  completion        Generate the autocompletion script for the specified shell
  config            Configuration for the CLI
  help              Help about any command
  version           Version information

Flags:
  -h, --help   help for prog

Use "prog [command] --help" for more information about a command.

If all subcommands have a group, we no longer get an empty "Available Commands:" line:

$ prog -h
My CLI

Usage:
  prog [command]

System
  config            Configuration for the CLI
  version           Version information

Administration
  completion        Generate the autocompletion script for the specified shell
  help              Help about any command

Flags:
  -h, --help   help for prog

Use "prog [command] --help" for more information about a command.

And if some commands have groups but some do not we get a new "Additional Commands:" section at the bottom:

$ prog -h
My CLI

Usage:
  prog [command]

System
  config            Configuration for the CLI

Administration
  version           Version information

Additional Commands:
  completion        Generate the autocompletion script for the specified shell
  help              Help about any command

Flags:
  -h, --help   help for prog

Use "prog [command] --help" for more information about a command.

@aawsome @jpmcb do you agree this is better? If not, we can just remove the top commit that I pushed.
If you like it, I'm ready to merge.

I wonder if this would be due for a refactor down the road since people may want a way to do more
interesting things with groups beyond matching on strings (i.e., a group struct with a superset of cobra commands).
But we'd need to see if people would even want that.

That is interesting and may indeed have potential for improvement. But I also can't tell at the moment what that would look like and if it will be in demand. So I agree with @jpmcb that we should go ahead with this version, which we know people want, and see where it takes us.

[aawsome]

So I went ahead and pushed a new commit that fixed it.

Just had a look at your commit. This is how I would have added these functionalities. So,your changes look good for me!

[jpmcb]

Amazing work @marckhouzam!!

[Airblader]

I'd just like to say thank you from a backseat watcher and user of the library to @aawsome for bringing this over the finish line even after such a long time! And of course everyone else who helped along the way.

[marckhouzam]

Yes thank you @aawsome ! You have shown great professionalism in what can be considered a frustrating situation 🙂

The community will reap the rewards 🎉

[Airblader]

If you don't mind, a follow-up question while adopting this: the groups appear in the order they are defined, but what about the commands in the group? Can we order them? It seems that currently this is alphabetical, which isn't usually very meaningful.

Edit: Just found

// EnableCommandSorting controls sorting of the slice of commands, which is turned on by default.
// To disable sorting, set it to false.
var EnableCommandSorting = defaultCommandSorting

so

cobra.EnableCommandSorting = false

does the trick. 👍