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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 24, 2021, 04:31 UTC
Message-ID: <xmqqv91n6o1u.fsf@gitster.g>
In-Reply-To: <20211024014256.3569322-2-eschwartz@archlinux.org>

Eli Schwartz <eschwartz@archlinux.org> writes:

Show 18 quoted lines

> It contains option arguments, not options. We would like to add option
> support here too.
>
> Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
> ---
>  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.

Show 17 quoted lines

>  	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;
>  			}
>  		}