Re: [PATCH v5 1/2] pack-objects: give fetch_if_missing call sites access to 'repo'

On 8/14/26 22:38, Junio C Hamano wrote:
> Tian Yuchen <[email protected]> writes:
> 
>> 'option_parse_missing_action()' is registered as an OPT_CALLBACK, so
>> its signature is fixed and cannot easily gain a 'repo' parameter of
>> its own.
> 
> Is that true?  OPT_CALLBACK takes (short, long, value, arghelp,
> help, callback), but the value parameter, which typically receives
> the location to store the parsed value in, is NULL for
> "--missing=<...>" option, like so:
> 
> 		OPT_CALLBACK_F(0, "missing", NULL, N_("action"),
> 		  N_("handling for missing objects"), PARSE_OPT_NONEG,
> 		  option_parse_missing_action),
> 
> And "value" does not have to be the exact location; we can pass
> the address of the repository instance there, and the callback can
> dereference it to reach repo->fetch_if_missing member to make an
> assignment, if it wants to.
> 
> Am I missing something?
> 

Oh yeah indeed, 'value' can be used for passing repo. My mind went blank 
at that moment :(

>> Let it only record 'arg_missing_action'. Instead, apply the
>> side effect right after 'parse_options()' returns in
>> 'cmd_pack_objects()', where 'repo' is available.
> 
> When I review a new iteration, I first apply and look at the
> difference from the previous iteration before reading the new
> iteration of patches or their proposed commit log messages, and I
> did notice this business about "arg_missing_action" and wondering
> what it was about (which by the way is precisely I read the patch
> before the log message to ensure that the log message explains what



> I find unclear in the code).  Thanks for describing this.  But if we
> use OPT_CALLBACK() to pass the location of the variable, this may no
> longer be necessary, no?
> 

Will change, thanks!

Regards, yuchen