| Message ID | 20220819160411.1791200-11-szeder.dev@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | aef7d75e5809eda765bbe407c7f8e0f8617f0fd0 |
| Headers | show |
| Series | parse-options: handle subcommands | expand |
On Fri, Aug 19 2022, SZEDER Gábor wrote: Nit: I wouldn't mind keeping this variable: > }; > - int result; > > argc = parse_options(argc, argv, prefix, options, builtin_bundle_usage, > - PARSE_OPT_STOP_AT_NON_OPTION); > + 0); > > packet_trace_identity("bundle"); > > - if (argc < 2) > - usage_with_options(builtin_bundle_usage, options); > - > - else if (!strcmp(argv[0], "create")) > - result = cmd_bundle_create(argc, argv, prefix); > - else if (!strcmp(argv[0], "verify")) > - result = cmd_bundle_verify(argc, argv, prefix); > - else if (!strcmp(argv[0], "list-heads")) > - result = cmd_bundle_list_heads(argc, argv, prefix); > - else if (!strcmp(argv[0], "unbundle")) > - result = cmd_bundle_unbundle(argc, argv, prefix); > - else { > - error(_("Unknown subcommand: %s"), argv[0]); > - usage_with_options(builtin_bundle_usage, options); > - } Then just doing: result = fn(argc, argv, prefix); Which would eliminate the need to change this: > - return result ? 1 : 0; > + return !!fn(argc, argv, prefix); > } I wondered about why !! v.s. 0/1 for a second or so, but realized you were just golf-ing an existing pattern. FWIW I *think* if we're changing this we could just make it "return fn()", as the functions themselves seem to return 0/1 or a !!'d variable.
diff --git a/builtin/bundle.c b/builtin/bundle.c index 2adad545a2..e80efce3a4 100644 --- a/builtin/bundle.c +++ b/builtin/bundle.c @@ -195,30 +195,19 @@ static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix) int cmd_bundle(int argc, const char **argv, const char *prefix) { + parse_opt_subcommand_fn *fn = NULL; struct option options[] = { + OPT_SUBCOMMAND("create", &fn, cmd_bundle_create), + OPT_SUBCOMMAND("verify", &fn, cmd_bundle_verify), + OPT_SUBCOMMAND("list-heads", &fn, cmd_bundle_list_heads), + OPT_SUBCOMMAND("unbundle", &fn, cmd_bundle_unbundle), OPT_END() }; - int result; argc = parse_options(argc, argv, prefix, options, builtin_bundle_usage, - PARSE_OPT_STOP_AT_NON_OPTION); + 0); packet_trace_identity("bundle"); - if (argc < 2) - usage_with_options(builtin_bundle_usage, options); - - else if (!strcmp(argv[0], "create")) - result = cmd_bundle_create(argc, argv, prefix); - else if (!strcmp(argv[0], "verify")) - result = cmd_bundle_verify(argc, argv, prefix); - else if (!strcmp(argv[0], "list-heads")) - result = cmd_bundle_list_heads(argc, argv, prefix); - else if (!strcmp(argv[0], "unbundle")) - result = cmd_bundle_unbundle(argc, argv, prefix); - else { - error(_("Unknown subcommand: %s"), argv[0]); - usage_with_options(builtin_bundle_usage, options); - } - return result ? 1 : 0; + return !!fn(argc, argv, prefix); }
'git bundle' parses its subcommands with a couple of if-else if statements. parse-options has just learned to parse subcommands, so let's use that facility instead, with the benefits of shorter code, handling missing or unknown subcommands, and listing subcommands for Bash completion. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- builtin/bundle.c | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-)