Git development
 help / color / mirror / Atom feed
From: Tian Yuchen <cat@malon.dev>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org,
	Christian Couder <christian.couder@gmail.com>,
	Ayush Chandekar <ayu.chandekar@gmail.com>,
	Olamide Caleb Bello <belkid98@gmail.com>
Subject: Re: [PATCH v5 1/2] pack-objects: give fetch_if_missing call sites access to 'repo'
Date: Sat, 15 Aug 2026 14:31:54 +0800	[thread overview]
Message-ID: <0b5e76c2-4fa6-47ae-982c-f138cf343bd7@malon.dev> (raw)
In-Reply-To: <xmqq7blsrclt.fsf@gitster.g>

On 8/14/26 22:38, Junio C Hamano wrote:
> Tian Yuchen <cat@malon.dev> 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


  reply	other threads:[~2026-08-15  6:32 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-15  1:18 [PATCH v1] repository: move fetch_if_missing into struct repository Tian Yuchen
2026-07-15  3:27 ` Junio C Hamano
2026-07-15  4:58   ` Tian Yuchen
2026-07-15  6:35 ` Patrick Steinhardt
2026-07-16  7:06   ` Tian Yuchen
2026-07-16 15:28   ` Junio C Hamano
2026-07-16  7:29 ` [PATCH v2] " Tian Yuchen
2026-08-01 15:53   ` Tian Yuchen
2026-08-04  8:24   ` Patrick Steinhardt
2026-08-04 17:38     ` Junio C Hamano
2026-08-05 12:34       ` Tian Yuchen
2026-08-05 12:10     ` Tian Yuchen
2026-08-07  9:41   ` [PATCH v3] " Tian Yuchen
2026-08-07 17:03     ` Junio C Hamano
2026-08-09 15:00       ` Tian Yuchen
2026-08-13  6:11     ` [PATCH v4] " Tian Yuchen
2026-08-13 16:32       ` Junio C Hamano
2026-08-14  7:24       ` [PATCH v5 0/2] " Tian Yuchen
2026-08-14  7:24         ` [PATCH v5 1/2] pack-objects: give fetch_if_missing call sites access to 'repo' Tian Yuchen
2026-08-14 14:38           ` Junio C Hamano
2026-08-15  6:31             ` Tian Yuchen [this message]
2026-08-14  7:24         ` [PATCH v5 2/2] repository: move fetch_if_missing into struct repository Tian Yuchen
2026-08-14 14:44         ` [PATCH v5 0/2] " Junio C Hamano
2026-08-15  6:47         ` [PATCH v6] " Tian Yuchen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0b5e76c2-4fa6-47ae-982c-f138cf343bd7@malon.dev \
    --to=cat@malon.dev \
    --cc=ayu.chandekar@gmail.com \
    --cc=belkid98@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox