git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3] completion: add new _GIT_complete helper

From
Felipe Contreras <felipe.contreras@gmail.com>
Date
May 6, 2012, 20:37 UTC
Message-ID
<CAMP44s3xwgsfjZA6r+bydu-5r1nEO5cGc=wcDSr+WdwunTtpwg@mail.gmail.com>
In-Reply-To
<20120506111425.GJ2164@goldbirke>
Hi,
On Sun, May 6, 2012 at 1:14 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
Show 9 quoted lines
> On Sat, May 05, 2012 at 05:23:20PM +0200, Felipe Contreras wrote:
>> This simplifies the completions, and makes it easier to define aliases:
>>
>>  _GIT_complete gf git_fetch
>
> So, 'gf' is an alias for 'git fetch', for which the user would like to
> use the completion for 'git fetch', right?  But that completion
> function is caled _git_fetch(), so the underscore prefix is missing
> here.
No, it's not missing:
       local name="${2-$1}"
       eval "$(typeset -f __git_func_wrap | sed -e "s/__git_func/_$name/")"
       complete -o bashdefault -o default -o nospace -F _${name}_wrap
$1 2>/dev/null \
               || complete -o default -o nospace -F _${name}_wrap $1

See how we add '_' before $name? There's not point in burdening the user with adding a prefix that will _always_ be there anyway.

> Besides, this example won't work, because the completion for 'git
> fetch' uses __git_complete_remote_or_refspec(), which in turn relies
> on finding out the name of the git command from the word on the
> command line, and it won't be able to do that from 'gf'.

That's irrelevant, it's an example; replace with another command that doesn't use 'words', and it would work.

> I remember we discussed this in an earlier round, and you even
> suggested a possible fix (passing the command name as argument to
> __git_complete_remote_or_refspec()).  I think that's the right thing
> to do here.

Yeah, but I suggested that in order to avoid the eval and the typeset that I require for future future patches, but it turns out it's still needed anyway, so my suggestion is to have a 'cmd' variable that stores the command; __git_func_wrap would take the responsibility of doing that.

Show 5 quoted lines
>> +     __git_func "$@"
>
> What is this "$@" for and why?  None of the _git_<cmd>() functions
> take any arguments, nor does _git() and _gitk(), and AFAICT Bash won't
> pass any either.

bash's complete passes 3 arguments. They might not be used, but it doesn't hurt to pass them either.

Show 8 quoted lines
>> +}
>> +
>> +_GIT_complete ()
>> +{
>> +     local name="${2-$1}"
>> +     eval "$(typeset -f __git_func_wrap | sed -e "s/__git_func/_$name/")"
>
> Still don't like the subshell and sed here ...
I haven't found any other way.

As for the fix to 'git fetch' and others, I've attached a crude diff of all the fixes I have queued together, and there it works perfectly fine, but there's a lot of modifications required to properly traverse the words array. I've meant to create a special function to traverse this array, but I haven't had time time for that; I haven't even landed this patch due to apparently irrelevant discussions.

Cheers.
-- 
Felipe Contreras


diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9f56ec7..ec2c3cc 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -721,10 +721,18 @@ __git_complete_revlist ()
 	__git_complete_revlist_file
 }
 
