Re: [PATCH v3 09/11] t4014: more tests about appending s-o-b lines

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 28, 2013, 05:42 UTC
Message-ID: <7vboc9zwv1.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20130128033146.GM8206@elie.Belkin>

Jonathan Nieder <jrnieder@gmail.com> writes:

Show 20 quoted lines

> 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.

Show 8 quoted lines

> 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.