Skip to content

Conversation

@jonathanpenn
Copy link
Contributor

@jonathanpenn jonathanpenn commented Mar 6, 2020

When using option groups to bring options into subcommands that are also
used in parent commands, the help system duplicates the options in the
generated output.

For instance, in the SubcommandEndToEndTest example, the --name
argument is listed twice for both the subcommands CommandA and
CommandB, and expected by the test assertion.

It is annoying in a simple case like this but causes a lot of unhelpful
noise in help output when you share more and more options across a
command heirarchy.

To solve this we need to keep track of what
HelpGenerator.Section.Element values have already been processed from
parent commands in the command stack. I acheived this by making
Element conform to Hashable to track in a Set.

This assumes that we don't need to support re-using command names up and
down a command heirarchy. It doesn't work at all today (you get an error
when trying to use the exact same option in a parent and child
command). If we choose to support this eventually then we'll need to
augment this solution to keep track of where the Element was generated
in the heirarchy.

This also updates the test to properly assert that the --name option
is only output once for the subcommands in SubcommandEndToEndTest.

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

When using option groups to bring options into subcommands that are also
used in parent commands, the help system duplicates the options in the
generated output.

For instance, in the `SubcommandEndToEndTest` example, the `--name`
argument is listed twice for both the subcommands `CommandA` and
`CommandB`, and expected by the test assertion.

It is annoying in a simple case like this but causes a lot of unhelpful
noise in help output when you share more and more options across a
command heirarchy.

To solve this we need to keep track of what
`HelpGenerator.Section.Element` values have already been processed from
parent commands in the command stack. I acheived this by making
`Element` conform to `Hashable` to track in a `Set`.

This assumes that we don't need to support re-using command names up and
down a command heirarchy. It doesn't work at all today (you get an error
when trying to use the exact same option in a parent and child
command). If we choose to support this eventually then we'll need to
augment this solution to keep track of where the `Element` was generated
in the heirarchy.

This also updates the test to properly assert that the `--name` option
is only output once for the subcommands in `SubcommandEndToEndTest`.
@natecook1000
Copy link
Member

This looks great, @jonathanpenn — how great that we already had a test case covering the incorrect behavior. 🙃

Eventually we want to add some command definition validation that would catch the duplicated option/flag names that you mentioned. We don't need to worry about that at this point.

@natecook1000 natecook1000 merged commit b5426db into apple:master Mar 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants