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

[PATCH 02/12] completion: wrap __git_refs() for better option parsing

From
SZEDER Gábor <szeder.dev@gmail.com>
Date
Feb 3, 2017, 02:53 UTC
Message-ID
<20170203025405.8242-3-szeder.dev@gmail.com>
In-Reply-To
<20170203025405.8242-1-szeder.dev@gmail.com>

__git_refs() currently accepts two optional positional parameters: a remote and a flag for 'git checkout's tracking DWIMery. To fix a minor bug, and, more importantly, for faster refs completion, this series will add three more parameters: a prefix, the current word to be completed and a suffix, i.e. the options accepted by __gitcomp() & friends, and will change __git_refs() to list only refs matching that given current word and to add that given prefix and suffix to the listed refs.

However, __git_refs() is the helper function that is most likely used in users' custom completion scriptlets for their own git commands, and we don't want to break those, so

  - we can't change __git_refs()'s default output format, i.e. we
    can't by default append a trailing space to every listed ref,
    meaning that the suffix parameter containing the default trailing
    space would have to be specified on every invocation, and
  - we can't change the position of existing positional parameters
    either, so there would have to be plenty of set-but-empty
    placeholder positional parameters all over the completion script.

Furthermore, with five positional parameters it would be really hard to remember which position means what.

To keep callsites simple, add the new wrapper function __git_complete_refs() around __git_refs(), which:

  - instead of positional parameters accepts real '--opt=val'-style
    options and with minimalistic option parsing translates them to
    __git_refs()'s and __gitcomp_nl()'s positional parameters, and
  - includes the '__gitcomp_nl "$(__git_refs ...)" ...' command
    substitution to make its behavior match its name and the behavior
    of other __git_complete_* functions, and to limit future changes
    in this series to __git_refs() and this new wrapper function.

Call this wrapper function instead of __git_refs() wherever possible throughout the completion script, i.e. when __git_refs()'s output is fed to __gitcomp_nl() right away without further processing, which means all callsites except a single one in the __git_refs2() helper.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 contrib/completion/git-completion.bash | 102 ++++++++++++++++++++-----------
 t/t9902-completion.sh                  | 106 +++++++++++++++++++++++++++++++++
 2 files changed, 173 insertions(+), 35 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index f20d4600c..7f19e2a4f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -354,6 +354,8 @@ __git_tags ()
 #    Can be the name of a configured remote, a path, or a URL.
 # 2: In addition to local refs, list unique branches from refs/remotes/ for
 #    'git checkout's tracking DWIMery (optional; ignored, if set but empty).
+#
+# Use __git_complete_refs() instead.
 __git_refs ()
 {
 	local i hash dir track="${2-}"
@@ -446,6 +448,36 @@ __git_refs ()
 	esac
 }
 
+# Completes refs, short and long, local and remote, symbolic and pseudo.
+#
+# Usage: __git_complete_refs [<option>]...
+# --remote=<remote>: The remote to list refs from, can be the name of a
+#                    configured remote, a path, or a URL.
+# --track: List unique remote branches for 'git checkout's tracking DWIMery.
+# --pfx=<prefix>: A prefix to be added to each ref.
+# --cur=<word>: The current ref to be completed.  Defaults to the current
+#               word to be completed.
+# --sfx=<suffix>: A suffix to be appended to each ref instead of the default
+#                 space.
+__git_complete_refs ()
+{
+	local remote track pfx cur_="$cur" sfx=" "
+
+	while test $# != 0; do
+		case "$1" in
+		--remote=*)	remote="${1##--remote=}" ;;
+		--track)	track="yes" ;;
+		--pfx=*)	pfx="${1##--pfx=}" ;;
+		--cur=*)	cur_="${1##--cur=}" ;;
+		--sfx=*)	sfx="${1##--sfx=}" ;;
+		*)		return 1 ;;
+		esac
+		shift
+	done
+
+	__gitcomp_nl "$(__git_refs "$remote" "$track")" "$pfx" "$cur_" "$sfx"
+}
+
 # __git_refs2 requires 1 argument (to pass to __git_refs)
 __git_refs2 ()
 {
@@ -554,15 +586,15 @@ __git_complete_revlist_file ()
 	*...*)
 		pfx="${cur_%...*}..."
 		cur_="${cur_#*...}"
-		__gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
+		__git_complete_refs --pfx="$pfx" --cur="$cur_"
 		;;
 	*..*)
 		pfx="${cur_%..*}.."
 		cur_="${cur_#*..}"
-		__gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
+		__git_complete_refs --pfx="$pfx" --cur="$cur_"
 		;;
 	*)
