Re: [PATCH B v4 5/5] git config: don't allow --get-color* and variable type

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Feb 22, 2009 at 7:35 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:
>
>>>> 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.
>
> "--get-color" gets, escape sequence to throw at the terminal to get the
> color configured.  It does not make sense to ask for bool (or int) for
> that action.
>
> "--get-colorbool" asks if it is appropriate to use such escape sequence
> (e.g. when the output is tty and config says "auto", you would get "please
> use color").  In other words, its type is always bool, so using it as
> --int does not make sense but we cannot really say using it with --bool is
> nonsense.

Now I get it, but why would somebody want to do '--bool
--get-colorbool'? That is redundant.

I think --bool should be used only for boolean variables, not color ones.

Please let me know if the original patch is ok and I'll resend it with
'--get-color*'.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux