Junio C Hamano <gitster@xxxxxxxxx> writes:
> cmn@xxxxxxxx (Carlos Martín Nieto) writes:
>
>> Junio C Hamano <gitster@xxxxxxxxx> writes:
>>
>>> Carlos Martín Nieto <cmn@xxxxxxxx> writes:
>>>
>>>> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
>>>> index e9019ac..93e5d6e 100755
>>>> --- a/t/t3200-branch.sh
>>>> +++ b/t/t3200-branch.sh
>>>> @@ -383,6 +383,22 @@ test_expect_success 'use --set-upstream-to modify a particular branch' \
>>>> test "$(git config branch.my13.remote)" = "." &&
>>>> test "$(git config branch.my13.merge)" = "refs/heads/master"'
>>>>
>>>> +test_expect_success 'test --unset-upstream on HEAD' \
>>>> + 'git branch my14
>>>> + test_config branch.master.remote foo &&
>>>> + test_config branch.master.merge foo &&
>>>> + git branch --set-upstream-to my14 &&
>>>> + git branch --unset-upstream &&
>>>> + test_must_fail git config branch.master.remote &&
>>>> + test_must_fail git config branch.master.merge'
>>>> +
>>>> +test_expect_success 'test --unset-upstream on a particular branch' \
>>>> + 'git branch my15
>>>> + git branch --set-upstream-to master my14 &&
>>>> + git branch --unset-upstream my14 &&
>>>> + test_must_fail git config branch.my14.remote &&
>>>> + test_must_fail git config branch.my14.merge'
>>>> +
>>>
>>> What should happen when you say "--unset-upstream" on a branch B
>>> that does not have B@{upstream}? Should it fail? Should it be
>>> silently ignored? Is it undefined that we do not want to test?
>>
>> I'd say it should be ignored, as the end result we want is for there to
>> be no upstream information. What we do underneath is remove a couple of
>> config options, wich doesn't fail if they didn't insist in the first
>> place.
>
> I am not sure about that reasoning.
>
> $ git config --unset core.nosuchvariable ; echo $?
> 5
>
> looks like a failure to me.
>
> More importantly, wouldn't we want to catch typo in
>
> git branch --unset-upstream mext
>
> which admittedly should come from a different codepath (I do not
> think your patch checks if "mext" branch exists before going ahead
> to the config--unset dance)?
Sorry, the last paragraph was incomplete.
In general, we should detect errors and allow the user to choose to
ignore.
A script that wants to make sure that B does not have an upstream,
without knowing if it already has one, can say "--unset-upstream B"
and choose to ignore the error if B does not have an upstream.
If the script is carefully written, it would try to check if B has
one and call "--unset-upstream B" only when it doesn't. A casually
written loose script would say "--unset-upstream B 2>/dev/null"
and keep going (it would not notice other kinds of errors, but that
is what makes it "casual and loose").
--
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