From 7455ae8c5842b97000a43d4b82dc331b5ba4bdad Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Mon, 15 Feb 2021 10:32:50 -0800 Subject: [PATCH] Output subcommand help on argument errors If the user supplied insufficient or incorrect arguments, output the help for the subcommand if it's available, otherwise output the main help. This addresses a sub-optmimal UX with the current code: * User runs "zuul-client", sees a help message with subcommands. * User runs "zuul-client --zuul-url=... encrypt" and gets an error message telling them to supply arguments that they never saw a help message for. They must then intuit that they need to run "zuul-client encrypt --help" to learn what those arguments are. With the new code, the user will: * Run "zuul-client" and see a help message with subcommands. * Run "zuul-client --zuul-url=... encrypt" and see the help message for the encrypt subcommand along with a message telling them they need to supply one of the required args. * Run "zuul-client --zuul-url=... encrypt project" and succeed. Change-Id: I4a32acfcc433ff38455703dd560ff60631fb29a5 --- zuulclient/cmd/__init__.py | 60 +++++++++++++++++++++++++------------- 1 file changed, 40 insertions(+), 20 deletions(-) diff --git a/zuulclient/cmd/__init__.py b/zuulclient/cmd/__init__.py index 65b1b05..fab2a77 100644 --- a/zuulclient/cmd/__init__.py +++ b/zuulclient/cmd/__init__.py @@ -95,10 +95,10 @@ class ZuulClient(): return subparsers def parseArguments(self, args=None): - parser = self.createParser() - self.args = parser.parse_args(args) + self.parser = self.createParser() + self.args = self.parser.parse_args(args) if not getattr(self.args, 'func', None): - parser.print_help() + self.parser.print_help() sys.exit(1) if self.args.func == self.enqueue_ref: # if oldrev or newrev is set, ensure they're not the same @@ -142,12 +142,23 @@ class ZuulClient(): logging.basicConfig(level=logging.DEBUG) def _main(self, args=None): - self.parseArguments(args) - if not self.args.zuul_url: - self.readConfig() - self.setup_logging() # TODO make func return specific return codes - if self.args.func(): + try: + self.parseArguments(args) + if not self.args.zuul_url: + self.readConfig() + self.setup_logging() + ret = self.args.func() + except ArgumentException: + if self.args.func: + name = self.args.func.__name__ + parser = getattr(self, 'cmd_' + name, self.parser) + else: + parser = self.parser + parser.print_help() + print() + raise + if ret: return 0 else: return 1 @@ -197,6 +208,7 @@ class ZuulClient(): '(default: scheduler\'s default_hold_expiration value)'), required=False, type=int) cmd_autohold.set_defaults(func=self.autohold) + self.cmd_autohold = cmd_autohold def autohold(self): if self.args.change and self.args.ref: @@ -227,6 +239,7 @@ class ZuulClient(): required=False, default='') cmd_autohold_delete.add_argument('id', metavar='REQUEST_ID', help='the hold request ID') + self.cmd_autohold_delete = cmd_autohold_delete def autohold_delete(self): client = self.get_client() @@ -241,6 +254,7 @@ class ZuulClient(): required=False, default='') cmd_autohold_info.add_argument('id', metavar='REQUEST_ID', help='the hold request ID') + self.cmd_autohold_info = cmd_autohold_info def autohold_info(self): client = self.get_client() @@ -271,6 +285,7 @@ class ZuulClient(): cmd_autohold_list.add_argument('--tenant', help='tenant name', required=False, default='') cmd_autohold_list.set_defaults(func=self.autohold_list) + self.cmd_autohold_list = cmd_autohold_list def autohold_list(self): client = self.get_client() @@ -312,6 +327,7 @@ class ZuulClient(): cmd_enqueue.add_argument('--change', help='change id', required=True) cmd_enqueue.set_defaults(func=self.enqueue) + self.cmd_enqueue = cmd_enqueue def enqueue(self): client = self.get_client() @@ -324,7 +340,7 @@ class ZuulClient(): return r def add_enqueue_ref_subparser(self, subparsers): - cmd_enqueue = subparsers.add_parser( + cmd_enqueue_ref = subparsers.add_parser( 'enqueue-ref', help='enqueue a ref', formatter_class=argparse.RawDescriptionHelpFormatter, description=textwrap.dedent('''\ @@ -333,19 +349,20 @@ class ZuulClient(): Directly enqueue a trigger event. This is usually used to manually "replay" a trigger received from an external source such as gerrit.''')) - cmd_enqueue.add_argument('--tenant', help='tenant name', - required=False, default='') - cmd_enqueue.add_argument('--pipeline', help='pipeline name', - required=True) - cmd_enqueue.add_argument('--project', help='project name', - required=True) - cmd_enqueue.add_argument('--ref', help='ref name', - required=True) - cmd_enqueue.add_argument( + cmd_enqueue_ref.add_argument('--tenant', help='tenant name', + required=False, default='') + cmd_enqueue_ref.add_argument('--pipeline', help='pipeline name', + required=True) + cmd_enqueue_ref.add_argument('--project', help='project name', + required=True) + cmd_enqueue_ref.add_argument('--ref', help='ref name', + required=True) + cmd_enqueue_ref.add_argument( '--oldrev', help='old revision', default=None) - cmd_enqueue.add_argument( + cmd_enqueue_ref.add_argument( '--newrev', help='new revision', default=None) - cmd_enqueue.set_defaults(func=self.enqueue_ref) + cmd_enqueue_ref.set_defaults(func=self.enqueue_ref) + self.cmd_enqueue_ref = cmd_enqueue_ref def enqueue_ref(self): client = self.get_client() @@ -374,6 +391,7 @@ class ZuulClient(): cmd_dequeue.add_argument('--ref', help='ref name', default=None) cmd_dequeue.set_defaults(func=self.dequeue) + self.cmd_dequeue = cmd_dequeue def dequeue(self): client = self.get_client() @@ -396,6 +414,7 @@ class ZuulClient(): cmd_promote.add_argument('--changes', help='change ids', required=True, nargs='+') cmd_promote.set_defaults(func=self.promote) + self.cmd_promote = cmd_promote def promote(self): client = self.get_client() @@ -483,6 +502,7 @@ class ZuulClient(): 'supplied, the value will be written ' 'to standard output.') cmd_encrypt.set_defaults(func=self.encrypt) + self.cmd_encrypt = cmd_encrypt def encrypt(self): if self.args.project is None and self.args.public_key is None: