This change has been marked as completed.

Describe the completed change (optional):

Pushed to master (c3c2178)

Summary

Add base classes for multi-commands.

Review Request #11882 — Created Nov. 24, 2021 and submitted 3 years, 4 months ago — Latest diff uploaded 3 years, 4 months ago

Information

Owner

david*

Repository

RBTools

Branch

master

Bugs

Depends On

Blocks

11883

Reviewers

Groups

rbtools

People

Description*

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.

Testing Done

Used this extensively with the upcoming review command.

Changes between revision 2 and 3

orig

Commits

	Summary	ID	Author
	Add base classes for multi-commands. 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 `MultiCommand`s, 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. Testing Done: Used this extensively with the upcoming review command.	18a029ec319184099954b19bb70f435decd7cefc	David Trowbridge
	Add base classes for multi-commands. 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 `MultiCommand`s, 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. Testing Done: Used this extensively with the upcoming review command.	1b235b2d7d82590fa3b80f309ab4dff59df1408a	David Trowbridge

Files

rbtools/commands/__init__.py

-    Diff Revision 2
+    Diff Revision 3
                 argv (list, deprecated):
                     Unused legacy argument.
             """
             # First look for an existing group with the same name. This allows
-            # a SubCommand to merge review groups with its parent MultiCommand.
+            # a BaseSubCommand to merge review groups with its parent
             # BaseMultiCommand.
             for group in parser._action_groups:
                 if group.title == self.name:
                     break
             else:
                 group = parser.add_argument_group(self.name, self.description)
                 The parsed options.
             config (dict):
                 The loaded RBTools configuration.
             """
-            super(SubCommand, self).__init__(*args, **kwargs)
+            super(BaseSubCommand, self).__init__(*args, **kwargs)
             self.options = options
             self.config = config
     class BaseMultiCommand(Command):
         """Abstract base class for commands which offer subcommands.
         Some commands (such as :command:`rbt review`) want to offer many
         subcommands.
         """
         #: The available subcommands.
         #:
-        #: This is a list of SubCommand sub classes.
+        #: This is a list of BaseSubCommand sub classes.
         #:
         #: Type:
         #:     list
         subcommands = {}
             return parser
         def initialize(self):
             """Initialize the command."""
-            super(MultiCommand, self).initialize()
+            super(BaseMultiCommand, self).initialize()
             command = self.options.command_cls(options=self.options,
                                                config=self.config,
                                                transport_cls=self.transport_cls)
             command.stdout = self.stdout