Summary

Use parent parsers to define common multi-command arguments.

Review Request #12186 — Created March 21, 2022 and submitted March 21, 2022, 10:20 p.m.

Information

Owner

chipx86

Repository

RBTools

Branch

release-3.x

Bugs

Depends On

Reviewers

Groups

rbtools

People

Description

We now define a common option parser for the --help, --debug, and
--json global arguments, along with any common subparser options
defined in common_subcommand_option_list. This is shared by all
subcommands.

This isn't really fundamentally different than re-creating the options,
like we were doing before, but it does better define how options are
expected to be shared and reduces the work that must be done at argument
parser creation time.

Originally, the global options were also being shared with the main
argument parser, but this ended up causing problems where --json on
the main command would override that on the subcommand. This could be
solved by unsetting defaults for any global options and marking the
default as "suppress", but this was invasive (requiring other commands
to check for the presence of options.json_output) and probably not
worth doing. So for now, only subcommands share the options.

Testing Done

Tested all rbt review commands and options (along with an upcoming fix)
and the in-progress rbt status-update rewrite. Verified that all options
worked as expected.

Also checked the help output for each main command subcommand, checking
results.

Commits

Summary	ID
Use parent parsers to define common multi-command arguments. This updates the multi-command argument parsing code to leverage parent argparsers. This is a method of defining common sets of arguments that can be populated once up-front and then used by other argument parsers, avoiding duplication and in theory helping with option resolution. We now define a common option parser for the `--help`, `--debug`, and `--json` global arguments, along with any common subparser options defined in `common_subcommand_option_list`. This is shared by all subcommands. This isn't really fundamentally different than re-creating the options, like we were doing before, but it does better define how options are expected to be shared and reduces the work that must be done at argument parser creation time. Originally, the global options were also being shared with the main argument parser, but this ended up causing problems where `--json` on the main command would override that on the subcommand. This could be solved by unsetting defaults for any global options and marking the default as "suppress", but this was invasive (requiring other commands to check for the presence of `options.json_output`) and probably not worth doing. So for now, only subcommands share the options.	743786f6f92b9ac72c3d52172723551bfa28f621

Summary

Use parent parsers to define common multi-command arguments.

This updates the multi-command argument parsing code to leverage parent argparsers. This is a method of defining common sets of arguments that can be populated once up-front and then used by other argument parsers, avoiding duplication and in theory helping with option resolution. We now define a common option parser for the `--help`, `--debug`, and `--json` global arguments, along with any common subparser options defined in `common_subcommand_option_list`. This is shared by all subcommands. This isn't really fundamentally different than re-creating the options, like we were doing before, but it does better define how options are expected to be shared and reduces the work that must be done at argument parser creation time. Originally, the global options were also being shared with the main argument parser, but this ended up causing problems where `--json` on the main command would override that on the subcommand. This could be solved by unsetting defaults for any global options and marking the default as "suppress", but this was invasive (requiring other commands to check for the presence of `options.json_output`) and probably not worth doing. So for now, only subcommands share the options.

743786f6f92b9ac72c3d52172723551bfa28f621

Issues

Description	From	Last Updated
F821 undefined name 'globals_parser'	reviewbot	March 21, 2022, 10:17 p.m.

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Change Summary:

Removed the global options from the main parser, due to argparse handling them poorly.

Description:

		This updates the multi-command argument parsing code to leverage parent
		argparsers. This is a method of defining common sets of arguments that
		can be populated once up-front and then used by other argument parsers,
		avoiding duplication and in theory helping with option resolution.

~		We now define a global option parser for the `--help`, `--debug`, and
~		`--json` arguments. This is shared by the main command and all
	~	We now define a common option parser for the `--help`, `--debug`, and
	~	`--json` global arguments, along with any common subparser options
	+	defined in `common_subcommand_option_list`. This is shared by all
		subcommands.

-		Then we define a common subcommon parser for anything in
-		`common_subcommand_option_list`. This is shared by all subcommands.
-
		This isn't really fundamentally different than re-creating the options,
		like we were doing before, but it does better define how options are
		expected to be shared and reduces the work that must be done at argument
		parser creation time.
	+
	+	Originally, the global options were also being shared with the main
	+	argument parser, but this ended up causing problems where `--json` on
	+	the main command would override that on the subcommand. This could be
	+	solved by unsetting defaults for any global options and marking the
	+	default as "suppress", but this was invasive (requiring other commands
	+	to check for the presence of `options.json_output`) and probably not
	+	worth doing. So for now, only subcommands share the options.

