diff mbox series

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

Message ID 20211024014256.3569322-2-eschwartz@archlinux.org (mailing list archive)
State Superseded
Headers show
Series Add some more options to the pretty-formats | expand

Commit Message

Eli Schwartz Oct. 24, 2021, 1:42 a.m. UTC
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(-)

Comments

Junio C Hamano Oct. 24, 2021, 4:31 a.m. UTC | #1
Eli Schwartz <eschwartz@archlinux.org> writes:

> 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.

>  	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;
>  			}
>  		}
Eli Schwartz Oct. 24, 2021, 3:37 p.m. UTC | #2
On 10/24/21 12:31 AM, Junio C Hamano wrote:
> Eli Schwartz <eschwartz@archlinux.org> writes:
> 
>> 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.


Right, the reason this change was submitted in its own patch was
essentially for the sole purpose of making the diff for the second patch
intuitive to read.
diff mbox series

Patch

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