Re: [PATCH 15/20] builtin/notes.c: let parse-options parse subcommands

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 25, 2022, 16:49 UTC
Message-ID: <xmqq7d416sts.fsf@gitster.g>
In-Reply-To: <20220725123857.2773963-16-szeder.dev@gmail.com>

SZEDER Gábor <szeder.dev@gmail.com> writes:

Show 38 quoted lines

> 'git notes' parses its subcommands with a long list 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 unknown subcommands, and listing subcommands for Bash
> completion.  Make sure that the default operation mode doesn't accept
> any arguments.
>
> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
> ---
>  builtin/notes.c | 43 +++++++++++++++++--------------------------
>  1 file changed, 17 insertions(+), 26 deletions(-)
>
> diff --git a/builtin/notes.c b/builtin/notes.c
> index a3d0d15a22..42cbae4659 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -994,17 +994,31 @@ static int get_ref(int argc, const char **argv, const char *prefix)
>  
>  int cmd_notes(int argc, const char **argv, const char *prefix)
>  {
> -	int result;
>  	const char *override_notes_ref = NULL;
> +	parse_opt_subcommand_fn *fn = list;
>  	struct option options[] = {
>  		OPT_STRING(0, "ref", &override_notes_ref, N_("notes-ref"),
>  			   N_("use notes from <notes-ref>")),
> +		OPT_SUBCOMMAND("list", &fn, list),
> +		OPT_SUBCOMMAND("add", &fn, add),
> +		OPT_SUBCOMMAND("copy", &fn, copy),
> +		OPT_SUBCOMMAND("append", &fn, append_edit),
> +		OPT_SUBCOMMAND("edit", &fn, append_edit),
> +		OPT_SUBCOMMAND("show", &fn, show),
> +		OPT_SUBCOMMAND("merge", &fn, merge),
> +		OPT_SUBCOMMAND("remove", &fn, remove_cmd),
> +		OPT_SUBCOMMAND("prune", &fn, prune),
> +		OPT_SUBCOMMAND("get-ref", &fn, get_ref),
>  		OPT_END()
>  	};

Reading the series backwards from the end, I would expect that the above to replicate the current behaviour to allow commands to have both "command wide" options and "per subcommand" options, e.g.

    $ git notes get-ref --ref=amlog
    error: unknown option `ref=amlog`
    usage: git notes get-ref
    $ git notes --ref=amlog get-ref
    refs/notes/amlog

Assuming that is how the new OPT_SUBCOMMAND() interacts with the dashed options in a single "struct option []", everything I saw so far in [10-20/20] makes sense. [17/20] has another instance of the above, dashed-options and subcommands mixed in an array, to parse the option for "git remote --verbose <subcmd>" that applies to all subcommands.

Thanks.