-		__gitcomp_nl "$(__git_refs)"
+		__git_complete_refs
 		;;
 	esac
 }
@@ -649,21 +681,21 @@ __git_complete_remote_or_refspec ()
 		if [ $lhs = 1 ]; then
 			__gitcomp_nl "$(__git_refs2 "$remote")" "$pfx" "$cur_"
 		else
-			__gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
+			__git_complete_refs --pfx="$pfx" --cur="$cur_"
 		fi
 		;;
 	pull|remote)
 		if [ $lhs = 1 ]; then
-			__gitcomp_nl "$(__git_refs "$remote")" "$pfx" "$cur_"
+			__git_complete_refs --remote="$remote" --pfx="$pfx" --cur="$cur_"
 		else
-			__gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
+			__git_complete_refs --pfx="$pfx" --cur="$cur_"
 		fi
 		;;
 	push)
 		if [ $lhs = 1 ]; then
-			__gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
+			__git_complete_refs --pfx="$pfx" --cur="$cur_"
 		else
-			__gitcomp_nl "$(__git_refs "$remote")" "$pfx" "$cur_"
+			__git_complete_refs --remote="$remote" --pfx="$pfx" --cur="$cur_"
 		fi
 		;;
 	esac
@@ -1061,7 +1093,7 @@ _git_bisect ()
 
 	case "$subcommand" in
 	bad|good|reset|skip|start)
-		__gitcomp_nl "$(__git_refs)"
+		__git_complete_refs
 		;;
 	*)
 		;;
@@ -1083,7 +1115,7 @@ _git_branch ()
 
 	case "$cur" in
 	--set-upstream-to=*)
-		__gitcomp_nl "$(__git_refs)" "" "${cur##--set-upstream-to=}"
+		__git_complete_refs --cur="${cur##--set-upstream-to=}"
 		;;
 	--*)
 		__gitcomp "
@@ -1097,7 +1129,7 @@ _git_branch ()
 		if [ $only_local_ref = "y" -a $has_r = "n" ]; then
 			__gitcomp_nl "$(__git_heads)"
 		else
-			__gitcomp_nl "$(__git_refs)"
+			__git_complete_refs
 		fi
 		;;
 	esac
@@ -1140,18 +1172,18 @@ _git_checkout ()
 	*)
 		# check if --track, --no-track, or --no-guess was specified
 		# if so, disable DWIM mode
-		local flags="--track --no-track --no-guess" track=1
+		local flags="--track --no-track --no-guess" track_opt="--track"
 		if [ -n "$(__git_find_on_cmdline "$flags")" ]; then
-			track=''
+			track_opt=''
 		fi
-		__gitcomp_nl "$(__git_refs '' $track)"
+		__git_complete_refs $track_opt
 		;;
 	esac
 }
 
 _git_cherry ()
 {
-	__gitcomp_nl "$(__git_refs)"
+	__git_complete_refs
 }
 
 _git_cherry_pick ()
@@ -1166,7 +1198,7 @@ _git_cherry_pick ()
 		__gitcomp "--edit --no-commit --signoff --strategy= --mainline"
 		;;
 	*)
-		__gitcomp_nl "$(__git_refs)"
+		__git_complete_refs
 		;;
 	esac
 }
@@ -1216,7 +1248,7 @@ _git_commit ()
 {
 	case "$prev" in
 	-c|-C)
-		__gitcomp_nl "$(__git_refs)"
+		__git_complete_refs
 		return
 		;;
 	esac
@@ -1229,7 +1261,7 @@ _git_commit ()
 		;;
 	--reuse-message=*|--reedit-message=*|\
 	--fixup=*|--squash=*)
-		__gitcomp_nl "$(__git_refs)" "" "${cur#*=}"
+		__git_complete_refs --cur="${cur#*=}"
 		return
 		;;
 	--untracked-files=*)
@@ -1267,7 +1299,7 @@ _git_describe ()
 			"
 		return
 	esac
-	__gitcomp_nl "$(__git_refs)"
+	__git_complete_refs
 }
 
 __git_diff_algorithms="myers minimal patience histogram"
@@ -1453,7 +1485,7 @@ _git_grep ()
 		;;
 	esac
 
-	__gitcomp_nl "$(__git_refs)"
+	__git_complete_refs
 }
 
 _git_help ()
@@ -1621,7 +1653,7 @@ _git_merge ()
 			--rerere-autoupdate --no-rerere-autoupdate --abort --continue"
 		return
 	esac
-	__gitcomp_nl "$(__git_refs)"
+	__git_complete_refs
 }
 
 _git_mergetool ()
@@ -1646,7 +1678,7 @@ _git_merge_base ()
 		return
 		;;
 	esac
-	__gitcomp_nl "$(__git_refs)"
+	__git_complete_refs
 }
 
 _git_mv ()
@@ -1684,7 +1716,7 @@ _git_notes ()
 	,*)
 		case "$prev" in
 		--ref)
-			__gitcomp_nl "$(__git_refs)"
+			__git_complete_refs
 			;;
 		*)
 			__gitcomp "$subcommands --ref"
@@ -1693,7 +1725,7 @@ _git_notes ()
 		;;
 	add,--reuse-message=*|append,--reuse-message=*|\
 	add,--reedit-message=*|append,--reedit-message=*)
-		__gitcomp_nl "$(__git_refs)" "" "${cur#*=}"
+		__git_complete_refs --cur="${cur#*=}"
 		;;
 	add,--*|append,--*)
 		__gitcomp '--file= --message= --reedit-message=
@@ -1712,7 +1744,7 @@ _git_notes ()
 		-m|-F)
 			;;
 		*)
-			__gitcomp_nl "$(__git_refs)"
+			__git_complete_refs
 			;;
 		esac
 		;;
@@ -1750,10 +1782,10 @@ __git_complete_force_with_lease ()
 	--*=)
 		;;
 	*:*)
-		__gitcomp_nl "$(__git_refs)" "" "${cur_#*:}"
+		__git_complete_refs --cur="${cur_#*:}"
 		;;
 	*)
-		__gitcomp_nl "$(__git_refs)" "" "$cur_"
+		__git_complete_refs --cur="$cur_"
 		;;
 	esac
 }
@@ -1829,7 +1861,7 @@ _git_rebase ()
 
 		return
 	esac
-	__gitcomp_nl "$(__git_refs)"
+	__git_complete_refs
 }
 
 _git_reflog ()
@@ -1840,7 +1872,7 @@ _git_reflog ()
 	if [ -z "$subcommand" ]; then
 		__gitcomp "$subcommands"
 	else
-		__gitcomp_nl "$(__git_refs)"
+		__git_complete_refs
 	fi
 }
 
@@ -1986,7 +2018,7 @@ _git_config ()
 		return
 		;;
 	branch.*.merge)
-		__gitcomp_nl "$(__git_refs)"
+		__git_complete_refs
 		return
 		;;
 	branch.*.rebase)
@@ -2460,7 +2492,7 @@ _git_remote ()
 
 _git_replace ()
 {
-	__gitcomp_nl "$(__git_refs)"
+	__git_complete_refs
 }
 
 _git_reset ()
@@ -2473,7 +2505,7 @@ _git_reset ()
 		return
 		;;
 	esac
-	__gitcomp_nl "$(__git_refs)"
+	__git_complete_refs
 }
 
 _git_revert ()
@@ -2489,7 +2521,7 @@ _git_revert ()
 		return
 		;;
 	esac
-	__gitcomp_nl "$(__git_refs)"
+	__git_complete_refs
 }
 
 _git_rm ()
@@ -2597,7 +2629,7 @@ _git_stash ()
 			;;
 		branch,*)
 			if [ $cword -eq 3 ]; then
-				__gitcomp_nl "$(__git_refs)";
+				__git_complete_refs
 			else
 				__gitcomp_nl "$(__git stash list \
 						| sed -n -e 's/:.*//p')"
@@ -2755,7 +2787,7 @@ _git_tag ()
 		fi
 		;;
 	*)
-		__gitcomp_nl "$(__git_refs)"
+		__git_complete_refs
 		;;
 	esac
 
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index d711bef24..50c534072 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -775,6 +775,112 @@ test_expect_success '__git_refs - unique remote branches for git checkout DWIMer
 	test_cmp expected "$actual"
 '
 
