• 
      

    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

    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

    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

    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

    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:
    Completed
    Change Summary:
    Pushed to release-2.0.x (b9e4c9b)