Improve help support for custom commands and aliases

Review Request #11552 — Created March 23, 2021 and submitted

Information

RBTools
master

Reviewers

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 tested rbt help <cmd> and rbt <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
Improve help support for custom commands and aliases
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.
4d5771187e9822f9baa2fb305658c1e93d6fa4b8
Description From Last Updated

E501 line too long (80 > 79 characters)

reviewbotreviewbot

E231 missing whitespace after ','

reviewbotreviewbot

E231 missing whitespace after ','

reviewbotreviewbot

E501 line too long (80 > 79 characters)

reviewbotreviewbot

E231 missing whitespace after ','

reviewbotreviewbot

E231 missing whitespace after ','

reviewbotreviewbot

E501 line too long (80 > 79 characters)

reviewbotreviewbot

E231 missing whitespace after ','

reviewbotreviewbot

E231 missing whitespace after ','

reviewbotreviewbot

We prefer imports to be listed in alphabetical order.

daviddavid

I think I'd prefer if the subprocess.call() and sys.exit() are separate statements.

daviddavid

Instead of doing a whole bunch of list operations, then casting to set, then back to list, let's keep this …

daviddavid

E126 continuation line over-indented for hanging indent

reviewbotreviewbot

E126 continuation line over-indented for hanging indent

reviewbotreviewbot

E126 continuation line over-indented for hanging indent

reviewbotreviewbot

E126 continuation line over-indented for hanging indent

reviewbotreviewbot

One last thing I noticed--since we have the commands as a set, we should be able to filter common_commands out …

daviddavid
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

mblythe
Review request changed

Commits:

Summary ID
Improve help support for custom commands and aliases
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.
20e0063be3f35d60659bc0e0c1943b1526acdeec
Improve help support for custom commands and aliases
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.
f7d021829ec97078088ffad4552a106b6fbdec8c

Diff:

Revision 2 (+64 -2)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

mblythe
Review request changed

Commits:

Summary ID
Improve help support for custom commands and aliases
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.
f7d021829ec97078088ffad4552a106b6fbdec8c
Improve help support for custom commands and aliases
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.
f7d021829ec97078088ffad4552a106b6fbdec8c

Diff:

Revision 3 (+64 -2)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

mblythe
david
  1. 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?

  2. rbtools/commands/main.py (Diff revision 4)
     
     
    Show all issues

    We prefer imports to be listed in alphabetical order.

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

    I think I'd prefer if the subprocess.call() and sys.exit() are separate statements.

  4. rbtools/commands/main.py (Diff revision 4)
     
     
     
     
     
     
     
    Show all issues

    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)
    
  5. 
      
mblythe
  1. 
      
  2. rbtools/commands/main.py (Diff revision 4)
     
     
     
     
     
     
     

    Regarding separating the sys.exit and subprocess.call - I was copying the style from here. Would you like this one separated as well?

    1. Yes, that would be good.

  3. 
      
mblythe
mblythe
Review request changed

Change Summary:

Incorporating recommended changes

Commits:

Summary ID
Improve help support for custom commands and aliases
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.
d841b3ef455ea99d8455c0ecd39c383b17a0fada
Improve help support for custom commands and aliases
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.
319188bfa70aa4e1224ba35fdfc595bdbc0e10ae

Diff:

Revision 5 (+90 -14)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

mblythe
Review request changed

Change Summary:

fixing flake8 errors

Commits:

Summary ID
Improve help support for custom commands and aliases
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.
319188bfa70aa4e1224ba35fdfc595bdbc0e10ae
Improve help support for custom commands and aliases
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.
2d36a5925b5fd86679322feb34b6ec474e687877

Diff:

Revision 6 (+90 -14)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

mblythe
mblythe
  1. 
      
  2. Is there anything else you need from me?

  3. 
      
david
  1. This is looking pretty solid!

  2. rbtools/commands/main.py (Diff revision 7)
     
     
     
    Show all issues

    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 to list, since sorted 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)
    
  3. 
      
mblythe
mblythe
david
  1. Ship It!
  2. 
      
mblythe
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-2.0.x (b9e4c9b)
Loading...