From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 13 Apr 2012 19:48:49 GMT
Subject: Re: [PATCH v2] tests: add initial bash completion tests
Message-ID: <7v62d32z1a.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20120413091236.GC2164@goldbirke>

SZEDER Gábor <szeder@ira.uka.de> writes:

>> +_get_comp_words_by_ref ()
>> +{
>> +	while [ $# -gt 0 ]; do
>> +		case "$1" in
>> +		cur)
>> +			cur=${_words[_cword]}
>> +			;;
>> +		prev)
>> +			prev=${_words[_cword-1]}
>> +			;;
>> +		words)
>> +			words=("${_words[@]}")
>> +			;;
>> +		cword)
>> +			cword=$_cword
>> +			;;
>> +		esac
>> +		shift
>> +	done
>> +}
>
> Git's completion script already implements this function.  Why
> override it here?

It is not "already implements" that I am worried about, but it implements
it differently without explaining why, which is worrying.  I agree it
needs to be explained before the function.
>> +	# plumbing
>> +	! grep -q "^ls-files \$" out
>
> The && is missing here at the end of the line.

True.

>> +	run_completion "git f" &&
>> +	! grep -q -v "^f" out
>
> grep is not a git command, so I'm not sure, but shouldn't these use
> 'test_must_fail grep' instead of '! grep'?

"! grep" is fine.  We are not trying to catch the case where we break the
implementation of "grep" to cause it to segfault.