Re: [PATCH v2 4/6] completion: consolidate test_completion*() tests

2012-11-17 Thread Felipe Contreras
On Sat, Nov 17, 2012 at 12:41 AM, Junio C Hamano  wrote:
> Felipe Contreras  writes:
>
>> No need to have two versions; if a second argument is specified, use
>> that, otherwise use stdin.
>>
>> Signed-off-by: Felipe Contreras 
>> ---
>>  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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 4/6] completion: consolidate test_completion*() tests

2012-11-16 Thread Junio C Hamano
Felipe Contreras  writes:

> No need to have two versions; if a second argument is specified, use
> that, otherwise use stdin.
>
> Signed-off-by: Felipe Contreras 
> ---
>  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.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html