cli bug fixes

lbrynet/extras/cli.py

@@ -115,20 +115,37 @@ def set_kwargs(parsed_args):
     return kwargs
 
 
+class ArgumentParser(argparse.ArgumentParser):
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, add_help=False, **kwargs)
+        self.add_argument(
+            '--help', dest='help', action='store_true', default=False,
+            help='show this help message and exit'
+        )
+
+
+def add_command_parser(parent, command):
+    subcommand = parent.add_parser(
+        command['name'],
+        help=command['doc'].strip().splitlines()[0]
+    )
+    subcommand.set_defaults(
+        api_method_name=command['api_method_name'],
+        command=command['name'],
+        doc=command['doc'],
+        replaced_by=command.get('replaced_by', None)
+    )
+
+
 def get_argument_parser():
-    main = argparse.ArgumentParser('lbrynet', add_help=False)
+    main = ArgumentParser('lbrynet')
     main.add_argument(
         '--version', dest='cli_version', action="store_true",
         help='Show lbrynet CLI version and exit.'
     )
-    main.add_argument(
+    main.set_defaults(group=None, command=None)
-        '-h', '--help', dest='help', action="store_true",
-        help='Show this help message and exit'
-    )
     CLIConfig.contribute_args(main)
-    sub = main.add_subparsers(dest='command')
+    sub = main.add_subparsers()
-    help = sub.add_parser('help', help='Detailed help for remote commands.')
-    help.add_argument('help_command', nargs='*')
     start = sub.add_parser('start', help='Start lbrynet server.')
     start.add_argument(
         '--quiet', dest='quiet', action="store_true",
@@ -139,19 +156,22 @@ def get_argument_parser():
         help=('Enable debug output. Optionally specify loggers for which debug output '
               'should selectively be applied.')
     )
+    start.set_defaults(command='start', start_parser=start)
     Config.contribute_args(start)
+
     api = Daemon.get_api_definitions()
-    for group in sorted(api):
+    groups = {}
-        group_command = sub.add_parser(group, help=api[group]['doc'])
+    for group_name in sorted(api['groups']):
-        group_command.set_defaults(group_doc=group_command)
+        group_parser = sub.add_parser(group_name, help=api['groups'][group_name])
-        if group in ('status', 'publish', 'version', 'help', 'wallet_balance', 'get'):
+        group_parser.set_defaults(group=group_name, group_parser=group_parser)
-            continue
+        groups[group_name] = group_parser.add_subparsers()
-        commands = group_command.add_subparsers(dest='subcommand')
+    for command_name in sorted(api['commands']):
-        for command in api[group]['commands']:
+        command = api['commands'][command_name]
-            commands.add_parser(command['name'], help=command['doc'].strip().splitlines()[0])
+        if command['group'] is None:
-    for deprecated in Daemon.deprecated_methods:
+            add_command_parser(sub, command)
-        group_command = sub.add_parser(deprecated)
+        else:
-        group_command.add_subparsers(dest='subcommand')
+            add_command_parser(groups[command['group']], command)
+
     return main
 
 
@@ -168,6 +188,10 @@ def main(argv=None):
 
     elif args.command == 'start':
 
+        if args.help:
+            args.start_parser.print_help()
+            return 0
+
         log_support.configure_logging(conf.log_file_path, not args.quiet, args.verbose)
 
         if conf.share_usage_data:
@@ -187,44 +211,25 @@ def main(argv=None):
         else:
             log.info("Not connected to internet, unable to start")
 
-    elif args.command == 'help':
-
-        if args.help_command:
-            method = '_'.join(args.help_command)
-        else:
-            parser.print_help()
-            return 0
-
-        if method not in Daemon.callable_methods:
-            print('Invalid command name: {method}')
-            return 1
-
-        fn = Daemon.callable_methods[method]
-        print(fn.__doc__)
-
     elif args.command is not None:
 
-        if args.command in ('status', 'publish', 'version', 'help', 'wallet_balance', 'get'):
+        doc = args.doc
-            method = args.command
+        api_method_name = args.api_method_name
-        elif args.subcommand is not None:
+        if args.replaced_by:
-            method = f'{args.command}_{args.subcommand}'
+            print(f"{args.api_method_name} is deprecated, using {args.replaced_by['api_method_name']}.")
+            doc = args.replaced_by['doc']
+            api_method_name = args.replaced_by['api_method_name']
+
+        if args.help:
+            print(doc)
         else:
-            args.group_doc.print_help()
+            parsed = docopt(doc, command_args)
-            return 0
+            params = set_kwargs(parsed)
+            loop = asyncio.get_event_loop()
+            loop.run_until_complete(execute_command(conf, api_method_name, params))
 
