Re: [PATCH 2/3] parse-options: allow grouping subcommands

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> @@ -1432,35 +1432,39 @@ static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t
>  		}
>  
>  		pos = usage_indent(outfile);
> -		if (opts->short_name) {
> -			if (opts->flags & PARSE_OPT_NODASH)
> -				pos += fprintf(outfile, "%c", opts->short_name);
> -			else
> -				pos += fprintf(outfile, "-%c", opts->short_name);
> -		}
> -		if (opts->long_name && opts->short_name)
> -			pos += fprintf(outfile, ", ");
> -		if (opts->long_name) {
> -			const char *long_name = opts->long_name;
> -			if ((opts->flags & PARSE_OPT_NONEG) ||
> -			    skip_prefix(long_name, "no-", &positive_name))
> -				pos += fprintf(outfile, "--%s", long_name);
> -			else
> -				pos += fprintf(outfile, "--[no-]%s", long_name);
> -		}

It may have made it easier to follow if a preliminary step pushed
the above to a helper function.  It would have also prevented the
nesting becoming too deep as we see below.

> +		if (opts->type == OPTION_SUBCOMMAND) {
> +			pos += fprintf(outfile, "%s", opts->long_name);
> +		} else {
> +			if (opts->short_name) {
> +				if (opts->flags & PARSE_OPT_NODASH)
> +					pos += fprintf(outfile, "%c", opts->short_name);
> +				else
> +					pos += fprintf(outfile, "-%c", opts->short_name);
> +			}
> +			if (opts->long_name && opts->short_name)
> +				pos += fprintf(outfile, ", ");
> +			if (opts->long_name) {
> +				const char *long_name = opts->long_name;
> +				if ((opts->flags & PARSE_OPT_NONEG) ||
> +				    skip_prefix(long_name, "no-", &positive_name))
> +					pos += fprintf(outfile, "--%s", long_name);
> +				else

> diff --git a/parse-options.h b/parse-options.h
> index d7f896a933..5249404b46 100644
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -401,6 +401,13 @@ static char *parse_options_noop_ignored_value MAYBE_UNUSED;
>  	.subcommand_fn = (fn), \
>  }
>  #define OPT_SUBCOMMAND(l, v, fn)    OPT_SUBCOMMAND_F((l), (v), (fn), 0)
> +#define OPT_SUBCOMMAND_H(l, v, fn, h) { \
> +	.type = OPTION_SUBCOMMAND, \
> +	.long_name = (l), \
> +	.value = (v), \
> +	.help = (h), \
> +	.subcommand_fn = (fn), \
> +}

As presented, _F does not allow you to give it a help, and _H does
not allow you to give it a flag word.  I would have preferred to see
OPT_SUBCOMMAND_F to be extended to also take the help text, as we
only have two existing users in *.c code, rather than adding _H
variant that is incomplete and keeping _F incomplete.