[RFC/PATCH v4 0/6] completion: __git_complete and other stuff

STALE3736d

10 messages, 2 authors, 2016-06-15 · open the first message on its own page

[RFC/PATCH v4 0/6] completion: __git_complete and other stuff

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:53:45

Hi,

In order to simplify the discussion I've cleaned up my patches related to
_GIT_complete, aliasing stuff, and such.

I'm only intending the firat patch for inclusion for now.

Cheers.

Felipe Contreras (6):
  completion: add new __git_complete helper
  tests: add more bash completion tests
  completion: simplify _git_bundle
  completion: simplify command stuff
  completion: calculate argument position properly
  completion: add public _GIT_complete helper

 contrib/completion/git-completion.bash |  103 ++++++++++++++++----------------
 t/t9902-completion.sh                  |   85 +++++++++++++++++++++++++-
 2 files changed, 135 insertions(+), 53 deletions(-)

-- 
1.7.10

[RFC/PATCH v4 1/6] completion: add new __git_complete helper

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:53:45

This simplifies the completions, and would make it easier to define
aliases in the future.

Signed-off-by: Felipe Contreras <redacted>
---
Since v3:

 * Get rid of typeset and subshell

Since v2:

 * Rename to _GIT_complete to follow bash completion "guidelines"
 * Get rid of foo_wrap name

Since v1:

 * Remove stuff related to aliases fixes; should work on top of master

 contrib/completion/git-completion.bash |   69 +++++++++++++++-----------------
 t/t9902-completion.sh                  |    2 +-
 2 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9f56ec7..4098902 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash

