Re: [PATCH 4/5] Let git-add--interactive read colors from configuration
- From
- Dan Zwell <dzwell@gmail.com>
- Date
- Nov 23, 2007, 05:32 UTC
- Message-ID
- <474665E0.1010104@zwell.net>
- In-Reply-To
- <20071122223050.GC3620@sigill.intra.peff.net>
Jeff King wrote:
Show 20 quoted lines
>> (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).
> As you said, config_* converts git values to perl values. However, that conversion needs only be done for strings in .gitconfig. Is there any reason why the caller of the function would need to pass a string "false"? I just don't see the need for conversion of any kind.
Further, I think that we could return the default variable directly, without parsing it at all. It would be much simpler, and there would need to be no special cases for dealing with undef or 'false'. It's a perl function, being called with perl arguments, so a user should not be that surprised when 'false' does what perl says it should do.
Dan