All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Tian Yuchen <cat@malon.dev>
Cc: Patrick Steinhardt <ps@pks.im>,
	 git@vger.kernel.org, karthik.188@gmail.com,
	 phillip.wood@dunelm.org.uk,  jltobler@gmail.com
Subject: Re: [PATCH v1] builtin/mktree: remove USE_THE_REPOSITORY_VARIABLE
Date: Fri, 13 Mar 2026 10:54:41 -0700	[thread overview]
Message-ID: <xmqqzf4b4ntq.fsf@gitster.g> (raw)
In-Reply-To: <4fb9c915-7246-4c55-b7c6-b4ef7ca91230@malon.dev> (Tian Yuchen's message of "Sat, 14 Mar 2026 01:15:23 +0800")

Tian Yuchen <cat@malon.dev> writes:

> On 3/14/26 00:03, Junio C Hamano wrote:
>
>> There is one corner case; upon "git foo -h", your cmd_foo() will get
>> repo==NULL when the command is run outside a repository.  As long as
>> your cmd_foo() asks parse_options() to react to "-h" (which gives
>> the help message and then exits) before it uses repo assuming it
>> cannot be NULL, you are safe.
>
> I was completely blown away Σ( ° △ °)
>
> Thanks for pointing that out. Otherwise, I wouldn’t have been able to 
> figure it out no matter how hard I try.
>
> I just took a quick look at the code:
>
>> 	const struct option option[] = {
>> 		OPT_BOOL('z', NULL, &nul_term_line, N_("input is NUL terminated")),
>> 		OPT_SET_INT( 0 , "missing", &allow_missing, N_("allow missing objects"), 1),
>> 		OPT_SET_INT( 0 , "batch", &is_batch_mode, N_("allow creation of more than one tree"), 1),
>> 		OPT_END()
>> 	};
>> 
>> 	ac = parse_options(ac, av, prefix, option, mktree_usage, 0);
>> 	getline_fn = nul_term_line ? strbuf_getline_nul : strbuf_getline_lf;
>> 
>> 	while (!got_eof) {
>> 		while (1) { ...
>
> I think if there's a '-h' parameter, it gets intercepted in 
> parse_options() and the process exits before repo is called. So there’s 
> nothing to worry about, right?

Correct.  The function calls parse_options() before it looks at "repo".

> By the way, I find it a bit confusing that the'`-h' parameter — which is 
> solely used for documentation query — is parsed and intercepted within a 
> function that handles actual business logic.

I strongly disagree your idea that 'z' is more business logic than
'h' is.  Both are equally relevant.

  reply	other threads:[~2026-03-13 17:54 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-11 18:17 [PATCH v1] builtin/mktree: remove USE_THE_REPOSITORY_VARIABLE Tian Yuchen
2026-03-12  6:55 ` Patrick Steinhardt
2026-03-12 16:21   ` Tian Yuchen
2026-03-13  7:02     ` Patrick Steinhardt
2026-03-13 16:03     ` Junio C Hamano
2026-03-13 17:15       ` Tian Yuchen
2026-03-13 17:54         ` Junio C Hamano [this message]
2026-03-13 18:12           ` Tian Yuchen
2026-03-13 20:20             ` Junio C Hamano
2026-03-14  3:17               ` 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=xmqqzf4b4ntq.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=cat@malon.dev \
    --cc=git@vger.kernel.org \
    --cc=jltobler@gmail.com \
    --cc=karthik.188@gmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=ps@pks.im \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.