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@xxxxxxxxx>
> ---
> 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