Commits:

	Summary	ID
	Use parent parsers to define common multi-command arguments. This updates the multi-command argument parsing code to leverage parent argparsers. This is a method of defining common sets of arguments that can be populated once up-front and then used by other argument parsers, avoiding duplication and in theory helping with option resolution. We now define a global option parser for the `--help`, `--debug`, and `--json` arguments. This is shared by the main command and all subcommands. Then we define a common subcommon parser for anything in `common_subcommand_option_list`. This is shared by all subcommands. This isn't really fundamentally different than re-creating the options, like we were doing before, but it does better define how options are expected to be shared and reduces the work that must be done at argument parser creation time.	f465d0551130db03f20ce3c2348decb6573e9a3d
	Use parent parsers to define common multi-command arguments. This updates the multi-command argument parsing code to leverage parent argparsers. This is a method of defining common sets of arguments that can be populated once up-front and then used by other argument parsers, avoiding duplication and in theory helping with option resolution. We now define a common option parser for the `--help`, `--debug`, and `--json` global arguments, along with any common subparser options defined in `common_subcommand_option_list`. This is shared by all subcommands. This isn't really fundamentally different than re-creating the options, like we were doing before, but it does better define how options are expected to be shared and reduces the work that must be done at argument parser creation time. Originally, the global options were also being shared with the main argument parser, but this ended up causing problems where `--json` on the main command would override that on the subcommand. This could be solved by unsetting defaults for any global options and marking the default as "suppress", but this was invasive (requiring other commands to check for the presence of `options.json_output`) and probably not worth doing. So for now, only subcommands share the options.	8bca9441982c6412f2cc9bd99ca6fcfe095633c8

Diff:

Revision 2 (+40 -22)

Show changes

rbtools/commands/__init__.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

rbtools/commands/__init__.py (Diff revision 2)
The issue has been resolved. Show all issues
```
F821 undefined name 'globals_parser'
```

Change Summary:

Failed to git add -p one of the lines.

Commits:

	Summary	ID
	Use parent parsers to define common multi-command arguments. This updates the multi-command argument parsing code to leverage parent argparsers. This is a method of defining common sets of arguments that can be populated once up-front and then used by other argument parsers, avoiding duplication and in theory helping with option resolution. We now define a common option parser for the `--help`, `--debug`, and `--json` global arguments, along with any common subparser options defined in `common_subcommand_option_list`. This is shared by all subcommands. This isn't really fundamentally different than re-creating the options, like we were doing before, but it does better define how options are expected to be shared and reduces the work that must be done at argument parser creation time. Originally, the global options were also being shared with the main argument parser, but this ended up causing problems where `--json` on the main command would override that on the subcommand. This could be solved by unsetting defaults for any global options and marking the default as "suppress", but this was invasive (requiring other commands to check for the presence of `options.json_output`) and probably not worth doing. So for now, only subcommands share the options.	8bca9441982c6412f2cc9bd99ca6fcfe095633c8
	Use parent parsers to define common multi-command arguments. This updates the multi-command argument parsing code to leverage parent argparsers. This is a method of defining common sets of arguments that can be populated once up-front and then used by other argument parsers, avoiding duplication and in theory helping with option resolution. We now define a common option parser for the `--help`, `--debug`, and `--json` global arguments, along with any common subparser options defined in `common_subcommand_option_list`. This is shared by all subcommands. This isn't really fundamentally different than re-creating the options, like we were doing before, but it does better define how options are expected to be shared and reduces the work that must be done at argument parser creation time. Originally, the global options were also being shared with the main argument parser, but this ended up causing problems where `--json` on the main command would override that on the subcommand. This could be solved by unsetting defaults for any global options and marking the default as "suppress", but this was invasive (requiring other commands to check for the presence of `options.json_output`) and probably not worth doing. So for now, only subcommands share the options.	743786f6f92b9ac72c3d52172723551bfa28f621

Diff:

Revision 3 (+36 -20)

Show changes

rbtools/commands/__init__.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-3.x (0deee82)