Subject: Re: [PATCH 1/3] pretty.c: rename describe options variable to more descriptive name

From: Junio C Hamano <[email-masked]>
Date: Sat, 23 Oct 2021 21:31:57 -0700

Eli Schwartz <[email-masked]> writes:

> It contains option arguments, not options. We would like to add option
> support here too.
>
> Signed-off-by: Eli Schwartz <[email-masked]>
> ---
>  pretty.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/pretty.c b/pretty.c
> index 73b5ead509..9db2c65538 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -1216,7 +1216,7 @@ int format_set_trailers_options(struct process_trailer_options *opts,
>  
>  static size_t parse_describe_args(const char *start, struct strvec *args)
>  {
> -	const char *options[] = { "match", "exclude" };
> +	const char *option_arguments[] = { "match", "exclude" };

This renaming is more or less "Meh" without the other change in the
series that may (or may not) be helped with this step, but because I
haven't seen these later steps yet, I may sound too dismissive of
the change in this step.

Anyway, at least call that option_args[] to match the way the
function calls itself, not option_arguments[] that is a mouthful for
a mere implementation detail of local variable for a file-private
helper function.

Thanks.

>  	const char *arg = start;
>  
>  	for (;;) {
> @@ -1225,10 +1225,10 @@ static size_t parse_describe_args(const char *start, struct strvec *args)
>  		size_t arglen = 0;
>  		int i;
>  
> -		for (i = 0; i < ARRAY_SIZE(options); i++) {
> -			if (match_placeholder_arg_value(arg, options[i], &arg,
> +		for (i = 0; i < ARRAY_SIZE(option_arguments); i++) {
> +			if (match_placeholder_arg_value(arg, option_arguments[i], &arg,
>  							&argval, &arglen)) {
> -				matched = options[i];
> +				matched = option_arguments[i];
>  				break;
>  			}
>  		}