Improve help support for custom commands and aliases
Review Request #11552 — Created March 23, 2021 and submitted
rbtools supports adding custom commands by either finding a corresponding executable on $PATH beginning with 'rbt-' or by setting an alias. However, these custom commands do not appear when running 'rbt help'. Also, getting command-specific help does not work - neither 'rbt help new-command' nor 'rbt new-command --help'. This change fixes both of these shortcomings.
Tested locally on my own ReviewBoard instance with both commands on $PATH and aliases. I saw both listed in
rbt help
. I also testedrbt help <cmd>
andrbt <cmd> --help
for both commands on $PATH and aliases, and usage (or the alias string) was printed as expected.I did not run the automated rbtools test suite. \<excuses> It looked somewhat involved to get it set up (I'm a python newbie), and I wasn't sure whether the automated testing would actually hit my new code, anyway \</excuses>
Summary | ID |
---|---|
4d5771187e9822f9baa2fb305658c1e93d6fa4b8 |
Description | From | Last Updated |
---|---|---|
E501 line too long (80 > 79 characters) |
reviewbot | |
E231 missing whitespace after ',' |
reviewbot | |
E231 missing whitespace after ',' |
reviewbot | |
E501 line too long (80 > 79 characters) |
reviewbot | |
E231 missing whitespace after ',' |
reviewbot | |
E231 missing whitespace after ',' |
reviewbot | |
E501 line too long (80 > 79 characters) |
reviewbot | |
E231 missing whitespace after ',' |
reviewbot | |
E231 missing whitespace after ',' |
reviewbot | |
We prefer imports to be listed in alphabetical order. |
david | |
I think I'd prefer if the subprocess.call() and sys.exit() are separate statements. |
david | |
Instead of doing a whole bunch of list operations, then casting to set, then back to list, let's keep this … |
david | |
E126 continuation line over-indented for hanging indent |
reviewbot | |
E126 continuation line over-indented for hanging indent |
reviewbot | |
E126 continuation line over-indented for hanging indent |
reviewbot | |
E126 continuation line over-indented for hanging indent |
reviewbot | |
One last thing I noticed--since we have the commands as a set, we should be able to filter common_commands out … |
david |
- Commits:
-
Summary ID 20e0063be3f35d60659bc0e0c1943b1526acdeec f7d021829ec97078088ffad4552a106b6fbdec8c - Diff:
-
Revision 2 (+64 -2)
- Commits:
-
Summary ID f7d021829ec97078088ffad4552a106b6fbdec8c f7d021829ec97078088ffad4552a106b6fbdec8c - Diff:
-
Revision 3 (+64 -2)
- Change Summary:
-
sorry for the multiple diff uploads - it took me a while to realize that I was fixing the issues in a different repo than the one I was submitting from!
- Commits:
-
Summary ID f7d021829ec97078088ffad4552a106b6fbdec8c d841b3ef455ea99d8455c0ecd39c383b17a0fada - Diff:
-
Revision 4 (+64 -2)
Checks run (2 succeeded)
-
Thanks for the contribution! I've got a few comments mostly related to style.
Can you also detail what testing you've done for this change?
-
-
-
Instead of doing a whole bunch of list operations, then casting to set, then back to list, let's keep this all as a set until the end. We also like to have blank lines around indented blocks. Perhaps something like this?
commands = {entrypoint.name for entrypoint in entrypoints} for path_dir in os.environ.get('PATH').split(':'): path_prefix = '%s/%s-' % (path_dir, RB_MAIN) for cmd in glob.glob(path_prefix + '*'): commands.add(cmd.replace(path_prefix, '')) aliases = load_config.get('ALIASES', {}) commands |= aliases.keys() commands = list(commands)
- Change Summary:
-
Updated 'testing done'
- Testing Done:
-
+ Tested locally on my own ReviewBoard instance with both commands on $PATH and aliases. I saw both listed in
rbt help
. I also testedrbt help <cmd>
andrbt <cmd> --help
for both commands on $PATH and aliases, and usage (or the alias string) was printed as expected.+ + I did not run the automated rbtools test suite. \<excuses> It looked somewhat involved to get it set up (I'm a python newbie), and I wasn't sure whether the automated testing would actually hit my new code, anyway \</excuses>
- Change Summary:
-
Incorporating recommended changes
- Commits:
-
Summary ID d841b3ef455ea99d8455c0ecd39c383b17a0fada 319188bfa70aa4e1224ba35fdfc595bdbc0e10ae - Diff:
-
Revision 5 (+90 -14)
- Change Summary:
-
fixing flake8 errors
- Commits:
-
Summary ID 319188bfa70aa4e1224ba35fdfc595bdbc0e10ae 2d36a5925b5fd86679322feb34b6ec474e687877 - Diff:
-
Revision 6 (+90 -14)
- Change Summary:
-
fixing flake8 errors...again. It would be helpful if the error stated the expected intendation so I don't have to guess.
- Commits:
-
Summary ID 2d36a5925b5fd86679322feb34b6ec474e687877 5891d3df468881f663b12a97a789c3afb7f5324b - Diff:
-
Revision 7 (+90 -14)
Checks run (2 succeeded)
-
This is looking pretty solid!
-
One last thing I noticed--since we have the commands as a set, we should be able to filter
common_commands
out of it ahead of time. We probably can also skip the cast tolist
, sincesorted
should be able to handle it as-is:common_commands = ['post', 'patch', 'close', 'diff'] other_commands = commands - set(common_commands) print('\nThe most commonly used commands are:') for command in common_commands: print(' %s' % command) print ('\nOther commands:') for command in sorted(other_commands): print(' %s' % command)
- Change Summary:
-
Including suggestions from latest review
- Commits:
-
Summary ID 5891d3df468881f663b12a97a789c3afb7f5324b 828b7835cc639a47c40e4142a298d6b9e535a349 - Diff:
-
Revision 8 (+94 -20)
Checks run (2 succeeded)
- Change Summary:
-
rebased change to up-to-date master branch
- Commits:
-
Summary ID 828b7835cc639a47c40e4142a298d6b9e535a349 4d5771187e9822f9baa2fb305658c1e93d6fa4b8 - Diff:
-
Revision 9 (+94 -20)