From: Jeff King <peff@peff.net>
Date: Thu, 22 Nov 2007 22:30:50 GMT
Subject: Re: [PATCH 4/5] Let git-add--interactive read colors from configuration
Message-ID: <20071122223050.GC3620@sigill.intra.peff.net>
In-Reply-To: <7v63zu3r7h.fsf@gitster.siamese.dyndns.org>

On Thu, Nov 22, 2007 at 01:28:34PM -0800, Junio C Hamano wrote:

> I think the "config_bool with default" also makes sense but it
> needs to be coded a bit carefully.  Issues to consider:

Yes. It is not strictly necessary for this patch series, but I think it
is nice to stake out a claim on the third argument of config_* functions
for consistency sake. But perhaps in the name of avoiding regression, it
should come later, when somebody actually wants to use it.

>  (1) Non default form "$r->config_bool('key')" should keep the
>      original semantics; missing key in the configuration is the
>      same as false (i.e. "undef" in scalar, () in list context).

Yes, this is obviously the most important thing.

>  (2) What should be the second parameter in the form to default
>      to true?  '1'?  'true'?  Any kind of "true" value in Perl
>      should be accepted?
> 
>  (3) Same question as (2) but for defaulting to false.  Any kind
>      of "false"?

Hmm. I am tempted to say "yes, any true or any false value" in that the
point of config_* is to convert git config values to native perl
representations. OTOH, the moral equivalent of

  config_color('my.key', 'bold red');

is probably more appropriately

  config_bool('my.key', 'true');

so I am fine doing it that way, as well (though I think it makes us
duplicate the "translate these strings into bools" code into perl).

-Peff