+__git_get_pos ()
+{
+	echo $(( t = cmd_pos - 2 + $1 ))
+}
+
 __git_complete_remote_or_refspec ()
 {
-	local cur_="$cur" cmd="${words[1]}"
-	local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0
+	local cur_="$cur"
+	local i c remote="" pfx="" lhs=1 no_complete_refspec=0
+
+	c=$(__git_get_pos 2)
+
 	if [ "$cmd" = "remote" ]; then
 		((c++))
 	fi
@@ -976,7 +984,8 @@ __git_aliased_command ()
 # __git_find_on_cmdline requires 1 argument
 __git_find_on_cmdline ()
 {
-	local word subcommand c=1
+	local word subcommand c
+	c=$(__git_get_pos 1)
 	while [ $c -lt $cword ]; do
 		word="${words[c]}"
 		for subcommand in $1; do
@@ -991,7 +1000,8 @@ __git_find_on_cmdline ()
 
 __git_has_doubledash ()
 {
-	local c=1
+	local c
+	c=$(__git_get_pos 1)
 	while [ $c -lt $cword ]; do
 		if [ "--" = "${words[c]}" ]; then
 			return 0
@@ -1111,8 +1121,8 @@ _git_bisect ()
 
 _git_branch ()
 {
-	local i c=1 only_local_ref="n" has_r="n"
-
+	local i c only_local_ref="n" has_r="n"
+	c=$(__git_get_pos 1)
 	while [ $c -lt $cword ]; do
 		i="${words[c]}"
 		case "$i" in
@@ -1142,16 +1152,19 @@ _git_branch ()
 
 _git_bundle ()
 {
-	local cmd="${words[2]}"
-	case "$cword" in
+	local subcommands='create list-heads verify unbundle'
+	local subcommand="$(__git_find_on_cmdline "$subcommands")"
+	local r
+	(( r = cword - cmd_pos + 2 ))
+	case "$r" 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
 			;;
@@ -1427,6 +1440,7 @@ __git_match_ctag() {
 _git_grep ()
 {
 	__git_has_doubledash && return
+	local p=$(__git_get_pos 2)
 
 	case "$cur" in
 	--*)
@@ -1447,7 +1461,7 @@ _git_grep ()
 	esac
 
 	case "$cword,$prev" in
-	2,*|*,-*)
+	$p,*|*,-*)
 		if test -r tags; then
 			__gitcomp_nl "$(__git_match_ctag "$cur" tags)"
 			return
@@ -2562,7 +2576,8 @@ _git_svn ()
 
 _git_tag ()
 {
-	local i c=1 f=0
+	local i c f=0
+	c=$(__git_get_pos 1)
 	while [ $c -lt $cword ]; do
 		i="${words[c]}"
 		case "$i" in
@@ -2599,39 +2614,24 @@ _git_whatchanged ()
 	_git_log
 }
 
-_git ()
+_main_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 i c=1 cmd cmd_pos __git_dir
 
-	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
 		--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
@@ -2655,34 +2655,20 @@ _git ()
 		return
 	fi
 
-	local completion_func="_git_${command//-/_}"
+	(( cmd_pos = c + 1 ))
+
+	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
 	fi
 }
 
-_gitk ()
+_main_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 +2689,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
+foo_wrap ()
+{
+	local cmd=foo_cmd cmd_pos=1
+	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
+	foo "$@"
+}
+
+git_complete ()
+{
+	local name="${2-main_$1}"
+	local cmd="${name#git_}"
+	eval "$(typeset -f foo_wrap | sed -e "s/foo/_$name/" -e "s/foo_cmd/$cmd/")"
+	complete -o bashdefault -o default -o nospace -F _${name}_wrap $1 2>/dev/null \
+		|| complete -o default -o nospace -F _${name}_wrap $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 main_git
 fi
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index cc12732..2966a18 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
+	${comp_wrap-_main_git_wrap} && print_comp
 }
 
 test_completion ()
@@ -73,6 +73,22 @@ test_completion ()
 	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
@@ -155,4 +171,71 @@ 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_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_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
Previous: Felipe ContrerasNext: SZEDER Gábor
Message 21 of 24 in “completion: add new _GIT_complete helper”
  1. completion: add new _GIT_complete helperFelipe Contreras, May 5, 2012
  2. Jonathan NiederMay 5, 2012
  3. Felipe ContrerasMay 5, 2012
  4. Jonathan NiederMay 5, 2012
  5. Jonathan NiederMay 5, 2012
  6. Felipe ContrerasMay 5, 2012
  7. Jonathan gives feedback --> flamewars inevitable? (Re: [PATCH v3] completion: add new _GIT_complete helper)Jonathan Nieder, May 5, 2012
  8. Felipe ContrerasMay 5, 2012
  9. Jonathan NiederMay 5, 2012
  10. Jonathan NiederMay 5, 2012
  11. Tay Ray ChuanMay 6, 2012
  12. Felipe ContrerasMay 14, 2012
  13. Jonathan NiederMay 5, 2012
  14. SZEDER GáborMay 6, 2012
  15. Jonathan NiederMay 6, 2012
  16. SZEDER GáborMay 6, 2012
  17. SZEDER GáborMay 6, 2012
  18. SZEDER GáborMay 6, 2012
  19. SZEDER GáborMay 6, 2012
  20. Felipe ContrerasMay 6, 2012
  21. Felipe ContrerasMay 6, 2012
  22. SZEDER GáborMay 6, 2012
  23. Felipe ContrerasMay 7, 2012
  24. SZEDER GáborMay 7, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.