@@ -2603,21 +2603,6 @@ _git ()
 {
 	local i c=1 command __git_dir
 
-	if [[ -n ${ZSH_VERSION-} ]]; then
-		emulate -L bash
-		setopt KSH_TYPESET
-
-		# workaround zsh's bug that leaves 'words' as a special
-		# variable in versions < 4.3.12
-		typeset -h words
-
-		# workaround zsh's bug that quotes spaces in the COMPREPLY
-		# array if IFS doesn't contain spaces.
-		typeset -h IFS
-	fi
-
-	local cur words cword prev
-	_get_comp_words_by_ref -n =: cur words cword prev
 	while [ $c -lt $cword ]; do
 		i="${words[c]}"
 		case "$i" in

@@ -2667,22 +2652,6 @@ _git ()
 
 _gitk ()
 {
-	if [[ -n ${ZSH_VERSION-} ]]; then
-		emulate -L bash
-		setopt KSH_TYPESET
-
-		# workaround zsh's bug that leaves 'words' as a special
-		# variable in versions < 4.3.12
-		typeset -h words
-
-		# workaround zsh's bug that quotes spaces in the COMPREPLY
-		# array if IFS doesn't contain spaces.
-		typeset -h IFS
-	fi
-
-	local cur words cword prev
-	_get_comp_words_by_ref -n =: cur words cword prev
-
 	__git_has_doubledash && return
 
 	local g="$(__gitdir)"

@@ -2703,16 +2672,42 @@ _gitk ()
 	__git_complete_revlist
 }
 
-complete -o bashdefault -o default -o nospace -F _git git 2>/dev/null \
-	|| complete -o default -o nospace -F _git git
-complete -o bashdefault -o default -o nospace -F _gitk gitk 2>/dev/null \
-	|| complete -o default -o nospace -F _gitk gitk
+__git_func_wrap ()
+{
+	if [[ -n ${ZSH_VERSION-} ]]; then
+		emulate -L bash
+		setopt KSH_TYPESET
+
+		# workaround zsh's bug that leaves 'words' as a special
+		# variable in versions < 4.3.12
+		typeset -h words
+
+		# workaround zsh's bug that quotes spaces in the COMPREPLY
+		# array if IFS doesn't contain spaces.
+		typeset -h IFS
+	fi
+	local cur words cword prev
+	_get_comp_words_by_ref -n =: cur words cword prev
+	_$1
+}
+
+# this is NOT a public function; use at your own risk
+__git_complete ()
+{
+	local name="${2-$1}"
+	local wrapper="_${name}_wrap"
+	eval "$wrapper () { __git_func_wrap $name ; }"
+	complete -o bashdefault -o default -o nospace -F $wrapper $1 2>/dev/null \
+		|| complete -o default -o nospace -F $wrapper $1
+}
+
+__git_complete git
+__git_complete gitk
 
 # The following are necessary only for Cygwin, and only are needed
 # when the user has tab-completed the executable name and consequently
 # included the '.exe' suffix.
 #
 if [ Cygwin = "$(uname -o 2>/dev/null)" ]; then
-complete -o bashdefault -o default -o nospace -F _git git.exe 2>/dev/null \
-	|| complete -o default -o nospace -F _git git.exe
+__git_complete git.exe git
 fi

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 5bda6b6..331a5b9 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh

@@ -63,7 +63,7 @@ run_completion ()
 	local _cword
 	_words=( $1 )
 	(( _cword = ${#_words[@]} - 1 ))
-	_git && print_comp
+	_git_wrap && print_comp
 }
 
 test_completion ()

-- 
1.7.10

[RFC/PATCH v4 2/6] tests: add more bash completion tests

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:53:45

These tests try to excercise code that deals with 'words' and 'cword'.

Signed-off-by: Felipe Contreras <redacted>
---

 t/t9902-completion.sh |   55 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 331a5b9..bb66848 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh

@@ -158,6 +158,22 @@ test_expect_success '__gitcomp - suffix' '
 	test_cmp expected out
 '
 
+setup_repository ()
+{
+	mkdir "$1" && (
+	cd "$1" &&
+	git init &&
+	test_tick &&
+	git commit --allow-empty -m "Initial"
+	)
+}
+
+test_expect_success 'prepare' '
+	setup_repository one &&
+	git clone one test &&
+	cd test
+'
+
 test_expect_success 'basic' '
 	run_completion "git \"\"" &&
 	# built-in

@@ -240,4 +256,43 @@ test_expect_success 'general options plus command' '
 	test_completion "git --no-replace-objects check" "checkout "
 '
 
+test_expect_success 'remote or refspec' '
+	test_completion "git fetch o" "origin " &&
+	test_completion "git fetch origin m" "master:master " &&
+	test_completion "git pull o" "origin " &&
+	test_completion "git pull origin m" "master " &&
+	test_completion "git push o" "origin " &&
+	test_completion "git push origin m" "master "
+'
+
+test_expect_success 'subcomands' '
+	test_completion "git bisect st" "start "
+'
+
+test_expect_success 'has double dash' '
+	test_completion "git add -- foo" ""
+'
+
+test_expect_success 'config' '
+	git config --file=foo color.ui auto &&
+	test_completion "git config --file=foo --get c" "color.ui "
+'
+
+test_expect_success 'other' '
+	cat >expected <<-\EOF &&
+	origin/HEAD 
+	origin/master 
+	EOF
+	test_completion "git branch -r o" &&
+	test_completion "git bundle cr" "create " &&
+
+	echo foobar > tags &&
+	test_completion "git grep f" "foobar " &&
+
+	test_completion "git notes --ref m" "master " &&
+
+	git tag v0.1 &&
+	test_completion "git tag -d v" "v0.1 "
+'
+
 test_done

-- 
1.7.10

[RFC/PATCH v4 3/6] completion: simplify _git_bundle

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:53:45

Signed-off-by: Felipe Contreras <redacted>
---
 contrib/completion/git-completion.bash |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 4098902..8de0358 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash

@@ -1142,16 +1142,18 @@ _git_branch ()
 
 _git_bundle ()
 {
-	local cmd="${words[2]}"
+	local subcommands='create list-heads verify unbundle'
+	local subcommand="$(__git_find_on_cmdline "$subcommands")"
+
 	case "$cword" in
 	2)
-		__gitcomp "create list-heads verify unbundle"
+		__gitcomp "$subcommands"
 		;;
 	3)
 		# looking for a file
 		;;
 	*)
-		case "$cmd" in
+		case "$subcommand" in
 			create)
 				__git_complete_revlist
 			;;

-- 
1.7.10

[RFC/PATCH v4 6/6] completion: add public _GIT_complete helper

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:53:45

So that users can easily define aliases, such as:

 _GIT_complete gf git_fetch

Signed-off-by: Felipe Contreras <redacted>
---
 contrib/completion/git-completion.bash |   10 +++++-----
 t/t9902-completion.sh                  |    9 ++++++++-
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 049110e..2b7ef02 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash

@@ -2678,6 +2678,7 @@ _gitk ()
 
 __git_func_wrap ()
 {
+	local cmd="${1#git_}" cmd_pos=1
 	if [[ -n ${ZSH_VERSION-} ]]; then
 		emulate -L bash
 		setopt KSH_TYPESET

@@ -2695,8 +2696,7 @@ __git_func_wrap ()
 	_$1
 }
 
