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

[Jeff King <peff@peff.net>]

On Fri, Sep 28, 2012 at 12:49:25PM -0700, Junio C Hamano wrote:

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

I think we may eventually want to pass arguments with IFS into the
function, just to make sure it works (the tests I added checked for IFS
in the completion list rather than the input, but we should probably
check both).

I'm OK if it needs to be an alternate interface (right now you could do
it by calling run_completion yourself).

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

Right, that's why I suggested bumping that to stdin for the function.

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

That would also work. I mainly suggested the stdin thing because we need
it anyway for output that generates multiple answers (well, you don't
_need_ it; you can call run_completion yourself, but it saves a few
lines at each call site).

-Peff