Re: Cannot override `remote.origin.url` with `-c` option

From: Jeff King <peff@peff.net>
Date: Jun 13, 2024, 10:24 UTC
Message-ID: <20240613102409.GE817573@coredump.intra.peff.net>
In-Reply-To: <xmqq34pjxzva.fsf@gitster.g>

On Tue, Jun 11, 2024 at 08:28:41AM -0700, Junio C Hamano wrote:

Show 9 quoted lines

> >   2. Is there a way to override the list?
> >
> >      Sadly, no. For some config keys, we allow a value-less boolean
> >      entry to reset the list.
> 
> Hmph, I somehow thought that a more widely used "clear" was an empty
> string, but if we can make the convention around the "I exist and I
> mean true" entries, that would be great.  It would not make much
> sense to have multi-valued Boolean variable to begin with.

No, you're right. I was just misremembering (maybe because "--no-foo" is the command-line way to reset the "foo" list). The three instances of config I found (merge.suppressdest, http.extraheaders, and credential.helper) all treat the empty string that way.

The multi-valued boolean thing would cover config keys where an empty string was semantically meaningful, but I wouldn't be surprised if we never run across such a case. So probably using "empty string is reset" as the convention is fine, and matches existing cases.

Show 6 quoted lines

> > diff --git a/remote.c b/remote.c
> [...]
> 
> I was expecting (with excitement) a mess, but the above is as clean
> as we can make the idea, I would say.  Lack of documentation and
> tests do count as incompleteness though of course.

Yeah, and we should probably do the same for pushurl. And I think there could be some cleanup of the memory ownership handling of add_url().

-Peff