Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new HelpOption: Allow full subcommand flag display #306

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Commits on Mar 5, 2024

  1. feat: Allow full subcommand flag display

    By default, only `required` flags are displayed for subcommands
    
    For applications with a lot of simple subcommands this obscures basic usage flags
    
    == Mitigation
    
    - Add a new `HelpOption` called `IncludeOptionalFlagsInSummary`
    - Move `Node.Summary()` functionality into `Node.SummaryWithOptions(bool)`
        - Retain `Node.Summary()` as a default usage (forwards default
          value to `Node.SummaryWithOptions(bool)`)
    - Modify `help.go@printCommandSummary()` to forward the
       `HelpOption` to `Node.SummaryWithOptions()` instead of
       calling `Node.Summary()`
    - Add tests for default and optional behaviour
    
    == Caveats
    
    The main issue with this implementation is due to the `Node` receiver
    functions doing display logic internally.  This makes sense
    for most of the use cases (application error logic), but
    overly restricts the help configurability
    
    As a result:
    
    - Must pass awkward `bool` value down from the help context
    - `FlagSummary()` ends up having to explicitly exclude the `help` flag.
    
    == Potential improvements to be made
    
    - Remove explicit exclusion of `help` from `FlagSummary()`
        - Potentially add an equivalent `Node` receiver which just returns an
          array of flag summaries to be displayed by `help@printCommandSummary`
    keilin-anz authored and alecthomas committed Mar 5, 2024
    Configuration menu
    Copy the full SHA
    6140b4d View commit details
    Browse the repository at this point in the history
  2. fix: Improve subcommand optional flag summary logic

    - Adds `Parent` field to `Value`, allowing us to easily check whether
       a value originated with a given `Node`
    - Renames the new `HelpOption` to `SubcommandsWithOptionalFlags`
    - When the option is enabled
        - Groups required flags
        - Groups optional flags (by checking if the flag originated on
           the current Node)
        - Prints required flags followed by optional flags surrounded
           by brackets, eg.
           `--mandatory [ --optional1 --optional2 ]`
    - Removes janky check if a flag is `help`
        - Now just checks if the flag is our own
    - Added criteria to the test suite to confirm:
        - Ancestor's optional flags are not included
        - Ancestor's required flags are included
    keilin-anz authored and alecthomas committed Mar 5, 2024
    Configuration menu
    Copy the full SHA
    913732a View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    86b234f View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    2f0bdbb View commit details
    Browse the repository at this point in the history
  5. Update test to use new assert package

    original PR was from prior to the switch from `require` to `assert` for tests (in this commit 8b82618)
    
    Just change to using `assert` in keeping with the rebase
    keilin-anz authored and alecthomas committed Mar 5, 2024
    Configuration menu
    Copy the full SHA
    3957ac6 View commit details
    Browse the repository at this point in the history