Re: [PATCH 2/5] completion: fix args of run_completion() test helper

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> On Fri, Sep 28, 2012 at 12:23:47PM -0700, Junio C Hamano wrote:
>
>> >> > @@ -57,7 +57,7 @@ run_completion ()
>> >> >  test_completion ()
>> >> >  {
>> >> >  	test $# -gt 1 && echo "$2" > expected
>> >> > -	run_completion "$@" &&
>> >> > +	run_completion $1 &&
>> >> >  	test_cmp expected out
>> >> >  }
>> >> 
>> >> I can understand the other three hunks, but this one is fishy.
>> >> Shouldn't "$1" be inside a pair of dq?  I.e.
>> >> 
>> >> 	+	run_completion "$1" &&
>> >
>> > No.  $1 holds all words on the command line.  If it was between a pair
>> > of dq, then the whole command line would be passed to the completion
>> > script as a single word.
>> 
>> And these "words" can be split at $IFS boundaries without any
>> issues?  IOW, nobody would ever want to make words array in the
>> run_completion function to ['git' 'foo bar' 'baz']?
>
> It might be simpler to just convert test_completion into the
> test_completion_long I added in my series; the latter takes the expected
> output on stdin, leaving the actual arguments free to represent the real
> command-line. E.g., your example would become:
>
>   test_completion git "foo bar" baz <<-\EOF
>   ... expected output ...
>   EOF

I realize that the way my question was stated was misleading.  It
was not meant as a rhetorical "You would never be able to pass
['git' 'foo bar' 'baz'] with that interface, and the patch sucks."
but was meant as a pure question "Do we want to pass such word
list?".  "test_completion is almost always used to test completion
with inputs without any $IFS letters in it, so not being able to
test such an input via this interface is fine. If needed, we can
give another less often used interface to let you pass such an
input" is perfectly fine by me.

But I suspect that the real reason test_completion requires the
caller to express the list of inputs to run_completion as $IFS
separate list is because it needs to also get expected from the
command line:

>> >  test_completion ()
>> >  {
>> >  	test $# -gt 1 && echo "$2" > expected
>> > -	run_completion "$@" &&
>> > +	run_completion $1 &&
>> >  	test_cmp expected out
>> >  }

I wonder if doing something like this would be a far simpler
solution:

	test_completion ()
        {
		case "$1" in
                '')
			;;
		*)
			echo "$1" >expect &&
	                shift
                        ;;
		esac &&
                run_completion "$@" &&
                test_cmp expect output
	}