-# this is NOT a public function; use at your own risk
-__git_complete ()
+_GIT_complete ()
 {
 	local name="${2-$1}"
 	local wrapper="_${name}_wrap"

@@ -2705,13 +2705,13 @@ __git_complete ()
 		|| complete -o default -o nospace -F $wrapper $1
 }
 
-__git_complete git
-__git_complete gitk
+_GIT_complete git
+_GIT_complete gitk
 
 # The following are necessary only for Cygwin, and only are needed
 # when the user has tab-completed the executable name and consequently
 # included the '.exe' suffix.
 #
 if [ Cygwin = "$(uname -o 2>/dev/null)" ]; then
-__git_complete git.exe git
+_GIT_complete git.exe git
 fi

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 6904594..2037452 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh

@@ -63,7 +63,7 @@ run_completion ()
 	local _cword
 	_words=( $1 )
 	(( _cword = ${#_words[@]} - 1 ))
-	_git_wrap && print_comp
+	${comp_wrap-_git_wrap} && print_comp
 }
 
 test_completion ()

@@ -316,4 +316,11 @@ test_expect_success 'global options extra checks' '
 	test_completion "git --no-pager tag -d v" "v0.1 "
 '
 
+test_expect_success 'aliases' '
+	local comp_wrap=_git_fetch_wrap &&
+	_GIT_complete gf git_fetch &&
+	test_completion "gf o" "origin " &&
+	test_completion "gf origin m" "master:master "
+'
+
 test_done

-- 
1.7.10

[RFC/PATCH v4 4/6] completion: simplify command stuff

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:53:45

No need to recalculate it.

Signed-off-by: Felipe Contreras <redacted>
---
 contrib/completion/git-completion.bash |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8de0358..e648d7c 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash

@@ -723,7 +723,7 @@ __git_complete_revlist ()
 
 __git_complete_remote_or_refspec ()
 {
-	local cur_="$cur" cmd="${words[1]}"
+	local cur_="$cur"
 	local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0
 	if [ "$cmd" = "remote" ]; then
 		((c++))

@@ -2603,22 +2603,22 @@ _git_whatchanged ()
 
 _git ()
 {
-	local i c=1 command __git_dir
+	local i c=1 cmd __git_dir
 
 	while [ $c -lt $cword ]; do
 		i="${words[c]}"
 		case "$i" in
 		--git-dir=*) __git_dir="${i#--git-dir=}" ;;
 		--bare)      __git_dir="." ;;
-		--help) command="help"; break ;;
+		--help) cmd="help"; break ;;
 		-c) c=$((++c)) ;;
 		-*) ;;
-		*) command="$i"; break ;;
+		*) cmd="$i"; break ;;
 		esac
 		((c++))
 	done
 
-	if [ -z "$command" ]; then
+	if [ -z "$cmd" ]; then
 		case "$cur" in
 		--*)   __gitcomp "
 			--paginate

@@ -2642,10 +2642,10 @@ _git ()
 		return
 	fi
 
-	local completion_func="_git_${command//-/_}"
+	local completion_func="_git_${cmd//-/_}"
 	declare -f $completion_func >/dev/null && $completion_func && return
 
-	local expansion=$(__git_aliased_command "$command")
+	local expansion=$(__git_aliased_command "$cmd")
 	if [ -n "$expansion" ]; then
 		completion_func="_git_${expansion//-/_}"
 		declare -f $completion_func >/dev/null && $completion_func

-- 
1.7.10

[RFC/PATCH v4 5/6] completion: calculate argument position properly

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:53:45

Positions are currently hard-coded, which means completions in the form
of 'git --foo bar' fail, because positions have been shifted.

This fixes the issue that was spotted in the mailing list regarding
certain aliases[1].

[1] http://thread.gmane.org/gmane.comp.version-control.git/185184

Cc: SZEDER Gábor <redacted>
Cc: Scott Bronson <redacted>
Cc: Nathan Broadbent <redacted>
Signed-off-by: Felipe Contreras <redacted>
---
 contrib/completion/git-completion.bash |   16 +++++++++-------
 t/t9902-completion.sh                  |   21 +++++++++++++++++++++
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index e648d7c..049110e 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash

