From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Sat, 17 Nov 2012 20:26:00 GMT
Subject: Re: [PATCH v2 4/6] completion: consolidate test_completion*() tests
Message-ID: <CAMP44s2ppLttMkqBDQ6ROnMfMyRUUq371LCH+FWOvKX-uE4=SA@mail.gmail.com>
In-Reply-To: <7v7gplktdt.fsf@alter.siamese.dyndns.org>

On Sat, Nov 17, 2012 at 12:41 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> No need to have two versions; if a second argument is specified, use
>> that, otherwise use stdin.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>>  t/t9902-completion.sh | 30 +++++++++++++-----------------
>>  1 file changed, 13 insertions(+), 17 deletions(-)
>>
>> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
>> index 204c92a..59cdbfd 100755
>> --- a/t/t9902-completion.sh
>> +++ b/t/t9902-completion.sh
>> @@ -60,19 +60,15 @@ run_completion ()
>>  # 2: expected completion
>>  test_completion ()
>>  {
>> -     test $# -gt 1 && echo "$2" > expected
>> +     if [ $# -gt 1 ]; then
>> +             echo "$2" > expected
>> +     else
>> +             sed -e 's/Z$//' > expected
>> +     fi &&
>
> As "$2" could begin with dash, end with \c, etc. that possibly can
> be misinterpred by echo, I'd rewrite this as
>
>                 printf '%s\n' "$2" >expected
>
> Otherwise looked fine; thanks.

But that was the case before. I would do that in a separate patch.

Cheers.

-- 
Felipe Contreras