From: SZEDER Gábor <szeder@ira.uka.de>
Date: Sat, 04 Feb 2012 13:54:04 GMT
Subject: Re: [PATCH v4 4/4] completion: simplify __gitcomp*
Message-ID: <20120204135404.GF16099@goldbirke>
In-Reply-To: <7vty37oedr.fsf@alter.siamese.dyndns.org>

Hi,


On Fri, Feb 03, 2012 at 12:23:12PM -0800, Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > @@ -495,11 +495,7 @@ fi
> >  # 4: A suffix to be appended to each possible completion word (optional).
> >  __gitcomp ()
> >  {
> > -	local cur_="$cur"
> > -
> > -	if [ $# -gt 2 ]; then
> > -		cur_="$3"
> > -	fi
> > +	local cur_="${3:-$cur}"
> >  	case "$cur_" in
> >  	--*=)
> >  		COMPREPLY=()
> 
> I think this rewrite is wrong, even though it may not make a difference to
> the current callers (I didn't check).  Drop the colon from ${3:-...}.
> 
> > @@ -524,18 +520,8 @@ __gitcomp ()
> >  #    appended.
> >  __gitcomp_nl ()
> >  {
> > -	local s=$'\n' IFS=' '$'\t'$'\n'
> > -	local cur_="$cur" suffix=" "
> > -
> > -	if [ $# -gt 2 ]; then
> > -		cur_="$3"
> > -		if [ $# -gt 3 ]; then
> > -			suffix="$4"
> > -		fi
> > -	fi
> > -
> > -	IFS=$s
> > -	COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))
> > +	local IFS=$'\n'
> > +	COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3:-$cur}"))
> 
> So is this.
> 
> Fixing the above two gives me what I've already sent in $gmane/189683,
> so...
> 

Good point, I missed this when pointed out the similar issue with $4
earlier.

And it does make a difference, it breaks the completion of a single
word in multiple steps, e.g. git log --pretty=<TAB> master..<TAB>.  In
such cases we pass "${cur##--pretty=}" and "${cur_#*..}" as third
argument to __gitcomp() and __gitcomp_nl(), which can be empty strings
when the user hits TAB right after the '=' and '..'.  Replacing that
empty string with $cur is bad, because none of the possible completion
words (i.e. $1) will match it, and bash will fall back to filename
completion.

Without the colon, i.e. using "${3-$cur}", it works as expected.


Best,
Gábor