Junio C Hamano <gitster@xxxxxxxxx> writes:
> 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").
OK, that's a good point, I'll update the patch.
cmn
--
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