Re: [PATCH 6/7] completion: fix expansion issue in __gitcomp()

["SZEDER Gábor" <szeder@ira.uka.de>]

On Sat, Nov 17, 2012 at 12:39:24PM +0100, Felipe Contreras wrote:
> On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> 
> > -# Generates completion reply with compgen, appending a space to possible
> > -# completion words, if necessary.
> > +# Generates completion reply for the word in $cur, appending a space to
> > +# possible completion words, if necessary.
> >  # It accepts 1 to 4 arguments:
> >  # 1: List of possible completion words.
> >  # 2: A prefix to be added to each possible completion word (optional).
> > -# 3: Generate possible completion matches for this word (optional).
> > +# 3: Generate possible completion matches for this word instead of $cur
> > +#    (optional).
> >  # 4: A suffix to be appended to each possible completion word (optional).
> >  __gitcomp ()
> >  {
> > @@ -241,10 +242,22 @@ __gitcomp ()
> >                 COMPREPLY=()
> >                 ;;
> >         *)
> > -               local IFS=$'\n'
> > -               COMPREPLY=($(compgen -P "${2-}" \
> > -                       -W "$(__gitcomp_1 "${1-}" "${4-}")" \
> > -                       -- "$cur_"))
> > +               local i=0 c IFS=$' \t\n'
> > +               for c in $1; do
> > +                       case $c in
> > +                       "$cur_"*)
> > +                               c="$c${4-}"
> > +                               case $c in
> > +                               --*=*|*.) ;;
> > +                               *) c="$c " ;;
> > +                               esac
> > +                               COMPREPLY[$i]="${2-}$c"
> > +                               i=$((++i))
> > +                               ;;
> > +                       *)
> > +                               ;;
> > +                       esac
> > +               done
> 
> This is not quite the same as before, is it? Before the suffix would
> be taken into consideration for the comparison with $cur_, but not any
> more.

That's a good catch, thanks.

I remember it puzzled me that the suffix is considered in the
comparison (and that a trailing space would be appended even after a
given suffix, too, so there seems to be no way to disable the trailing
space).  However, currently it doesn't make a difference for us,
because afaics we never specify a suffix for __gitcomp().  There were
two instances in _git_config() where we specified "." as suffix, but
those two were converted to __gitcomp_nl().  I changed those callsites
back to __gitcomp() and tried to provoke wrong behavior with the above
patch, but couldn't.

Anyway, it's better to err on the safe side, so here's the fixup.

-- >8 --
Subject: [PATCH] fixup! completion: fix expansion issue in __gitcomp()

---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a1bf732f..29818fb5 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -231,9 +231,9 @@ __gitcomp ()
 	*)
 		local i=0 c IFS=$' \t\n'
 		for c in $1; do
+			c="$c${4-}"
 			case $c in
 			"$cur_"*)
-				c="$c${4-}"
 				case $c in
 				--*=*|*.) ;;
 				*) c="$c " ;;
-- 
1.8.0.220.g4d14ece