Add aliases support for sub commands

[dcantah]

This adds support for aliases for subcommands via a new parameter to CommandConfiguration's constructors. The aliases are passed as an array of strings, where the default is just an empty array that signifies there are no aliases. The aliases are supported regardless of if a different commandName is chosen or not. This also updates how subcommands show up in the help text; any aliases are now displayed to the right of the original command.

In addition to the functionality itself, this change:

1. Updates some of the EndToEnd parsing tests to make sure they function while using aliases.
2. Sprinkles mentions where I saw fit in the documentation.
3. Updates the Math example to have aliases for math stats average (math stats avg), and math multiply (math mul).

math's help text now looks like the below:

~ math --help
OVERVIEW: A utility for performing maths.

USAGE: math <subcommand>

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

SUBCOMMANDS:
  add (default)           Print the sum of the values.
  multiply, mul           Print the product of the values.
  stats                   Calculate descriptive statistics.

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

~ math stats --help
OVERVIEW: Calculate descriptive statistics.

USAGE: math stats <subcommand>

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

SUBCOMMANDS:
  average, avg            Print the average of the values.
  stdev                   Print the standard deviation of the values.
  quantiles               Print the quantiles of the values (TBD).

  See 'math help stats <subcommand>' for detailed help.

and use of the aliases:

~ math mul 10 10
100

~ math stats avg 10 20
15.0

This change does NOT add any updates to the shell completion logic for this feature.

Fixes #248

Checklist

• I've added at least one test that validates that my change is working, if appropriate
• I've followed the code style of the rest of the project
• I've read the Contribution Guidelines
• I've updated the documentation if necessary

[rauhul]

What happens if conflicting command names and aliases are defined?

[dcantah]

@rauhul With the current implementation I think it'd basically be a no-op (but also just look dumb in the help text). The old tree parsing would stop if the current subcommand being checked against matched the value. The new logic just adds a case to check if the current value matches any of the aliases for that subcommand also. So fairly sure all that would happen is we'd take the first case as we would prior. Would you prefer to throw in some way (I'm leaning towards yes myself)?

[rauhul]

@rauhul With the current implementation I think it'd basically be a no-op (but also just look dumb in the help text). The old tree parsing would stop if the current subcommand being checked against matched the value. The new logic just adds a case to check if the current value matches any of the aliases for that subcommand also. So fairly sure all that would happen is we'd take the first case as we would prior. Would you prefer to throw in some way (I'm leaning towards yes myself)?

I would personal prefer an assertion failure or precondition failure, not sure which right now.

[dcantah]

Ack, I'll see if we have any similar validations and how they're handled for now

[natecook1000]

@dcantah Thanks for working on this! There's a set of validations that run in debug mode – this should have its own so that we don't end up with ambiguous commands. It should be okay to have duplicate names as long as they're in different parts of a command hierarchy, or we could disallow that as well to be clearer.

[dcantah]

A pretty decent spot to plop these checks are in the convenience constructor for Tree, this already has a InitializationError that handles if a sub command has a recursive entry of itself. Now, that seems much more fatal than our case, and indeed y'all use fatalError when catching the error, but this seems a decent spot to introduce a new error (whether it be fatal, precondition, assert etc.):

do {
    self.commandTree = try Tree(root: rootCommand)
} catch Tree<ParsableCommand.Type>.InitializationError.recursiveSubcommand(let command) {
    fatalError("The ParsableCommand \"\(command)\" can't have itself as its own subcommand.")
} catch Tree<ParsableCommand.Type>.InitializationError.aliasMatchingCommand(let command) {
    fatalError("The ParsableCommand \"\(command)\" can't have an alias with the same name as the command itself.")
} catch {
    fatalError("Unexpected error: \(error).")
}

The other case I'm thinking is there's really no reason to have aliases for the root command, so possibly another error here for that case. I can fix up the documentation for the above case, and this one if we agree on not allowing root commands to have aliases.

[dcantah]

Updated with a couple new tests to trial out not allowing an alias to match the commands name, as well as documented this behavior. I went with the approach we have right now for Tree initialization failures, which is this is treated as a fatalError. If we want to lessen this let me know

[natecook1000]

I think the situation we're looking to prevent is one subcommand's alias colliding with a sibling subcommand's name or alias (for example, two subcommands named subtract and substitute could both define a sub alias). The validator should do something like traversing the command hierarchy tree, collecting all the names/aliases of subcommands, and making sure there are no duplicates.

That said, I don't think we perform this validation right now for subcommand names! So I don't think this validation is a strict requirement here, and could be a usability improvement in a follow-up PR.

[dcantah]

Great point @natecook1000. I opened up a change here to add in general ambiguous subcommand name detection. This can be expanded upon for aliases fairly easily I think #629

[dcantah]

@swift-ci please test

[dcantah]

@rauhul Added this as we spoke about for source stability, let me know if I'm dumb and missed something.

[dcantah]

@natecook1000 Rauhul and I talked on this. I think it makes sense for this case to not be a validation and be a hard error given it won't break any existing programs.

[dcantah]

@swift-ci please test

[natecook1000]

Looks great! Thanks so much, @dcantah!