From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Sun, 22 Feb 2009 17:12:07 GMT
Subject: Re: [PATCH B v4 5/5] git config: don't allow --get-color* and variable type
Message-ID: <94a0d4530902220912r94e00d6ha7592bc397baa57c@mail.gmail.com>
In-Reply-To: <7vmyce2z9q.fsf@gitster.siamese.dyndns.org>

On Sun, Feb 22, 2009 at 6:48 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Doing so would be incoherent since --get-color would pick a color slot
>> and ignore the variable type option (e.g. --bool), and the type would
>> require a variable name.
>>
>> Suggested by Junio C Hamano.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>>  builtin-config.c |    5 +++++
>>  1 files changed, 5 insertions(+), 0 deletions(-)
>>
>> diff --git a/builtin-config.c b/builtin-config.c
>> index 8045926..9930568 100644
>> --- a/builtin-config.c
>> +++ b/builtin-config.c
>> @@ -359,6 +359,11 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)
>>       if (get_colorbool_slot)
>>           actions |= ACTION_GET_COLORBOOL;
>>
>> +     if ((get_color_slot || get_colorbool_slot) && types) {
>> +             error("--get-color and variable type are incoherent");
>> +             usage_with_options(builtin_config_usage, builtin_config_options);
>> +     }
>> +
>
> I do not think I suggested anything like this, so I'd decline to take
> credit for this patch.
>
> Strictly speaking, "--bool --get-colorbool diff.color 1" shouldn't error
> out, don't you think?  And it certainly shouldn't say "--get-color".

Huh? I misinterpreted:
I see "git config --bool --get-color diff.color.whitespace" is still
allowed, which you might want to tighten further.

I don't really understand the --get-color* options, so please drop the patch.

-- 
Felipe Contreras