Add base classes for multi-commands.

Review Request #11882 — Created Nov. 24, 2021 and submitted

david
RBTools
master
11883
rbtools

We'll soon be adding two new commands, rbt api and rbt review, which
both want to implement additional subcommands (rbt api get,
rbt review publish, etc). This change adds the framework to do so,
consisting of two new base classes. SubCommand should be inherited for
each subcommand offered, and MultiCommand aggregates them together.
Arguments from the MultiCommand are added in to the subcommand
parsers.

The one tricky thing here was getting --help to do the right thing. In
the case of MultiCommands, we need to actually parse the arguments in
order to get the right subparser, but doing so was enforcing any usage
requirements. It turns out that the correct fix here was to allow
add_help to default to True in the parser initialization. This
allows us to call parse_args and have it not fail.

I did also have to move the logging setup out of the initialize
method, so that we wouldn't attach multiple log handlers.

Used this extensively with the upcoming review command.

Summary
Add base classes for multi-commands.
Description From Last Updated

Can we modernize the docs while here?

chipx86chipx86

Can we name this BaseSubCommand?

chipx86chipx86

Is this a default help text, or turning off help text? Can we add a comment/doc comment?

chipx86chipx86

This is missing a docstring.

chipx86chipx86

Can we name this BaseMultiCommand?

chipx86chipx86

Needs a double backtick, or it will be a reference. Or, ideally: :command:`rbt review `

chipx86chipx86

, optional

chipx86chipx86

What happens if this triggers an argument parsing error condition (like an invalid argument)? Those result in a sys.exit, which ...

chipx86chipx86

F821 undefined name 'SubCommand'

reviewbotreviewbot

F821 undefined name 'MultiCommand'

reviewbotreviewbot

"review groups" or "option groups"?

chipx86chipx86

Missing Args:, *args, and **kwargs.

chipx86chipx86

Should be in sorted alphabetical order (capitalized is first).

chipx86chipx86
chipx86
  1. 
      
  2. rbtools/commands/__init__.py (Diff revision 1)
     
     

    Can we modernize the docs while here?

  3. rbtools/commands/__init__.py (Diff revision 1)
     
     

    Is this a default help text, or turning off help text? Can we add a comment/doc comment?

  4. rbtools/commands/__init__.py (Diff revision 1)
     
     

    This is missing a docstring.

  5. rbtools/commands/__init__.py (Diff revision 1)
     
     

    Needs a double backtick, or it will be a reference. Or, ideally: :command:`rbt review `

  6. rbtools/commands/__init__.py (Diff revision 1)
     
     

    , optional

  7. rbtools/commands/main.py (Diff revision 1)
     
     
     

    What happens if this triggers an argument parsing error condition (like an invalid argument)? Those result in a sys.exit, which we'd have to catch (SystemExit). We'd probably have to catch ArgumentError as well.

    1. This is handled correctly:

      $ rbt review ax
      usage: rbt review  <subcommand> [options]
      
      Creates, updates, and edits reviews.
      rbt review: error: invalid choice: 'ax' (choose from 'edit', 'publish', 'add-diff-comment', 'add-general-comment', 'add-file-attachment-comment', 'discard')
      

      $ rbt review edit -r 1 --blah
      usage: rbt review  <subcommand> [options]
      
      Creates, updates, and edits reviews.
      rbt review: error: unrecognized arguments: --blah
      
  8. 
      
chipx86
  1. 
      
  2. rbtools/commands/__init__.py (Diff revision 1)
     
     

    Can we name this BaseSubCommand?

  3. rbtools/commands/__init__.py (Diff revision 1)
     
     

    Can we name this BaseMultiCommand?

  4. 
      
david
Review request changed

Commits:

Summary
-
Add base classes for multi-commands.
+
Add base classes for multi-commands.

Diff:

Revision 2 (+368 -20)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

david
chipx86
  1. 
      
  2. rbtools/commands/__init__.py (Diff revision 3)
     
     

    "review groups" or "option groups"?

  3. rbtools/commands/__init__.py (Diff revision 3)
     
     
     
     
     
     
     
     

    Missing Args:, *args, and **kwargs.

  4. rbtools/commands/main.py (Diff revision 3)
     
     
     
     
     

    Should be in sorted alphabetical order (capitalized is first).

  5. 
      
david
chipx86
  1. Ship It!
  2. 
      
david
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (c3c2178)
Loading...