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

[Junio C Hamano <gitster@pobox.com>]

Felipe Contreras <felipe.contreras@gmail.com> 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.

The two use formats with and without is_tty are meant to be used in
scripts this way:

	# without $is_tty
	if git config --get-colorbool color.diff
        then
        	use_color=true
	else
        	use_color=false
	fi

        # with $is_tty
        if test -t 1
        then
        	is_tty=1
	else
        	is_tty=0
	fi
        use_color=$(git config --get-colorbool color.diff $is_tty)

In the latter form, output from the command is captured by $() and does
not see if the stdout of the calling script is a tty, hence the script
feeds that information to the command as an extra parameter.