Introduce (optional) categories to group subcommands

[sahib]

For some larger programs it's sometimes useful to group commands into categories.
This patch should not alter normal output, but when filling out Command.Category,
it will be grouped accordingly.

Here's an example from ipfs:

    ipfs - global p2p merkle-dag filesystem

    ipfs [<flags>] <command> [<arg>] ...

    BASIC COMMANDS

        init          Initialize ipfs local configuration
        add <path>    Add an object to ipfs
        cat <ref>     Show ipfs object data
        get <ref>     Download ipfs objects
        ls <ref>      List links from an object
        refs <ref>    List hashes of links from an object

    DATA STRUCTURE COMMANDS

        block         Interact with raw blocks in the datastore
        object        Interact with raw dag nodes
        file          Interact with Unix filesystem objects

    ADVANCED COMMANDS

        daemon        Start a long-running daemon process
        mount         Mount an ipfs read-only mountpoint
        resolve       Resolve any type of name
        name          Publish or resolve IPNS names
        dns           Resolve DNS links
        pin           Pin objects to local storage
        repo gc       Garbage collect unpinned objects

    NETWORK COMMANDS

        id            Show info about ipfs peers
        bootstrap     Add or remove bootstrap peers
        swarm         Manage connections to the p2p network
        dht           Query the dht for values or peers
        ping          Measure the latency of a connection
        diag          Print diagnostics

    TOOL COMMANDS

        config        Manage configuration
        version       Show ipfs version information
        update        Download and apply go-ipfs updates
        commands      List all available commands

[tucnak]

Looks pretty great to me! I'd love you to provide an example code listing of this PR in use, so I could test it out locally and look for possible improvements?

[sahib]

Sure thing. Here's an example based on your official example:

https://gist.github.com/sahib/c7d141a5d467450c78ad

I also changed it a bit: It uses slices now to remember the order categories (and commands) were inserted - I guess, mostly you do not want MISC COMMANDS before GENERAL COMMANDS.

I guess the templating could be a bit prettier (first time I had to look a it...).
Also, Application.Commands is not directly used currently in this PR.

[tucnak]

I am really into what you've done here, but I am still not sure how this generally fits the feel of the tool itself. I am kind of antagonistic to the whole caps thing, it just looks a little bit alien to me. Aaand I haven't got a chance to deeply look into the code so far.

Wouldn't you mind if I take some time on reviewing this?

[sahib]

Take your time. The caps are not necessary to be honest, but were kinda convenient.
If I still want them, I can specify them in the Category field myself.

[tucnak]

Here is a draft diff (against b236233) I hacked a while ago: https://gist.github.com/tucnak/ef17042fa1b5a9d2363b

That's how I see it, but I am totally open to discussion. The diff above is just a draft, sort of PoC implementation. Code listing https://gist.github.com/tucnak/6ac7fd3dce564b21a8fb produces:

Demo is a funky demonstation of Climax capabilities.

Usage:

    demo command [arguments]

The commands are:

    # Actions

    perform     lorem ipsum dolor
    another     some other action
    google      meh, whatever

    # Observers

    watchers    liek some watchers
    monitors    these are different
    fancitors   meh, whatever

    # Misc

    help        find out more
    version     current build info
    authors     we are legion

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

[tucnak]

Btw, don't you know, am I anyhow able to push into your PR?

[sahib]

I like your draft better - I was afraid to add more API (like the extra Group struct) than necessary.
I'look at it later in more detail to see if it works out for my usecase.

Btw, don't you know, am I anyhow able to push into your PR?

I don't think that's possible directly.

Usually you would add my forked repo as origin, git fetch from it and merge it locally to a feature branch for review before merging it to master. Something like this (basically a manual PR):

$ git remote add sahib https://github.com/sahib/climax.git
$ git fetch sahib
$ git checkout -b feature/groups
$ git merge sahib/master
$ # play around and review it, possibly push it for others.
$ git checkout master && git merge feature/groups

(Sorry if I re-iterated on something you already knew)

[tucnak]

Fork-remote & fetch is exactly the way I'm doing this now. I was just wondering if it's possible to push to refs/pull/id/head, turns out it's not.

Anyway, looking forward to hearing some feedback on patch from you!

[sahib]

I like it. .AddGroup() is definitely future proof.

One thing I'd like to change is the formatting of the group headers, i.e. I would just let the user define how it's formatted. I could use caps (MISC COMMANDS:), your default could use (<tab># misc).

So basically, as some sort of mini-patch: 😄

-    # {{.Name}}
+{{.Name}}

P.S: go test failed after applying the patch.
There is a \t in the empty line after The commands are.

[tucnak]

Didn't touch tests at all so far. I believe you'd be able to tweak it, wouldn't you? Regarding the caps: I just don't think it fits the feel. Using caps'ed section names kind of pushes us towards using caps everywhere (e.g. USAGE, TOPICS, etc), otherwise it'd be inconsistent and inconsistency is not my thing. I still fully understand that we do need some sort of sectioning for bigger applications.

Why don't you like approach I proposed? I am asking cos it looks perfectly fine and just right to me.

[sahib]

I added the patch to my pull request and fixed the tests in the same run.
(Sorry for the merge commit, had to do since the patch was not based on the most recent commit).

Regarding caps: I guess that's a bit subjective - I like the look, since it separates the groups nicely.
I would be okay too without caps, but left-align the group headers for visual separation (just a # feels too weak for me). Therefore my proposal was to leave formatting to the user's liking.
"just right" might feel different another day (I speak from experience here...).

(Current PR includes the version with no formatting (like <tab>#) pre-defined).

[tucnak]

Alright, we could define some sort of a struct with a list of preferences.

type Preferences struct {
    // Defaults to false => weak separation, while
    // true breaks it into separate top-level sections.
    HardGroupSeparation bool

    // Allows to replace # with .e.g "//", which looks funky :)
    GroupSeparationSign string
}

So in case of hard group separation, The commands are: string should turn into The general commands: and this could be pretty much the format of section headers: The %name% commands:.

What do you think?

[sahib]

That would work out too, but is somewhat less simpler and is probably not be in the scope of this PR.
I would still favour the just let the user inject hist own formatting-approach for it's simplicity.

[tucnak]

Sorry, couldn't get in touch these days. Yuppy yeah, I love your current implementation, but it misses some documentation. Can you please add some so I could merge it in?

[sahib]

Aye, updated. See 0a8f769.