Re: [PATCH 1/3] parse-options: fix completion format when first option is skipped

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

Patrick Steinhardt <ps@pks.im> writes:

> The "--git-completion-helper" option can be passed to any command or
> subcommand that uses the parse-options interface. The output it
> generates is a space-separated list of subcommands or options understood
> by the command.
>
> The format is slightly broken though in the case where the first option
> is not being printed, like for example a group or a hidden option. In
> that case, `show_gitcomp()` will of course skip that first entry. But
> when printing the next option it checks for `opts == original_opts` to
> verify whether we're printing the first option. The check will evaluate
> to false though as we have skipped it, and thus we'll print a leading
> space even though we have printed nothing else yet.
>
> Fix that bug by tracking whether we have already printed anything via a
> local variable.

Very clearly articulated.  I would have chosen 'shown' as the
variable name to so do, but 'first' may also be OK.

>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  parse-options.c               | 4 +++-
>  t/helper/test-parse-options.c | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/parse-options.c b/parse-options.c
> index 4519ead9dc..356eeff016 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -845,6 +845,7 @@ static int show_gitcomp(const struct option *opts, int show_all)
>  {
>  	const struct option *original_opts = opts;
>  	int nr_noopts = 0;
> +	bool first = true;
>  
>  	for (; opts->type != OPTION_END; opts++) {
>  		const char *prefix = "--";
> @@ -882,8 +883,9 @@ static int show_gitcomp(const struct option *opts, int show_all)
>  			suffix = "=";
>  		if (starts_with(opts->long_name, "no-"))
>  			nr_noopts++;
> -		printf("%s%s%s%s", opts == original_opts ? "" : " ",
> +		printf("%s%s%s%s", first ? "" : " ",
>  		       prefix, opts->long_name, suffix);
> +		first = false;
>  	}
>  	show_negated_gitcomp(original_opts, show_all, -1);
>  	show_negated_gitcomp(original_opts, show_all, nr_noopts);
> diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
> index f181f0c02d..fbafd67756 100644
> --- a/t/helper/test-parse-options.c
> +++ b/t/helper/test-parse-options.c
> @@ -351,6 +351,7 @@ static int parse_subcommand__cmd(int argc, const char **argv,
>  	parse_opt_subcommand_fn *fn = NULL;
>  	int opt = 0;
>  	struct option options[] = {
> +		OPT_GROUP("Subcommands"),
>  		OPT_SUBCOMMAND("subcmd-one", &fn, subcmd_one),
>  		OPT_SUBCOMMAND("subcmd-two", &fn, subcmd_two),
>  		OPT_INTEGER('o', "opt", &opt, "an integer option"),