From: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Date: Wed, 11 Apr 2012 22:13:36 GMT
Subject: Re: [PATCH 2/5] Provide branch name in error message when using @{u}
Message-ID: <4F860210.5030409@in.waw.pl>
In-Reply-To: <7v7gxmkv1m.fsf@alter.siamese.dyndns.org>

On 04/11/2012 08:00 PM, Junio C Hamano wrote:
> Zbigniew Jędrzejewski-Szmek<zbyszek@in.waw.pl>  writes:
>
>> diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
>> index 1342915..a00b689 100755
>> --- a/t/t1507-rev-parse-upstream.sh
>> +++ b/t/t1507-rev-parse-upstream.sh
>> @@ -135,7 +135,7 @@ test_expect_success 'branch@{u} error message when no upstream' '
>>
>>   test_expect_success '@{u} error message when no upstream' '
>>   	cat>expect<<-EOF&&
>> -	error: No upstream branch found for ${sq}${sq}
>> +	error: No upstream branch found for ${sq}master${sq}
>>   	fatal: Needed a single revision
>>   	EOF
>>   	test_must_fail git rev-parse --verify @{u} 2>actual&&
>
> I am not sure if saying "... for 'master'" is better or "... for the
> current branch" is better.  Using different wording reflects the fact that
> the user gave "@{u}" and not "master@{u}".
Hi,

I think that explicitly providing the name of the branch is useless when
the user has a properly configured git prompt which always shows the 
current branch. But not everybody does that, and for such people 
providing the name in the error message could be useful.

> But I do not care too deeply.
I don't either. I'll wait to see if other people chime in.

 > Either way, it is a vast improvement over
 > the current "... for ''" output.
> And the "detached" case is definitely better.
Thanks!

Thank you for the review. I'll send a reroll taking into account your 
and Matthieu's comments in a day or two if nobody else comments.

Zbyszek