@@ -724,7 +724,8 @@ __git_complete_revlist ()
 __git_complete_remote_or_refspec ()
 {
 	local cur_="$cur"
-	local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0
+	local i c=$cmd_pos remote="" pfx="" lhs=1 no_complete_refspec=0
+
 	if [ "$cmd" = "remote" ]; then
 		((c++))
 	fi

@@ -1144,12 +1145,11 @@ _git_bundle ()
 {
 	local subcommands='create list-heads verify unbundle'
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
-
-	case "$cword" in
-	2)
+	case $(( cword - cmd_pos )) in
+	0)
 		__gitcomp "$subcommands"
 		;;
-	3)
+	1)
 		# looking for a file
 		;;
 	*)

@@ -1449,7 +1449,7 @@ _git_grep ()
 	esac
 
 	case "$cword,$prev" in
-	2,*|*,-*)
+	$cmd_pos,*|*,-*)
 		if test -r tags; then
 			__gitcomp_nl "$(__git_match_ctag "$cur" tags)"
 			return

@@ -2603,7 +2603,7 @@ _git_whatchanged ()
 
 _git ()
 {
-	local i c=1 cmd __git_dir
+	local i c=1 cmd cmd_pos __git_dir
 
 	while [ $c -lt $cword ]; do
 		i="${words[c]}"

@@ -2642,6 +2642,8 @@ _git ()
 		return
 	fi
 
+	(( cmd_pos = c + 1 ))
+
 	local completion_func="_git_${cmd//-/_}"
 	declare -f $completion_func >/dev/null && $completion_func && return

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index bb66848..6904594 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh

@@ -295,4 +295,25 @@ test_expect_success 'other' '
 	test_completion "git tag -d v" "v0.1 "
 '
 
+test_expect_success 'global options extra checks' '
+	test_completion "git --no-pager fetch o" "origin " &&
+	test_completion "git --no-pager fetch origin m" "master:master " &&
+	test_completion "git --no-pager pull o" "origin " &&
+	test_completion "git --no-pager pull origin m" "master " &&
+	test_completion "git --no-pager push o" "origin " &&
+	test_completion "git --no-pager push origin m" "master " &&
+	test_completion "git --no-pager bisect st" "start " &&
+	test_completion "git --no-pager add -- foo" "" &&
+	test_completion "git --no-pager config --file=foo --get c" "color.ui " &&
+	cat >expected <<-\EOF &&
+	origin/HEAD 
+	origin/master 
+	EOF
+	test_completion "git --no-pager branch -r o" &&
+	test_completion "git --no-pager bundle cr" "create " &&
+	test_completion "git --no-pager grep f" "foobar " &&
+	test_completion "git --no-pager notes --ref m" "master " &&
+	test_completion "git --no-pager tag -d v" "v0.1 "
+'
+
 test_done

-- 
1.7.10

Re: [RFC/PATCH v4 5/6] completion: calculate argument position properly

From: Felipe Contreras <hidden>
Date: 2016-06-15 22:53:45

On Mon, May 7, 2012 at 3:23 AM, Felipe Contreras
[off-list ref] wrote:

quoted hunk

@@ -2642,6 +2642,8 @@ _git ()

               return
       fi

+       (( cmd_pos = c + 1 ))

Actually, I would prefer cmd_pos=$((c + 1)).

-- 
Felipe Contreras

Re: [RFC/PATCH v4 1/6] completion: add new __git_complete helper

From: SZEDER Gábor <hidden>
Date: 2016-06-15 22:53:46

Hi,

On Mon, May 07, 2012 at 03:23:15AM +0200, Felipe Contreras wrote:

+__git_func_wrap ()
+{
+	if [[ -n ${ZSH_VERSION-} ]]; then
+		emulate -L bash
+		setopt KSH_TYPESET
+
+		# workaround zsh's bug that leaves 'words' as a special
+		# variable in versions < 4.3.12
+		typeset -h words
+
+		# workaround zsh's bug that quotes spaces in the COMPREPLY
+		# array if IFS doesn't contain spaces.
+		typeset -h IFS
+	fi
+	local cur words cword prev
+	_get_comp_words_by_ref -n =: cur words cword prev
+	_$1
+}
+
+# this is NOT a public function; use at your own risk
+__git_complete ()
+{
+	local name="${2-$1}"

Please add the underscore prefix here right away, so we don't need
that "_$1" above in __git_func_wrap().

There is still no documentation about the parameters of
__git_complete(), so...

It seems to set up completion for the command given as first argument
to invoke the completion function given as second argument, right?
But the completion function argument is optional, and if it's not
specified, its name is derived from the command name by adding an
underscore prefix (in __git_func_wrap()).  So, in case of 'git', $name
becomes 'git', hence the completion function to be invoked is _git().
So far so good.

+	local wrapper="_${name}_wrap"

But then the wrapper function becomes _git_wrap().  Uh, oh.

+	eval "$wrapper () { __git_func_wrap $name ; }"

... because this would then define the function _git_wrap(), which is
not good, because that's supposed to be the completion function for
the 'git wrap' command.

That's exactly why I wrote

  local wrapper="__git_wrap_$1"

in my earlier post, and it was not an unnecessary change that could be
simplified away.

quoted hunk

+	complete -o bashdefault -o default -o nospace -F $wrapper $1 2>/dev/null \
+		|| complete -o default -o nospace -F $wrapper $1
+}
+
+__git_complete git
+__git_complete gitk
 
 # The following are necessary only for Cygwin, and only are needed
 # when the user has tab-completed the executable name and consequently
 # included the '.exe' suffix.
 #
 if [ Cygwin = "$(uname -o 2>/dev/null)" ]; then
-complete -o bashdefault -o default -o nospace -F _git git.exe 2>/dev/null \
-	|| complete -o default -o nospace -F _git git.exe
+__git_complete git.exe git
 fi

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 5bda6b6..331a5b9 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh

@@ -63,7 +63,7 @@ run_completion ()
 	local _cword
 	_words=( $1 )
 	(( _cword = ${#_words[@]} - 1 ))
-	_git && print_comp
+	_git_wrap && print_comp
 }
 
 test_completion ()

-- 
1.7.10

Re: [RFC/PATCH v4 4/6] completion: simplify command stuff

From: SZEDER Gábor <hidden>
Date: 2016-06-15 22:53:46

Hi,

On Mon, May 07, 2012 at 03:23:18AM +0200, Felipe Contreras wrote:

No need to recalculate it.

Signed-off-by: Felipe Contreras <redacted>
---
 contrib/completion/git-completion.bash |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

This seems to be the same patch you sent earlier as 7/12 in the first
round.  I had a few comments back then, see

  http://thread.gmane.org/gmane.comp.version-control.git/194958/focus=195276

But this $cmd variable will be only used in
__git_complete_remote_or_refspec(), so perhaps your idea later in that
thread about invoking it from _git_fetch() as

  __git_complete_remote_or_refspec "fetch"

would be better.  That would only require one modification in its four
callers, but neither in _git() in this patch nor in __git_complete()
later in the series to set $cmd.


Best,
Gábor

quoted hunk

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8de0358..e648d7c 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash

@@ -723,7 +723,7 @@ __git_complete_revlist ()
 
 __git_complete_remote_or_refspec ()
 {
-	local cur_="$cur" cmd="${words[1]}"
+	local cur_="$cur"
 	local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0
 	if [ "$cmd" = "remote" ]; then
 		((c++))

@@ -2603,22 +2603,22 @@ _git_whatchanged ()
 
 _git ()
 {
-	local i c=1 command __git_dir
+	local i c=1 cmd __git_dir
 
 	while [ $c -lt $cword ]; do
 		i="${words[c]}"
 		case "$i" in
 		--git-dir=*) __git_dir="${i#--git-dir=}" ;;
 		--bare)      __git_dir="." ;;
-		--help) command="help"; break ;;
+		--help) cmd="help"; break ;;
 		-c) c=$((++c)) ;;
 		-*) ;;
-		*) command="$i"; break ;;
+		*) cmd="$i"; break ;;
 		esac
 		((c++))
 	done
 
-	if [ -z "$command" ]; then
+	if [ -z "$cmd" ]; then
 		case "$cur" in
 		--*)   __gitcomp "
 			--paginate

@@ -2642,10 +2642,10 @@ _git ()
 		return
 	fi
 
-	local completion_func="_git_${command//-/_}"
+	local completion_func="_git_${cmd//-/_}"
 	declare -f $completion_func >/dev/null && $completion_func && return
 
-	local expansion=$(__git_aliased_command "$command")
+	local expansion=$(__git_aliased_command "$cmd")
 	if [ -n "$expansion" ]; then
 		completion_func="_git_${expansion//-/_}"
 		declare -f $completion_func >/dev/null && $completion_func

-- 
1.7.10

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help