Re: [PATCH v2] tests: add initial bash completion tests

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

On Fri, Apr 13, 2012 at 01:48:51PM +0300, Felipe Contreras wrote:
> 2012/4/13 SZEDER Gábor <szeder@ira.uka.de>:
> > On Fri, Apr 13, 2012 at 11:12:36AM +0200, SZEDER Gábor wrote:
> >> On Thu, Apr 12, 2012 at 12:57:03AM +0300, Felipe Contreras wrote:
> >> > +. "$GIT_BUILD_DIR/contrib/completion/git-completion.bash"
> >> > +
> >> > +_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?
> >
> > Ah, ok, I think I got it.
> >
> > Of course, the words on the command line must be specified somehow to
> > test completion functions.  But the two implementations of
> > _get_comp_words_by_ref() for bash and zsh in the completion script
> > take the words on the command line from different variables, so we
> > need a common implementation to test completion functions both on bash
> > and zsh.  Hence the _get_comp_words_by_ref() above, which takes the
> > words on the command line and their count from $_words and $_cword,
> > respectively, and run_completion() below, which fills those variables
> > with its arguments.
> 
> Well, yeah, that's one reason, but also I don't see the point in
> trying to fill the internal bash completion variables, maybe there
> would be some conflicts? Plus, the bash version of
> _get_comp_words_by_ref is rather complicated, so I decided to start
> with something simple that I could understand and see exactly what's
> going on. And for zsh I would definitely prefer to override
> _get_comp_words_by_ref than to mess with the internal variables,
> although I haven't found a way to test completion for zsh.

The tests are run in a non-interactive shell, which by default doesn't
load bash completion with its complicated _get_comp_words_by_ref().
So these tests use _get_comp_words_by_ref() from git's completion
script.


Anyway, out of curiosity I quickly tried this on top of b8574ba7 (i.e.
your patch from today's pu):

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 3bbec79b..6c1ea956 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -27,27 +27,6 @@ complete ()
 
 . "$GIT_BUILD_DIR/contrib/completion/git-completion.bash"
 
-_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
-}
-
 print_comp ()
 {
 	local IFS=$'\n'
@@ -56,10 +35,10 @@ print_comp ()
 
 run_completion ()
 {
-	local -a COMPREPLY _words
-	local _cword
-	_words=( $1 )
-	(( _cword = ${#_words[@]} - 1 ))
+	local -a COMPREPLY COMP_WORDS
+	local COMP_CWORD
+	COMP_WORDS=( $1 )
+	(( COMP_CWORD = ${#COMP_WORDS[@]} - 1 ))
 	_git && print_comp
 }

i.e. to set COMP_WORDS and COMP_CWORD in run_completion() and it
worked.  However, I agree that it feels iffy to mess with a
shell-specific variable, and I'm afraid that this just happened to
work on my system, but it might be broken in previous or future bash
versions.


Best,
Gábor