-        if method in Daemon.deprecated_methods:
+    elif args.group is not None:
-            new_method = Daemon.deprecated_methods[method].new_command
+        args.group_parser.print_help()
-            if new_method is None:
-                print(f"{method} is permanently deprecated and does not have a replacement command.")
-                return 0
-            print(f"{method} is deprecated, using {new_method}.")
-            method = new_method
-
-        fn = Daemon.callable_methods[method]
-        parsed = docopt(fn.__doc__, command_args)
-        params = set_kwargs(parsed)
-        loop = asyncio.get_event_loop()
-        loop.run_until_complete(execute_command(conf, method, params))
 
     else:
         parser.print_help()

lbrynet/extras/daemon/Daemon.py

@@ -399,21 +399,50 @@ class Daemon(metaclass=JSONRPCServerType):
 
     @classmethod
     def get_api_definitions(cls):
-        groups = {}
+        prefix = 'jsonrpc_'
-        for method in dir(cls):
+        skip = ['commands', 'help']
-            if method.startswith('jsonrpc_'):
+        not_grouped = ['block_show', 'report_bug', 'resolve_name', 'routing_table_get']
-                parts = method.split('_', 2)
+        api = {
-                group = command = parts[1]
+            'groups': {
-                if len(parts) == 3:
+                group_name[:-len('_DOC')].lower(): getattr(cls, group_name).strip()
-                    command = parts[2]
+                for group_name in dir(cls) if group_name.endswith('_DOC')
-                group_dict = {'doc': getattr(cls, f'{group.upper()}_DOC', ''), 'commands': []}
+            },
-                groups.setdefault(group, group_dict)['commands'].append({
+            'commands': {}
-                    'name': command,
+        }
-                    'doc': getattr(cls, method).__doc__
+        for jsonrpc_method in dir(cls):
-                })
+            if jsonrpc_method.startswith(prefix):
-        del groups['commands']
+                full_name = jsonrpc_method[len(prefix):]
-        del groups['help']
+                if full_name in skip:
-        return groups
+                    continue
+                method = getattr(cls, jsonrpc_method)
+                if full_name in not_grouped:
+                    name_parts = [full_name]
+                else:
+                    name_parts = full_name.split('_', 1)
+                if len(name_parts) == 1:
+                    group = None
+                    name, = name_parts
+                elif len(name_parts) == 2:
+                    group, name = name_parts
+                    assert group in api['groups'],\
+                        f"Group {group} does not have doc string for command {full_name}."
+                else:
+                    raise NameError(f'Could not parse method name: {jsonrpc_method}')
+                api['commands'][full_name] = {
+                    'api_method_name': full_name,
+                    'name': name,
+                    'group': group,
+                    'doc': method.__doc__,
+                    'method': method,
+                }
+                if hasattr(method, '_deprecated'):
+                    api['commands'][full_name]['replaced_by'] = method.new_command
+
+        for command in api['commands'].values():
+            if 'replaced_by' in command:
+                command['replaced_by'] = api['commands'][command['replaced_by']]
+
+        return api
 
     @property
     def db_revision_file_path(self):
@@ -1319,6 +1348,10 @@ class Daemon(metaclass=JSONRPCServerType):
         """
         return sorted([command for command in self.callable_methods.keys()])
 
+    WALLET_DOC = """
+    Wallet management.
+    """
+
     @deprecated("account_balance")
     def jsonrpc_wallet_balance(self, address=None):
         """ deprecated """
@@ -2249,6 +2282,9 @@ class Daemon(metaclass=JSONRPCServerType):
                 log.info("Deleted file: %s", file_name)
             return True
 
+    STREAM_DOC = """
+    Stream information.
+    """
     @requires(WALLET_COMPONENT, EXCHANGE_RATE_MANAGER_COMPONENT, BLOB_COMPONENT,
               DHT_COMPONENT, RATE_LIMITER_COMPONENT, PAYMENT_RATE_COMPONENT, DATABASE_COMPONENT,
               conditions=[WALLET_IS_UNLOCKED])
@@ -2909,7 +2945,7 @@ class Daemon(metaclass=JSONRPCServerType):
                     results[u]['claims_in_channel'] = resolved[u].get('claims_in_channel', [])
         return results
 
-    CHANNEL_DOC = """
+    TRANSACTION_DOC = """
     Transaction management.
     """
 
@@ -3147,6 +3183,10 @@ class Daemon(metaclass=JSONRPCServerType):
         await d2f(self.blob_manager.delete_blobs([blob_hash]))
         return "Deleted %s" % blob_hash
 
