From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 28 Jan 2013 05:42:58 GMT
Subject: Re: [PATCH v3 09/11] t4014: more tests about appending s-o-b lines
Message-ID: <7vboc9zwv1.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20130128033146.GM8206@elie.Belkin>

Jonathan Nieder <jrnieder@gmail.com> writes:

> Brandon Casey wrote:
>
> [...]
>> --- a/t/t4014-format-patch.sh
>> +++ b/t/t4014-format-patch.sh
>> @@ -1021,4 +1021,246 @@ test_expect_success 'cover letter using branch description (6)' '
>>  	grep hello actual >/dev/null
>>  '
>>  
>> +append_signoff()
>> +{
>> +	C=`git commit-tree HEAD^^{tree} -p HEAD` &&
>> +	git format-patch --stdout --signoff ${C}^..${C} |
>> +		tee append_signoff.patch |
>> +		sed -n "1,/^---$/p" |
>> +		grep -n -E "^Subject|Sign|^$"
>> +}
>
> Is "grep -n" portable?  I didn't find any uses of it elsewhere in the
> testsuite.

Yes, "-n" is in POSIX.  Even though we use it ourselves, "git grep"
supports it, too.

Any Emacs user would scream if their platform "grep" does not
support it, as it will make M-x grep (or grep-find) useless.

> Style: checking exit status from format-patch, avoiding sed|grep pipeline:
>
> 	C=$(git commit-tree HEAD^ -p HEAD) &&
> 	git format-patch --stdout --signoff $C^..$C >append_signoff.patch &&
> 	awk '
> 		/^---$/ { exit; }
> 		/^Subject/ || /^Sign/ || /^$/ { print NR ":" $0 }
> 	' <append_signoff.patch >actual

Yeah, awk/perl would be fine, too, and it is good that you pointed
out that the original was losing the exit status from format-patch.

Thanks.