+test_expect_success '__git_complete_refs - simple' '
+	sed -e "s/Z$//g" >expected <<-EOF &&
+	HEAD Z
+	master Z
+	matching-branch Z
+	other/branch-in-other Z
+	other/master-in-other Z
+	matching-tag Z
+	EOF
+	(
+		cur= &&
+		__git_complete_refs &&
+		print_comp
+	) &&
+	test_cmp expected out
+'
+
+test_expect_success '__git_complete_refs - matching' '
+	sed -e "s/Z$//g" >expected <<-EOF &&
+	matching-branch Z
+	matching-tag Z
+	EOF
+	(
+		cur=mat &&
+		__git_complete_refs &&
+		print_comp
+	) &&
+	test_cmp expected out
+'
+
+test_expect_success '__git_complete_refs - remote' '
+	sed -e "s/Z$//g" >expected <<-EOF &&
+	HEAD Z
+	branch-in-other Z
+	master-in-other Z
+	EOF
+	(
+		cur=
+		__git_complete_refs --remote=other &&
+		print_comp
+	) &&
+	test_cmp expected out
+'
+
+test_expect_success '__git_complete_refs - track' '
+	sed -e "s/Z$//g" >expected <<-EOF &&
+	HEAD Z
+	master Z
+	matching-branch Z
+	other/branch-in-other Z
+	other/master-in-other Z
+	matching-tag Z
+	branch-in-other Z
+	master-in-other Z
+	EOF
+	(
+		cur=
+		__git_complete_refs --track &&
+		print_comp
+	) &&
+	test_cmp expected out
+'
+
+test_expect_success '__git_complete_refs - current word' '
+	sed -e "s/Z$//g" >expected <<-EOF &&
+	matching-branch Z
+	matching-tag Z
+	EOF
+	(
+		cur="--option=mat" &&
+		__git_complete_refs --cur="${cur#*=}" &&
+		print_comp
+	) &&
+	test_cmp expected out
+'
+
+test_expect_success '__git_complete_refs - prefix' '
+	sed -e "s/Z$//g" >expected <<-EOF &&
+	v1.0..matching-branch Z
+	v1.0..matching-tag Z
+	EOF
+	(
+		cur=v1.0..mat &&
+		__git_complete_refs --pfx=v1.0.. --cur=mat &&
+		print_comp
+	) &&
+	test_cmp expected out
+'
+
+test_expect_success '__git_complete_refs - suffix' '
+	cat >expected <<-EOF &&
+	HEAD.
+	master.
+	matching-branch.
+	other/branch-in-other.
+	other/master-in-other.
+	matching-tag.
+	EOF
+	(
+		cur= &&
+		__git_complete_refs --sfx=. &&
+		print_comp
+	) &&
+	test_cmp expected out
+'
+
 test_expect_success 'teardown after ref completion' '
 	git branch -d matching-branch &&
 	git tag -d matching-tag &&

-- 
2.11.0.555.g967c1bcb3
Previous: SZEDER GáborNext: SZEDER Gábor
Message 3 of 23 in “completion: speed up refs completion”
  1. 00/12 completion: speed up refs completionSZEDER Gábor, Feb 3, 2017
  2. 01/12 completion: remove redundant __gitcomp_nl() options from _git_commit()SZEDER Gábor, Feb 3, 2017
  3. 02/12 completion: wrap __git_refs() for better option parsingSZEDER Gábor, Feb 3, 2017
  4. 07/12 completion: let 'for-each-ref' and 'ls-remote' filter matching refsSZEDER Gábor, Feb 3, 2017
  5. 03/12 completion: support completing full refs after '--option=refs/<TAB>'SZEDER Gábor, Feb 3, 2017
  6. 08/12 completion: let 'for-each-ref' strip the remote name from remote branchesSZEDER Gábor, Feb 3, 2017
  7. 12/12 completion: fill COMPREPLY directly when completing refsSZEDER Gábor, Feb 3, 2017
  8. squash! completion: fill COMPREPLY directly when completing refsSZEDER Gábor, Feb 6, 2017
  9. Junio C HamanoFeb 10, 2017
  10. SZEDER GáborFeb 13, 2017
  11. Junio C HamanoFeb 13, 2017
  12. 10/12 completion: let 'for-each-ref' sort remote branches for 'checkout' DWIMerySZEDER Gábor, Feb 3, 2017
  13. 11/12 completion: list only matching symbolic and pseudorefs when completing refsSZEDER Gábor, Feb 3, 2017
  14. 06/12 completion: don't disambiguate short refsSZEDER Gábor, Feb 3, 2017
  15. 04/12 completion: support excluding full refsSZEDER Gábor, Feb 3, 2017
  16. 05/12 completion: don't disambiguate tags and branchesSZEDER Gábor, Feb 3, 2017
  17. 09/12 completion: let 'for-each-ref' filter remote branches for 'checkout' DWIMerySZEDER Gábor, Feb 3, 2017
  18. Jacob KellerFeb 3, 2017
  19. Jacob KellerFeb 4, 2017
  20. Junio C HamanoFeb 4, 2017
  21. Jacob KellerFeb 6, 2017
  22. SZEDER GáborFeb 6, 2017
  23. Jacob KellerFeb 6, 2017

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.