Re: [RFC PATCH v3 5/6] parse-options: parse into argv_array

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 29, 2020, 19:33 UTC
Message-ID: <xmqqmu3i9kvg.fsf@gitster.c.googlers.com>
In-Reply-To: <20200728222455.3023400-6-emilyshaffer@google.com>

Emily Shaffer <emilyshaffer@google.com> writes:

Show 13 quoted lines

> parse-options already knows how to read into a string_list, and it knows
> how to read into an argv_array as a passthrough (that is, including the
> argument as well as its value). string_list and argv_array serve similar
> purposes but are somewhat painful to convert between; so, let's teach
> parse-options to read values of string arguments directly into an
> argv_array without preserving the argument name.
>
> This is useful if collecting generic arguments to pass through to
> another command, for example, 'git hook run --arg "--quiet" --arg
> "--format=pretty" some-hook'. The resulting argv_array would contain
> { "--quiet", "--format=pretty" }.
>
> The implementation is based on that of OPT_STRING_LIST.

Be it argv_array or strvec, I think this is a useful thing to do.

I grepped for the users of OPT_STRING_LIST() to see if some of them are better served by this, but none of them stood out as candidates that are particularly good match.

Show 11 quoted lines

> +int parse_opt_argv_array(const struct option *opt, const char *arg, int unset)
> +{
> +	struct argv_array *v = opt->value;
> +
> +	if (unset) {
> +		argv_array_clear(v);
> +		return 0;
> +	}
> +
> +	if (!arg)
> +		return -1;

I think the calling parse_options() loop would catch this negative return and raise an error, but is it better for this code to stay silent or would it be better to say that opt->long_name/short_name is not a boolean?

> +	argv_array_push(v, arg);
> +	return 0;
> +}