Introduce subcommand grouping into the command configuration to improve help

[DougGregor]

Add optional support for grouping subcommands into named groups, to help bring order to commands with many subcommands without requiring additional structure. For example, here's the help for a "subgroupings" command that has an ungrouped subcommand (m), and two groups of subcommands ("broken" and "complicated").

USAGE: subgroupings <subcommand>

OPTIONS:
  -h, --help              Show help information.

SUBCOMMANDS:
  m

BROKEN SUBCOMMANDS:
  foo                     Perform some foo
  bar                     Perform bar operations

COMPLICATED SUBCOMMANDS:
  n

  See 'subgroupings help <subcommand>' for detailed help.

The help output above is created like this:

  struct WithSubgroups: ParsableCommand {
    static let configuration = CommandConfiguration(
      commandName: "subgroupings",
      subcommands: [ M.self ],
      groupedSubcommands: [
        CommandGroup(
          name: "Broken",
          subcommands: [ Foo.self, Bar.self ]
        ),
        CommandGroup(name: "Complicated", subcommands: [ N.self ]),
        CommandGroup(name: "Invisible", subcommands: [ ])
      ]
    )
  }

Each CommandGroup names a new group with its subcommands (there are no groups within groups).

This structure is only cosmetic, and only affects help generation by providing more structure for the reader. It doesn't impact existing clients, who can still reason about the flattened list of subcommands if they prefer.

[DougGregor]

@swift-ci please test

[dickoff]

I was just looking at doing something very similar! (WIP here: https://github.com/apple/swift-argument-parser/tree/subcommand-groups).

The CommandsBuilder idea is an interesting one. However is it limiting in some of the manipulation that you can do with the existing simple array subcommands parameter? Currently it's quite easy to do things like dynamically alphabetically sort the subcommands array or only include some commands based on #if checks. I checked out this branch and played around with a it a bit and it wasn't obvious to me how you could achieve those kinds of things with the builder (but I'm very inexperienced with resultBuilders!)

[dickoff]

Do we think there's value in allowing for a longer description string for the group? For example how subcommand groups are described in git -h. E.g. accomplishing something like this in the test, adding a description to the header title.

BROKEN SUBCOMMANDS: These commands are currently non-functional due to XYZ
  foo                     Perform some foo
  bar                     Perform bar operations

[DougGregor]

The CommandsBuilder idea is an interesting one. However is it limiting in some of the manipulation that you can do with the existing simple array subcommands parameter?

It shouldn't be too limiting. Result builders can support more language syntax if we'd like (e.g., the ability to do if #available and such), although I didn't add them.

I checked out this branch and played around with a it a bit and it wasn't obvious to me how you could achieve those kinds of things with the builder (but I'm very inexperienced with resultBuilders!)

I'd expect #if to work inside the builder already. If we want if, if-else, switch, and if #available to work, we can add that support to the builder by introducing the appropriate buildXYZ methods.

Do we think there's value in allowing for a longer description string for the group?

Yes, I think that's a good idea! It's easy enough to add an "abstract" that we put here.

[DougGregor]

@swift-ci please test

[DougGregor]

@dickoff See the latest two commits, which add an optional abstract and then extend out the result builders to handle all of the declarative syntax.

[DougGregor]

I've opened up a discussion thread on the forums for this change: https://forums.swift.org/t/grouping-subcommands/72219

[dickoff]

@dickoff See the latest two commits, which add an optional abstract and then extend out the result builders to handle all of the declarative syntax.

New changes look great! They resolve all the questions I brought up.

[rauhul]

I would recommend splitting out the abstract into a different PR. If we add this functionality to CommandGroups then we should probably add them OptionGroups and that feels like scope creep for this change.

[DougGregor]

could you add initializers that just accept arrays that are parallel to the builder-based initializers for CommandConfiguration and CommandGroup?

Hmm. If we're going to do this, I think I'd rather drop the builder-based initializers entirely from this pull request than introduce two new interfaces for the same thing. It's completely reasonable to say that the only way to get this functionality is with syntax like this:

subcommands: [
  // all of the ungrouped subcommands
],
groupedSubcommands: [
  CommandGroup(
    name: "group name",
    subcommands: [
      // subcommands in this group
    ]
  ),
  // ...
].

That avoids having to create the Subcommand type at all, and is arguably more in keeping with the current API design. It would leave the door open to a builder-based design in the future should we want it.

[natecook1000]

That makes sense! I could see that fitting into a whole builder-based configuration approach (which I think we've looked at before). If we make this change, let's also make flattenedSubcommands (or allSubcommands) public API as well, since the subcommands will live in two different places.

[dickoff]

That makes sense! I could see that fitting into a whole builder-based configuration approach (which I think we've looked at before). If we make this change, let's also make flattenedSubcommands (or allSubcommands) public API as well, since the subcommands will live in two different places.

Could we not just have the existing subcommands still return all ungrouped and grouped subcommands together? Having subcommands start to only return ungrouped commands would be odd for clients I would think.

[DougGregor]

Could we not just have the existing subcommands still return all ungrouped and grouped subcommands together?

I think we need subcommands to still return all ungrouped and grouped subcommands. I can add ungroupedSubcommands and groupedSubcommands for clients that want them separate.

[DougGregor]

@swift-ci please test

[DougGregor]

@natecook1000 I believe I addressed all of your comments. The updated PR description contains the non-result-builder interface, which is a more straightforward extension of the existing syntax:

  struct WithSubgroups: ParsableCommand {
    static let configuration = CommandConfiguration(
      commandName: "subgroupings",
      subcommands: [ M.self ],
      groupedSubcommands: [
        CommandGroup(
          name: "Broken",
          subcommands: [ Foo.self, Bar.self ]
        ),
        CommandGroup(name: "Complicated", subcommands: [ N.self ]),
        CommandGroup(name: "Invisible", subcommands: [ ])
      ]
    )
  }