Re: [PATCH/RFC 5/5] add tests for checking the behaviour of "unset.variable"

From: Tanay Abhra <tanayabh@gmail.com>
Date: Oct 2, 2014, 20:35 UTC
Message-ID: <542DB711.9040503@gmail.com>
In-Reply-To: <xmqqmw9emdax.fsf@gitster.dls.corp.google.com>

On 10/3/2014 1:53 AM, Junio C Hamano wrote:

Show 35 quoted lines

> Tanay Abhra <tanayabh@gmail.com> writes:
> 
>> On 10/3/2014 1:39 AM, Junio C Hamano wrote:
>>> Tanay Abhra <tanayabh@gmail.com> writes:
>>>
>>>> +test_expect_success 'document how unset.variable will behave in shell scripts' '
>>>> +	rm -f .git/config &&
>>>> +	cat >expect <<-\EOF &&
>>>> +	EOF
>>>> +	git config foo.bar boz1 &&
>>>> +	git config --add foo.bar boz2 &&
>>>> +	git config unset.variable foo.bar &&
>>>> +	git config --add foo.bar boz3 &&
>>>> +	test_must_fail git config --get-all foo.bar >actual &&
>>>
>>> You make foo.bar a multi-valued one, then you unset it, so I would
>>> imagine that the value given after that, 'boz3', would be the only
>>> value foo.bar has.  Why should --get-all fail?
>>>
>>> I am having a hard time imagining how this behaviour can make any
>>> sense.
>>>
>>
>> git config -add appends the value to a existing header, after these
>> two commands have executed the config file would look like,
>> ...
> 
> I *know* how it is implemented before the changes of this series.
> And if the original implementation of "add" is left as-is, I can
> imagine how such a behaviour that is unintuitive to end-users can
> arise.
> 
> I was and am having a hard time how this behaviour can make any
> sense from an end-user's point of view.
>

That's what I was trying to document. I think that it makes no sense to use the feature as I had shown above. I had envisioned "unset.variable" to be explicitly typed in on the appropriate place as can be seen in the first two tests but Matthieu had suggested to add tests using git config too. This is when I had discovered these inconsistencies.

I can think of two solutions, one leave it as it is and advertise it to be explicitly typed in the config files at the appropriate position or to change the behavior of unset.variable to unset all matching variables in that file, before and after. We could also change git config --add to append at the end of the file regardless the variable exists or not. Which course of action do you think would be best?