| Message ID | patch-5.6-7acb4bd75ce-20210720T113707Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | commit-graph: usage fixes | expand |
On Tue, Jul 20, 2021 at 01:39:44PM +0200, Ævar Arnfjörð Bjarmason wrote: > Rather than guarding all of the !argc with an additional "if" arm > let's do an early goto to "usage". This also makes it clear that > "save_commit_buffer" is not needed in this case. > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > builtin/commit-graph.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c > index 6e49184439f..bf34aa43f22 100644 > --- a/builtin/commit-graph.c > +++ b/builtin/commit-graph.c > @@ -331,16 +331,17 @@ int cmd_commit_graph(int argc, const char **argv, const char *prefix) > builtin_commit_graph_options, > builtin_commit_graph_usage, > PARSE_OPT_STOP_AT_NON_OPTION); > + if (!argc) > + goto usage; > > save_commit_buffer = 0; > > - if (argc > 0) { > - if (!strcmp(argv[0], "verify")) > - return graph_verify(argc, argv); > - if (!strcmp(argv[0], "write")) > - return graph_write(argc, argv); > - } > + if (!strcmp(argv[0], "verify")) > + return graph_verify(argc, argv); > + else if (argc && !strcmp(argv[0], "write")) > + return graph_write(argc, argv); Nice, what you wrote here is definitely an improvement in readability. This could potentially also have an else like I suggested in the multi-pack-index patch earlier (later?) in the thread. Maybe something like: else if (strcmp(argv[0], "...")) return graph_xyz(...); else error(_("unrecognized subcommand: %s"), argv[0]); usage: usage_with_options(...); Arguably that could be done in this patch, separately, or not at all ;). Thanks, Taylor
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index 6e49184439f..bf34aa43f22 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -331,16 +331,17 @@ int cmd_commit_graph(int argc, const char **argv, const char *prefix) builtin_commit_graph_options, builtin_commit_graph_usage, PARSE_OPT_STOP_AT_NON_OPTION); + if (!argc) + goto usage; save_commit_buffer = 0; - if (argc > 0) { - if (!strcmp(argv[0], "verify")) - return graph_verify(argc, argv); - if (!strcmp(argv[0], "write")) - return graph_write(argc, argv); - } + if (!strcmp(argv[0], "verify")) + return graph_verify(argc, argv); + else if (argc && !strcmp(argv[0], "write")) + return graph_write(argc, argv); +usage: usage_with_options(builtin_commit_graph_usage, builtin_commit_graph_options); }
Rather than guarding all of the !argc with an additional "if" arm let's do an early goto to "usage". This also makes it clear that "save_commit_buffer" is not needed in this case. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- builtin/commit-graph.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)