From: Tanay Abhra <tanayabh@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Matthieu Moy <Matthieu.Moy@imag.fr>
Subject: Re: [PATCH/RFC 5/5] add tests for checking the behaviour of "unset.variable"
Date: Fri, 03 Oct 2014 02:05:29 +0530 [thread overview]
Message-ID: <542DB711.9040503@gmail.com> (raw)
In-Reply-To: <xmqqmw9emdax.fsf@gitster.dls.corp.google.com>
On 10/3/2014 1:53 AM, Junio C Hamano wrote:
> 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?
next prev parent reply other threads:[~2014-10-02 20:35 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-02 13:24 [PATCH/RFC 0/5] add "unset.variable" for unsetting previously set variables Tanay Abhra
2014-10-02 13:24 ` [PATCH/RFC 1/5] config.c : move configset_iter() to an appropriate position Tanay Abhra
2014-10-02 13:24 ` [PATCH/RFC 2/5] make git_config_with_options() to use a configset Tanay Abhra
2014-10-02 13:24 ` [PATCH/RFC 3/5] add "unset.variable" for unsetting previously set variables Tanay Abhra
2014-10-02 13:24 ` [PATCH/RFC 4/5] document the new "unset.variable" variable Tanay Abhra
2014-10-02 13:24 ` [PATCH/RFC 5/5] add tests for checking the behaviour of "unset.variable" Tanay Abhra
2014-10-02 20:09 ` Junio C Hamano
2014-10-02 20:18 ` Tanay Abhra
2014-10-02 20:23 ` Junio C Hamano
2014-10-02 20:35 ` Tanay Abhra [this message]
2014-10-02 23:38 ` Junio C Hamano
2014-10-03 7:01 ` Matthieu Moy
2014-10-03 18:25 ` Junio C Hamano
2014-10-03 18:48 ` Junio C Hamano
2014-10-03 19:49 ` Matthieu Moy
2014-10-03 20:12 ` Junio C Hamano
2014-10-06 18:59 ` Tanay Abhra
2014-10-06 19:28 ` Junio C Hamano
2014-10-06 19:43 ` Tanay Abhra
2014-10-02 19:29 ` [PATCH/RFC 0/5] add "unset.variable" for unsetting previously set variables Junio C Hamano
2014-10-02 20:41 ` Jeff King
2014-10-02 19:58 ` Junio C Hamano
2014-10-07 11:17 ` Jakub Narębski
2014-10-07 16:44 ` Junio C Hamano
2014-10-08 5:46 ` Matthieu Moy
2014-10-08 17:14 ` Junio C Hamano
2014-10-08 18:37 ` Matthieu Moy
2014-10-08 19:52 ` Junio C Hamano
2014-10-10 8:11 ` Jeff King
2014-10-13 18:21 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=542DB711.9040503@gmail.com \
--to=tanayabh@gmail.com \
--cc=Matthieu.Moy@imag.fr \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).