public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Han Jiang <jhcarl0814@gmail.com>,
	git@vger.kernel.org, Patrick Steinhardt <ps@pks.im>,
	Derrick Stolee <stolee@gmail.com>
Subject: Re: git config set --file, key-value pair without '= value', gives Segmentation fault
Date: Wed, 31 Jul 2024 19:36:34 -0400	[thread overview]
Message-ID: <ZqrKeG+alFldB7OY@nand.local> (raw)
In-Reply-To: <xmqq5xslyyaf.fsf@gitster.g>

On Wed, Jul 31, 2024 at 03:47:04PM -0700, Junio C Hamano wrote:
> I would have actually expected the fix that follow your analysis (by
> the way, I found it really well done) would say something more like
>
> 	if (strcmp(key, store->key))
> 		return 0; /* not ours */
> +	if (!value)
> +		value = "true";
> 	if (store->fixed_value)
> 		return !strcmp(store->fixed_value, value);
>
> but I am not sure exactly how we want to handle synonymous Boolean
> values here.  Regardless of how "true" value is spelled in the
> configuration file, e.g.
>
>         [section]               [section]
>                 key                     key = true
>
> I wonder if "git config set --value=yes [--fixed-value] section.key
> false" should replace either of them to false.

I think it gets tricky when we talk about the implicit true value
here for exactly that reason. Do we take true to mean just "true"? Or
"true" and "yes", "1", "", and so on?

I tried to match the behavior a few lines down in that function, where
we only call regexec() if value is non-NULL.

> It would especially true if the command is invoked with --type=bool
> but it seems that the --type option does not participate in the
> matching with the current value.

I think I could be convinced that --type=bool should treat "", "true",
"1", "on", and so on as matching here, but I'm not sure.

> > I'd like to hear from Stolee (CC'd), who is the author of c90702a1f6
> > before submitting this as a standalone patch.

Thanks,
Taylor

  reply	other threads:[~2024-07-31 23:36 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-31 11:55 git config set --file, key-value pair without '= value', gives Segmentation fault Han Jiang
2024-07-31 21:36 ` Taylor Blau
2024-07-31 22:47   ` Junio C Hamano
2024-07-31 23:36     ` Taylor Blau [this message]
2024-07-31 23:46       ` Han Jiang
2024-08-01  1:28       ` Junio C Hamano

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=ZqrKeG+alFldB7OY@nand.local \
    --to=me@ttaylorr.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jhcarl0814@gmail.com \
    --cc=ps@pks.im \
    --cc=stolee@gmail.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