+    PEER_DOC = """
+    DHT / Blob Exchange peer commands.
+    """
+
     @requires(DHT_COMPONENT)
     async def jsonrpc_peer_list(self, blob_hash, timeout=None):
         """

tests/unit/test_cli.py

@@ -1,13 +1,25 @@
 import contextlib
 from io import StringIO
-from twisted.trial import unittest
+import unittest
 
+from docopt import DocoptExit
 from lbrynet.extras.cli import normalize_value, main
 from lbrynet.extras.system_info import get_platform
 
 
 class CLITest(unittest.TestCase):
 
+    @staticmethod
+    def shell(argv):
+        actual_output = StringIO()
+        with contextlib.redirect_stdout(actual_output):
+            with contextlib.redirect_stderr(actual_output):
+                try:
+                    main(argv)
+                except SystemExit as e:
+                    print(e.args[0])
+        return actual_output.getvalue().strip()
+
     def test_guess_type(self):
         self.assertEqual('0.3.8', normalize_value('0.3.8'))
         self.assertEqual('0.3', normalize_value('0.3'))
@@ -39,58 +51,65 @@ class CLITest(unittest.TestCase):
 
         self.assertEqual(3, normalize_value('3', key="some_other_thing"))
 
-    def test_help_command(self):
+    def test_help(self):
-        actual_output = StringIO()
+        self.assertIn(
-        with contextlib.redirect_stdout(actual_output):
+            'usage: lbrynet [--help] [--version] [--api API]', self.shell(['--help'])
-            main(['help'])
+        )
-        actual_output = actual_output.getvalue()
+        # start is special command, with separate help handling
-        self.assertSubstring('usage: lbrynet [--version] [-h]', actual_output)
+        self.assertIn(
-
+            '--share-usage-data', self.shell(['start', '--help'])
-    def test_help_for_command_command(self):
+        )
-        actual_output = StringIO()
+        # publish is ungrouped command, returns usage only implicitly
-        with contextlib.redirect_stdout(actual_output):
+        self.assertIn(
-            main(['help', 'publish'])
+            'publish (<name> | --name=<name>)', self.shell(['publish'])
-        actual_output = actual_output.getvalue()
+        )
-        self.assertSubstring('Make a new name claim and publish', actual_output)
+        # publish is ungrouped command, with explicit --help
-        self.assertSubstring('Usage:', actual_output)
+        self.assertIn(
-
+            'Make a new name claim and publish', self.shell(['publish', '--help'])
-    def test_help_for_command_command_with_invalid_command(self):
+        )
-        actual_output = StringIO()
+        # account is a group, returns help implicitly
-        with contextlib.redirect_stdout(actual_output):
+        self.assertIn(
-            main(['help', 'publish1'])
+            '{add,balance,create,decrypt,encrypt,fund,list,lock,max_address_gap,remove,send,set,unlock}',
-        self.assertSubstring('Invalid command name', actual_output.getvalue())
+            self.shell(['account'])
-
+        )
-    def test_version_command(self):
+        # account is a group, with explicit --help
-        actual_output = StringIO()
+        self.assertIn(
-        with contextlib.redirect_stdout(actual_output):
+            '{add,balance,create,decrypt,encrypt,fund,list,lock,max_address_gap,remove,send,set,unlock}',
-            main(['--version'])
+            self.shell(['account', '--help'])
-        self.assertEqual(
+        )
-            actual_output.getvalue().strip(),
+        # account add is a grouped command, returns usage implicitly
-            "lbrynet {lbrynet_version}".format(**get_platform())
+        self.assertIn(
+            'account_add (<account_name> | --account_name=<account_name>)',
+            self.shell(['account', 'add'])
+        )
+        # account add is a grouped command, with explicit --help
+        self.assertIn(
+            'Add a previously created account from a seed,', self.shell(['account', 'add', '--help'])
+        )
+        # help for invalid command, with explicit --help
+        self.assertIn(
+            "invalid choice: 'publish1'", self.shell(['publish1', '--help'])
+        )
+        # help for invalid command, implicit
+        self.assertIn(
+            "invalid choice: 'publish1'", self.shell(['publish1'])
         )
 
-    def test_invalid_command(self):
+    def test_version_command(self):
-        actual_output = StringIO()
+        self.assertEqual(
-        with contextlib.redirect_stderr(actual_output):
+            "lbrynet {lbrynet_version}".format(**get_platform()), self.shell(['--version'])
-            try:
+        )
-                main(['publish1'])
-            except SystemExit:
-                pass
-        self.assertSubstring("invalid choice: 'publish1'", actual_output.getvalue())
 
     def test_valid_command_daemon_not_started(self):
-        actual_output = StringIO()
-        with contextlib.redirect_stdout(actual_output):
-            main(["publish", '--name=asd', '--bid=99'])
         self.assertEqual(
-            actual_output.getvalue().strip(),
+            "Could not connect to daemon. Are you sure it's running?",
-            "Could not connect to daemon. Are you sure it's running?"
+            self.shell(["publish", '--name=asd', '--bid=99'])
         )
 
     def test_deprecated_command_daemon_not_started(self):
         actual_output = StringIO()
         with contextlib.redirect_stdout(actual_output):
-            main(["wallet_balance"])
+            main(["wallet", "balance"])
         self.assertEqual(
             actual_output.getvalue().strip(),
             "wallet_balance is deprecated, using account_balance.\n"