Subject: Re: [PATCH] rev-list: make "struct rev_list_info" static to the only user

From: René Scharfe <[email-masked]>
Date: Sat, 19 Jul 2025 14:36:04 +0200

On 7/19/25 8:35 AM, Jeff King wrote:
> On Fri, Jul 18, 2025 at 04:58:03PM -0700, Junio C Hamano wrote:
> 
>> The structure has nothing to do with what "git bisect" does; as
>> nobody other than "git rev-list" implementation uses it, move it
>> as a private data type to builtin/rev-list.c
> 
> Nice improvement.

*nod*

>> @@ -652,7 +660,7 @@ int cmd_rev_list(int argc,
>>  	 */
>>  	/*
>>  	 * NEEDSWORK: These loops that attempt to find presence of
>> -	 * options without understanding that the options they are
>> +	 * options without understanding the options they are
>>  	 * skipping are broken (e.g., it would not know "--grep
>>  	 * --exclude-promisor-objects" is not triggering
>>  	 * "--exclude-promisor-objects" option).  We really need
> 
> This tacked-on bit seems funny to me. Isn't the original more correct?
> The loops do not understand that the options are broken.

No, the options are fine, but the loops are broken -- they cannot tell
what they are looking at is an option or an argument of a preceding
option, yet they ignore that latter possibility.  So the word "that"
is best left out.  I also don't see a connection to the struct move,
though.

René