Grouping subcommands

Hey all,

In a discussion on adding another swift package subcommand, we came across the idea of "grouping" common subcommands under a common heading. This can improve the readability of the generated "help" when there are lots of subcommands.

I implemented support for this in a pull request that extends CommandConfiguration with support for naming groups of subcommands along with other commands. I ended up using result builders, because it felt reasonable. Here's a (partial!) example for swift package:

     public static var configuration = CommandConfiguration(
        commandName: "package",
        _superCommandName: "swift",
        abstract: "Perform operations on Swift packages",
        discussion: "SEE ALSO: swift build, swift run, swift test",
        version: SwiftVersion.current.completeDisplayString) {
            CommandGroup(name: "Manifest editing") {
              AddDependency.self
              AddProduct.self
              AddTarget.self
            )

            Clean.self
            PurgeCache.self

            CommandGroup(name: "Package installation") {
              Install.self
              Uninstall.self
            }
        }

The result will be that ungrouped subcommands come first, followed by each of the command groups (in order they are specified), e.g.,

SUBCOMMANDS:
  clean                   Delete build artifacts
  purge-cache             Purge the global repository cache.
  reset                   Reset the complete cache/build directory
  update                  Update package dependencies
  describe                Describe the current package
  init                    Initialize a new package

MANIFEST EDITING SUBCOMMANDS:
  add-dependency          Add a package dependency to the manifest
  add-target              Add a new target to the manifest

PACKAGE INSTALLATION SUBCOMMANDS:
  experimental-install    Offers the ability to install executable products of
                          the current package.
  experimental-uninstall  Offers the ability to uninstall executable products
                          previously installed by `swift package
                          experimental-install`.

My change itself is backward-compatible, so one can still create custom configurations and pass the flat subcommands array directly.

Thoughts?

Doug

14 Likes

I like the idea a lot, but I am wondering if the term "Section" would be a better fit. Its a closer match to what SwiftUI calls the visual organization of content into separate sections and also leaves the door open to use the term "Group" for other future features that support the sharing of properties and functionality.

3 Likes

I think this is an excellent and valuable extension.

The only thing that seems imperfect, on first blush at least, is the trailing closure syntax. It works, technically, but it's a bit awkward re. the multiple levels of indentation (and the extra punctuation re. curly braces).

Could CommandConfiguration instead take a CommandGroup... as its final argument? i.e. variadic parameter.

1 Like

That wouldn't be quite equivalent, because the elements in the top-level result builder can be a mixture of CommandGroups and ParsableCommands. So a variadic argument would have to introduce a common protocol for both types, or introduce an explicit "unnamed/default" command group, and you'd lose the ability to easily conditionalize groups directly in the CommandConfiguration—the caller would have to hoist the construction of the list of CommandGroups to a helper, which means then you'd have to lose the variadic syntax (or at least have to provide another overload) because you'd need to take arbitrary arrays vs. just a known-at-compile-time variadic list.

Given the "declarative" nature of this kind of configuration, I think the result builder helps a lot more than it harms here.

4 Likes

Right. It gives some flexibility that isn't available within an array literal (e.g., to use if/else) and reduces ceremony somewhat. I considered the alternative of keeping the same "subcommands" but generalizing the list to take some value of type any CommandOrGroup for each element. It could look like this:

     public static var configuration = CommandConfiguration(
        commandName: "package",
        _superCommandName: "swift",
        abstract: "Perform operations on Swift packages",
        discussion: "SEE ALSO: swift build, swift run, swift test",
        version: SwiftVersion.current.completeDisplayString,
        subcommands: [
            CommandGroup(
              name: "Manifest editing",
              subcommands: [
                AddDependency.self,
                AddProduct.self,
                AddTarget.self
              ]
            ),
            Clean.self,
            PurgeCache.self,
            CommandGroup(
              name: "Package installation",
              subcommands: [
                Install.self,
                Uninstall.self
              ]
            )
         )

The result builder felt cleaner to me than the nested array literals, but either could work. It's also a little weird that the subcommands argument would have a different type from the subcommands property, because the former would be generalized (to [any CommandOrGroup]) while the latter needs to remain [any ParsableCommand.Type] for source-compatibility reasons.

Doug

3 Likes

Personally, I like the explicitness of calling this CommandGroup, and wouldn't expect this little result builder to grow much in generality. It's not the rendering of documentation that we're doing here, it's setting up the subcommand structure.

Doug

3 Likes

We use the term "Group" in the "OptionGroup" API and I think its best to align with the existing names.

2 Likes

Based on more feedback from @nnnnnnnn on the pull request itself, I'm going to revise my suggestion to make it a smaller change to the API. Specifically, adding groupedSubcommands: [CommandGroup] = [] to the CommandConfiguration initializer. So my example would look like this:

     public static var configuration = CommandConfiguration(
        commandName: "package",
        _superCommandName: "swift",
        abstract: "Perform operations on Swift packages",
        discussion: "SEE ALSO: swift build, swift run, swift test",
        version: SwiftVersion.current.completeDisplayString,
        subcommands: [
            Clean.self,
            PurgeCache.self
        ],
        groupedSubcommands: [
            CommandGroup(
              name: "Manifest editing",
              subcommands: [
                AddDependency.self,
                AddProduct.self,
                AddTarget.self
              ]
            ),
            CommandGroup(
              name: "Package installation",
              subcommands: [
                Install.self,
                Uninstall.self
              ]
            )
         ]
    )

Doug

3 Likes