Kirill Smelkov <kirr@xxxxxxxxxxxxxxxxxxx> writes:
> On Mon, Sep 27, 2010 at 11:23:35AM -0700, Junio C Hamano wrote:
>> Kirill Smelkov <kirr@xxxxxxxxxxxxxxxxxxx> writes:
>
>
>> > +sed 's/^bin:/converted:/' "$@" >helper.out
>>
>> Minor nit: this is inconsistent with the check done with grep above that
>> insists that the colon is followed by a SP.
>
> Yes, you are right. I'll amend it.
>
>> > diff --git a/t/t4042-diff-textconv-caching.sh b/t/t4042-diff-textconv-caching.sh
>> > index 91f8198..7668099 100755
>> > --- a/t/t4042-diff-textconv-caching.sh
>> > +++ b/t/t4042-diff-textconv-caching.sh
>> > @@ -5,18 +5,19 @@ test_description='test textconv caching'
>> >
>> > cat >helper <<'EOF'
>> > #!/bin/sh
>> > -sed 's/^/converted: /' "$@" >helper.out
>> > +grep -q '^bin: ' "$@" || { echo "E: $@ is not \"binary\" file" 1>&2; exit 1; }
>> ...
> ...
> I too think (1) is right. It was just that originally there was $@
> (which I now understand was wrong).
Well, the original's use of "$@" is perfectly fine; it would do the right
thing with one argument, of course, but it would do the right thing with
more than one, too. On the other hand, your use inside "echo" is not.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html