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

On Fri, Apr 13, 2012 at 01:48:51PM +0300, Felipe Contreras wrote:

2012/4/13 SZEDER Gábor [off-list ref]:

quoted

On Fri, Apr 13, 2012 at 11:12:36AM +0200, SZEDER Gábor wrote:

quoted

On Thu, Apr 12, 2012 at 12:57:03AM +0300, Felipe Contreras wrote:

quoted

+. "$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