All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv2 00/14] completion: speed up refs completion
@ 2017-03-23 15:29 SZEDER Gábor
  2017-03-23 15:29 ` [PATCHv2 01/14] completion: remove redundant __gitcomp_nl() options from _git_commit() SZEDER Gábor
                   ` (15 more replies)
  0 siblings, 16 replies; 22+ messages in thread
From: SZEDER Gábor @ 2017-03-23 15:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor

This series is the updated version of 'sg/completion-refs-speedup'.
It speeds up refs completion for large number of refs, partly by
giving up disambiguating ambiguous refs and partly by eliminating most
of the shell processing between 'git for-each-ref' and 'ls-remote' and
Bash's completion facility.  The rest is a bit of preparatory
reorganization, cleanup and bugfixes.

Changes since v1:

  - Patch 8 (let 'for-each-ref' and 'ls-remote' filter matching refs;
    it was patch 7 in v1) was modified in two ways:
    
    * __git_refs() now does that filtering only when the ref to match
      was explicitly given as parameter, as opposed to falling back to
      the current word to be completed.  The current word might be
      something like '--opt=maste', and in the fallback case we would
      then list only refs matching '--opt=maste', which is of course
      wrong.  Most of the subsequent patches had to be adjusted
      because of conflicts.

    * patch 11 (list only matching symbolic and pseudorefs when
      completing refs) was squashed into patch 8.  There was no reason
      to keep the two patches separate, and the docstring was
      inconsistent between the two patches.

  - Patch 12 now incorporates the squash! patch I sent out earlier
    [1].

  - Patch 4 (support completing fully qualified non-fast-forward
    refspecs) is new, to fix a bug that is similar in nature to the
    one fixed in patch 3.

  - Patches 13 and 14 are new and make use of the new and faster
    __gitcomp_direct() for branches, tags, and fetch refspecs.

  - Some new tests run 'sed s/Z$//g'.  Remove that 'g', because there
    is no point to ask to replace all instances of the match, when it
    matches only at the end of line.

  - A teardown test forgot to delete a branch.

[1] - http://public-inbox.org/git/20170206181545.12869-1-szeder.dev@gmail.com/

SZEDER Gábor (14):
  completion: remove redundant __gitcomp_nl() options from _git_commit()
  completion: wrap __git_refs() for better option parsing
  completion: support completing full refs after '--option=refs/<TAB>'
  completion: support completing fully qualified non-fast-forward
    refspecs
  completion: support excluding full refs
  completion: don't disambiguate tags and branches
  completion: don't disambiguate short refs
  completion: let 'for-each-ref' and 'ls-remote' filter matching refs
  completion: let 'for-each-ref' strip the remote name from remote
    branches
  completion: let 'for-each-ref' filter remote branches for 'checkout'
    DWIMery
  completion: let 'for-each-ref' sort remote branches for 'checkout'
    DWIMery
  completion: fill COMPREPLY directly when completing refs
  completion: fill COMPREPLY directly when completing fetch refspecs
  completion: speed up branch and tag completion

 contrib/completion/git-completion.bash | 252 +++++++++++++++------
 contrib/completion/git-completion.zsh  |   9 +
 t/t9902-completion.sh                  | 387 +++++++++++++++++++++++++++++++++
 3 files changed, 577 insertions(+), 71 deletions(-)

-- 
2.12.1.485.g1616aa492

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 56ededb09..bd07d9a74 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -352,14 +352,27 @@ __git_index_files ()
 	done | sort | uniq
 }
 
+# Lists branches from the local repository.
+# 1: A prefix to be added to each listed branch (optional).
+# 2: List only branches matching this word (optional; list all branches if
+#    unset or empty).
+# 3: A suffix to be appended to each listed branch (optional).
 __git_heads ()
 {
-	__git for-each-ref --format='%(refname:strip=2)' refs/heads
+	local pfx="${1-}" cur_="${2-}" sfx="${3-}"
+
+	__git for-each-ref --format="${pfx//\%/%%}%(refname:strip=2)$sfx" \
+			"refs/heads/$cur_*" "refs/heads/$cur_*/**"
 }
 
+# Lists tags from the local repository.
+# Accepts the same positional parameters as __git_heads() above.
 __git_tags ()
 {
-	__git for-each-ref --format='%(refname:strip=2)' refs/tags
+	local pfx="${1-}" cur_="${2-}" sfx="${3-}"
+
+	__git for-each-ref --format="${pfx//\%/%%}%(refname:strip=2)$sfx" \
+			"refs/tags/$cur_*" "refs/tags/$cur_*/**"
 }
 
 # Lists refs from the local (by default) or from a remote repository.
@@ -369,8 +382,8 @@ __git_tags ()
 # 2: In addition to local refs, list unique branches from refs/remotes/ for
 #    'git checkout's tracking DWIMery (optional; ignored, if set but empty).
 # 3: A prefix to be added to each listed ref (optional).
-# 4: List only refs matching this word instead of the current word being
-#    completed (optional; NOT ignored, if empty, but lists all refs).
+# 4: List only refs matching this word (optional; list all refs if unset or
+#    empty).
 # 5: A suffix to be appended to each listed ref (optional; ignored, if set
 #    but empty).
 #
@@ -381,7 +394,8 @@ __git_refs ()
 	local list_refs_from=path remote="${1-}"
 	local format refs
 	local pfx="${3-}" cur_="${4-$cur}" sfx="${5-}"
-	local fer_pfx="${pfx//\%/%%}"
+	local match="${4-}"
+	local fer_pfx="${pfx//\%/%%}" # "escape" for-each-ref format specifiers
 
 	__git_find_repo_path
 	dir="$__git_repo_path"
@@ -409,26 +423,28 @@ __git_refs ()
 			pfx="$pfx^"
 			fer_pfx="$fer_pfx^"
 			cur_=${cur_#^}
+			match=${match#^}
 		fi
 		case "$cur_" in
 		refs|refs/*)
 			format="refname"
-			refs=("$cur_*" "$cur_*/**")
+			refs=("$match*" "$match*/**")
 			track=""
 			;;
 		*)
 			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
 				case "$i" in
-				$cur_*)	if [ -e "$dir/$i" ]; then
+				$match*)
+					if [ -e "$dir/$i" ]; then
 						echo "$pfx$i$sfx"
 					fi
 					;;
 				esac
 			done
 			format="refname:strip=2"
-			refs=("refs/tags/$cur_*" "refs/tags/$cur_*/**"
-				"refs/heads/$cur_*" "refs/heads/$cur_*/**"
-				"refs/remotes/$cur_*" "refs/remotes/$cur_*/**")
+			refs=("refs/tags/$match*" "refs/tags/$match*/**"
+				"refs/heads/$match*" "refs/heads/$match*/**"
+				"refs/remotes/$match*" "refs/remotes/$match*/**")
 			;;
 		esac
 		__git_dir="$dir" __git for-each-ref --format="$fer_pfx%($format)$sfx" \
@@ -439,14 +455,14 @@ __git_refs ()
 			# but only output if the branch name is unique
 			__git for-each-ref --format="$fer_pfx%(refname:strip=3)$sfx" \
 				--sort="refname:strip=3" \
-				"refs/remotes/*/$cur_*" "refs/remotes/*/$cur_*/**" | \
+				"refs/remotes/*/$match*" "refs/remotes/*/$match*/**" | \
 			uniq -u
 		fi
 		return
 	fi
 	case "$cur_" in
 	refs|refs/*)
-		__git ls-remote "$remote" "$cur_*" | \
+		__git ls-remote "$remote" "$match*" | \
 		while read -r hash i; do
 			case "$i" in
 			*^{}) ;;
@@ -457,19 +473,19 @@ __git_refs ()
 	*)
 		if [ "$list_refs_from" = remote ]; then
 			case "HEAD" in
-			$cur_*)	echo "${pfx}HEAD$sfx" ;;
+			$match*)	echo "${pfx}HEAD$sfx" ;;
 			esac
 			__git for-each-ref --format="$fer_pfx%(refname:strip=3)$sfx" \
-				"refs/remotes/$remote/$cur_*" \
-				"refs/remotes/$remote/$cur_*/**"
+				"refs/remotes/$remote/$match*" \
+				"refs/remotes/$remote/$match*/**"
 		else
 			local query_symref
 			case "HEAD" in
-			$cur_*)	query_symref="HEAD" ;;
+			$match*)	query_symref="HEAD" ;;
 			esac
 			__git ls-remote "$remote" $query_symref \
-				"refs/tags/$cur_*" "refs/heads/$cur_*" \
-				"refs/remotes/$cur_*" |
+				"refs/tags/$match*" "refs/heads/$match*" \
+				"refs/remotes/$match*" |
 			while read -r hash i; do
 				case "$i" in
 				*^{})	;;
@@ -513,6 +529,7 @@ __git_complete_refs ()
 }
 
 # __git_refs2 requires 1 argument (to pass to __git_refs)
+# Deprecated: use __git_complete_fetch_refspecs() instead.
 __git_refs2 ()
 {
 	local i
@@ -521,6 +538,24 @@ __git_refs2 ()
 	done
 }
 
+# Completes refspecs for fetching from a remote repository.
+# 1: The remote repository.
+# 2: A prefix to be added to each listed refspec (optional).
+# 3: The ref to be completed as a refspec instead of the current word to be
+#    completed (optional)
+# 4: A suffix to be appended to each listed refspec instead of the default
+#    space (optional).
+__git_complete_fetch_refspecs ()
+{
+	local i remote="$1" pfx="${2-}" cur_="${3-$cur}" sfx="${4- }"
+
+	__gitcomp_direct "$(
+		for i in $(__git_refs "$remote" "" "" "$cur_") ; do
+			echo "$pfx$i:$i$sfx"
+		done
+		)"
+}
+
 # __git_refs_remotes requires 1 argument (to pass to ls-remote)
 __git_refs_remotes ()
 {
@@ -713,7 +748,7 @@ __git_complete_remote_or_refspec ()
 	case "$cmd" in
 	fetch)
 		if [ $lhs = 1 ]; then
-			__gitcomp_nl "$(__git_refs2 "$remote")" "$pfx" "$cur_"
+			__git_complete_fetch_refspecs "$remote" "$pfx" "$cur_"
 		else
 			__git_complete_refs --pfx="$pfx" --cur="$cur_"
 		fi
@@ -1161,7 +1196,7 @@ _git_branch ()
 		;;
 	*)
 		if [ $only_local_ref = "y" -a $has_r = "n" ]; then
-			__gitcomp_nl "$(__git_heads)"
+			__gitcomp_direct "$(__git_heads "" "$cur" " ")"
 		else
 			__git_complete_refs
 		fi
@@ -2156,7 +2191,7 @@ _git_config ()
 		;;
 	branch.*)
 		local pfx="${cur%.*}." cur_="${cur#*.}"
-		__gitcomp_nl "$(__git_heads)" "$pfx" "$cur_" "."
+		__gitcomp_direct "$(__git_heads "$pfx" "$cur_" ".")"
 		__gitcomp_nl_append $'autosetupmerge\nautosetuprebase\n' "$pfx" "$cur_"
 		return
 		;;
@@ -2802,7 +2837,7 @@ _git_tag ()
 		i="${words[c]}"
 		case "$i" in
 		-d|-v)
-			__gitcomp_nl "$(__git_tags)"
+			__gitcomp_direct "$(__git_tags "" "$cur" " ")"
 			return
 			;;
 		-f)
@@ -2817,7 +2852,7 @@ _git_tag ()
 		;;
 	-*|tag)
 		if [ $f = 1 ]; then
-			__gitcomp_nl "$(__git_tags)"
+			__gitcomp_direct "$(__git_tags "" "$cur" " ")"
 		fi
 		;;
 	*)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index be584c069..5ed28135b 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -571,6 +571,9 @@ test_expect_success '__git_refs - full refs' '
 	cat >expected <<-EOF &&
 	refs/heads/master
 	refs/heads/matching-branch
+	refs/remotes/other/branch-in-other
+	refs/remotes/other/master-in-other
+	refs/tags/matching-tag
 	EOF
 	(
 		cur=refs/heads/ &&
@@ -636,6 +639,7 @@ test_expect_success '__git_refs - configured remote' '
 
 test_expect_success '__git_refs - configured remote - full refs' '
 	cat >expected <<-EOF &&
+	HEAD
 	refs/heads/branch-in-other
 	refs/heads/master-in-other
 	refs/tags/tag-in-other
@@ -664,6 +668,7 @@ test_expect_success '__git_refs - configured remote - repo given on the command
 
 test_expect_success '__git_refs - configured remote - full refs - repo given on the command line' '
 	cat >expected <<-EOF &&
+	HEAD
 	refs/heads/branch-in-other
 	refs/heads/master-in-other
 	refs/tags/tag-in-other
@@ -708,6 +713,7 @@ test_expect_success '__git_refs - URL remote' '
 
 test_expect_success '__git_refs - URL remote - full refs' '
 	cat >expected <<-EOF &&
+	HEAD
 	refs/heads/branch-in-other
 	refs/heads/master-in-other
 	refs/tags/tag-in-other
@@ -861,6 +867,25 @@ test_expect_success 'setup for filtering matching refs' '
 	rm -f .git/FETCH_HEAD
 '
 
+test_expect_success '__git_refs - dont filter refs unless told so' '
+	cat >expected <<-EOF &&
+	HEAD
+	master
+	matching-branch
+	matching/branch
+	other/branch-in-other
+	other/master-in-other
+	other/matching/branch-in-other
+	matching-tag
+	matching/tag
+	EOF
+	(
+		cur=master &&
+		__git_refs >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
 test_expect_success '__git_refs - only matching refs' '
 	cat >expected <<-EOF &&
 	matching-branch
@@ -870,7 +895,7 @@ test_expect_success '__git_refs - only matching refs' '
 	EOF
 	(
 		cur=mat &&
-		__git_refs >"$actual"
+		__git_refs "" "" "" "$cur" >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
@@ -882,7 +907,7 @@ test_expect_success '__git_refs - only matching refs - full refs' '
 	EOF
 	(
 		cur=refs/heads/mat &&
-		__git_refs >"$actual"
+		__git_refs "" "" "" "$cur" >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
@@ -894,7 +919,7 @@ test_expect_success '__git_refs - only matching refs - remote on local file syst
 	EOF
 	(
 		cur=ma &&
-		__git_refs otherrepo >"$actual"
+		__git_refs otherrepo "" "" "$cur" >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
@@ -906,7 +931,7 @@ test_expect_success '__git_refs - only matching refs - configured remote' '
 	EOF
 	(
 		cur=ma &&
-		__git_refs other >"$actual"
+		__git_refs other "" "" "$cur" >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
@@ -918,7 +943,7 @@ test_expect_success '__git_refs - only matching refs - remote - full refs' '
 	EOF
 	(
 		cur=refs/heads/ma &&
-		__git_refs other >"$actual"
+		__git_refs other "" "" "$cur" >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
@@ -940,7 +965,7 @@ test_expect_success '__git_refs - only matching refs - checkout DWIMery' '
 	done &&
 	(
 		cur=mat &&
-		__git_refs "" 1 >"$actual"
+		__git_refs "" 1 "" "$cur" >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
@@ -948,7 +973,8 @@ test_expect_success '__git_refs - only matching refs - checkout DWIMery' '
 test_expect_success 'teardown after filtering matching refs' '
 	git branch -d matching/branch &&
 	git tag -d matching/tag &&
-	git update-ref -d refs/remotes/other/matching/branch-in-other
+	git update-ref -d refs/remotes/other/matching/branch-in-other &&
+	git -C otherrepo branch -D matching/branch-in-other
 '
 
 test_expect_success '__git_refs - for-each-ref format specifiers in prefix' '
@@ -963,7 +989,7 @@ test_expect_success '__git_refs - for-each-ref format specifiers in prefix' '
 '
 
 test_expect_success '__git_complete_refs - simple' '
-	sed -e "s/Z$//g" >expected <<-EOF &&
+	sed -e "s/Z$//" >expected <<-EOF &&
 	HEAD Z
 	master Z
 	matching-branch Z
@@ -980,7 +1006,7 @@ test_expect_success '__git_complete_refs - simple' '
 '
 
 test_expect_success '__git_complete_refs - matching' '
-	sed -e "s/Z$//g" >expected <<-EOF &&
+	sed -e "s/Z$//" >expected <<-EOF &&
 	matching-branch Z
 	matching-tag Z
 	EOF
@@ -993,7 +1019,7 @@ test_expect_success '__git_complete_refs - matching' '
 '
 
 test_expect_success '__git_complete_refs - remote' '
-	sed -e "s/Z$//g" >expected <<-EOF &&
+	sed -e "s/Z$//" >expected <<-EOF &&
 	HEAD Z
 	branch-in-other Z
 	master-in-other Z
@@ -1007,7 +1033,7 @@ test_expect_success '__git_complete_refs - remote' '
 '
 
 test_expect_success '__git_complete_refs - track' '
-	sed -e "s/Z$//g" >expected <<-EOF &&
+	sed -e "s/Z$//" >expected <<-EOF &&
 	HEAD Z
 	master Z
 	matching-branch Z
@@ -1026,7 +1052,7 @@ test_expect_success '__git_complete_refs - track' '
 '
 
 test_expect_success '__git_complete_refs - current word' '
-	sed -e "s/Z$//g" >expected <<-EOF &&
+	sed -e "s/Z$//" >expected <<-EOF &&
 	matching-branch Z
 	matching-tag Z
 	EOF
@@ -1039,7 +1065,7 @@ test_expect_success '__git_complete_refs - current word' '
 '
 
 test_expect_success '__git_complete_refs - prefix' '
-	sed -e "s/Z$//g" >expected <<-EOF &&
+	sed -e "s/Z$//" >expected <<-EOF &&
 	v1.0..matching-branch Z
 	v1.0..matching-tag Z
 	EOF
@@ -1068,6 +1094,74 @@ test_expect_success '__git_complete_refs - suffix' '
 	test_cmp expected out
 '
 
+test_expect_success '__git_complete_fetch_refspecs - simple' '
+	sed -e "s/Z$//" >expected <<-EOF &&
+	HEAD:HEAD Z
+	branch-in-other:branch-in-other Z
+	master-in-other:master-in-other Z
+	EOF
+	(
+		cur= &&
+		__git_complete_fetch_refspecs other &&
+		print_comp
+	) &&
+	test_cmp expected out
+'
+
+test_expect_success '__git_complete_fetch_refspecs - matching' '
+	sed -e "s/Z$//" >expected <<-EOF &&
+	branch-in-other:branch-in-other Z
+	EOF
+	(
+		cur=br &&
+		__git_complete_fetch_refspecs other "" br &&
+		print_comp
+	) &&
+	test_cmp expected out
+'
+
+test_expect_success '__git_complete_fetch_refspecs - prefix' '
+	sed -e "s/Z$//" >expected <<-EOF &&
+	+HEAD:HEAD Z
+	+branch-in-other:branch-in-other Z
+	+master-in-other:master-in-other Z
+	EOF
+	(
+		cur="+" &&
+		__git_complete_fetch_refspecs other "+" ""  &&
+		print_comp
+	) &&
+	test_cmp expected out
+'
+
+test_expect_success '__git_complete_fetch_refspecs - fully qualified' '
+	sed -e "s/Z$//" >expected <<-EOF &&
+	refs/heads/branch-in-other:refs/heads/branch-in-other Z
+	refs/heads/master-in-other:refs/heads/master-in-other Z
+	refs/tags/tag-in-other:refs/tags/tag-in-other Z
+	EOF
+	(
+		cur=refs/ &&
+		__git_complete_fetch_refspecs other "" refs/ &&
+		print_comp
+	) &&
+	test_cmp expected out
+'
+
+test_expect_success '__git_complete_fetch_refspecs - fully qualified & prefix' '
+	sed -e "s/Z$//" >expected <<-EOF &&
+	+refs/heads/branch-in-other:refs/heads/branch-in-other Z
+	+refs/heads/master-in-other:refs/heads/master-in-other Z
+	+refs/tags/tag-in-other:refs/tags/tag-in-other Z
+	EOF
+	(
+		cur=+refs/ &&
+		__git_complete_fetch_refspecs other + refs/ &&
+		print_comp
+	) &&
+	test_cmp expected out
+'
+
 test_expect_success 'teardown after ref completion' '
 	git branch -d matching-branch &&
 	git tag -d matching-tag &&

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCHv2 01/14] completion: remove redundant __gitcomp_nl() options from _git_commit()
  2017-03-23 15:29 [PATCHv2 00/14] completion: speed up refs completion SZEDER Gábor
@ 2017-03-23 15:29 ` SZEDER Gábor
  2017-03-23 15:29 ` [PATCHv2 02/14] completion: wrap __git_refs() for better option parsing SZEDER Gábor
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: SZEDER Gábor @ 2017-03-23 15:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor

Those two options are specifying the default values that
__gitcomp_nl() would use anyway when invoked with no options at all.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 66d84745c..91fda7ffb 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1216,7 +1216,7 @@ _git_commit ()
 {
 	case "$prev" in
 	-c|-C)
-		__gitcomp_nl "$(__git_refs)" "" "${cur}"
+		__gitcomp_nl "$(__git_refs)"
 		return
 		;;
 	esac
-- 
2.12.1.485.g1616aa492


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCHv2 02/14] completion: wrap __git_refs() for better option parsing
  2017-03-23 15:29 [PATCHv2 00/14] completion: speed up refs completion SZEDER Gábor
  2017-03-23 15:29 ` [PATCHv2 01/14] completion: remove redundant __gitcomp_nl() options from _git_commit() SZEDER Gábor
@ 2017-03-23 15:29 ` SZEDER Gábor
  2017-03-23 15:29 ` [PATCHv2 03/14] completion: support completing full refs after '--option=refs/<TAB>' SZEDER Gábor
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: SZEDER Gábor @ 2017-03-23 15:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor

__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 91fda7ffb..0b90cfa54 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..4e7f3076f 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$//" >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$//" >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$//" >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$//" >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$//" >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$//" >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.12.1.485.g1616aa492


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCHv2 03/14] completion: support completing full refs after '--option=refs/<TAB>'
  2017-03-23 15:29 [PATCHv2 00/14] completion: speed up refs completion SZEDER Gábor
  2017-03-23 15:29 ` [PATCHv2 01/14] completion: remove redundant __gitcomp_nl() options from _git_commit() SZEDER Gábor
  2017-03-23 15:29 ` [PATCHv2 02/14] completion: wrap __git_refs() for better option parsing SZEDER Gábor
@ 2017-03-23 15:29 ` SZEDER Gábor
  2017-03-23 15:29 ` [PATCHv2 04/14] completion: support completing fully qualified non-fast-forward refspecs SZEDER Gábor
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: SZEDER Gábor @ 2017-03-23 15:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor

Completing full refs currently only works when the full ref stands on
in its own on the command line, but doesn't work when the current word
to be completed contains a prefix before the full ref, e.g.
'--option=refs/<TAB>' or 'master..refs/bis<TAB>'.

The reason is that __git_refs() looks at the current word to be
completed ($cur) as a whole to decide whether it has to list full (if
it starts with 'refs/') or short refs (otherwise).  However, $cur also
holds said '--option=' or 'master..' prefixes, which of course throw
off this decision.  Luckily, the default action is to list short refs,
that's why completing short refs happens to work even after a
'master..<TAB>' prefix and similar cases.

Pass only the ref part of the current word to be completed to
__git_refs() as a new positional parameter, so it can make the right
decision even if the whole current word contains some kind of a
prefix.

Make this new parameter the 4. positional parameter and leave the 3.
as an ignored placeholder for now (it will be used later in this patch
series).

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 contrib/completion/git-completion.bash | 21 ++++++++++++++-------
 t/t9902-completion.sh                  | 31 +++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 0b90cfa54..b897cba4b 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).
+# 3: Currently ignored.
+# 4: The current ref to be completed (optional).
 #
 # Use __git_complete_refs() instead.
 __git_refs ()
@@ -361,6 +363,7 @@ __git_refs ()
 	local i hash dir track="${2-}"
 	local list_refs_from=path remote="${1-}"
 	local format refs pfx
+	local cur_="${4-$cur}"
 
 	__git_find_repo_path
 	dir="$__git_repo_path"
@@ -384,14 +387,17 @@ __git_refs ()
 	fi
 
 	if [ "$list_refs_from" = path ]; then
-		case "$cur" in
+		case "$cur_" in
 		refs|refs/*)
 			format="refname"
-			refs="${cur%/*}"
+			refs="${cur_%/*}"
 			track=""
 			;;
 		*)
-			[[ "$cur" == ^* ]] && pfx="^"
+			if [[ "$cur_" == ^* ]]; then
+				pfx="^"
+				cur_=${cur_#^}
+			fi
 			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
 				if [ -e "$dir/$i" ]; then echo $pfx$i; fi
 			done
@@ -411,16 +417,16 @@ __git_refs ()
 			while read -r entry; do
 				eval "$entry"
 				ref="${ref#*/}"
-				if [[ "$ref" == "$cur"* ]]; then
+				if [[ "$ref" == "$cur_"* ]]; then
 					echo "$ref"
 				fi
 			done | sort | uniq -u
 		fi
 		return
 	fi
-	case "$cur" in
+	case "$cur_" in
 	refs|refs/*)
-		__git ls-remote "$remote" "$cur*" | \
+		__git ls-remote "$remote" "$cur_*" | \
 		while read -r hash i; do
 			case "$i" in
 			*^{}) ;;
@@ -475,7 +481,8 @@ __git_complete_refs ()
 		shift
 	done
 
-	__gitcomp_nl "$(__git_refs "$remote" "$track")" "$pfx" "$cur_" "$sfx"
+	__gitcomp_nl "$(__git_refs "$remote" "$track" "" "$cur_")" \
+		"$pfx" "$cur_" "$sfx"
 }
 
 # __git_refs2 requires 1 argument (to pass to __git_refs)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 4e7f3076f..0a41ee1ea 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -775,6 +775,37 @@ test_expect_success '__git_refs - unique remote branches for git checkout DWIMer
 	test_cmp expected "$actual"
 '
 
+test_expect_success '__git_refs - after --opt=' '
+	cat >expected <<-EOF &&
+	HEAD
+	master
+	matching-branch
+	other/branch-in-other
+	other/master-in-other
+	matching-tag
+	EOF
+	(
+		cur="--opt=" &&
+		__git_refs "" "" "" "" >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - after --opt= - full refs' '
+	cat >expected <<-EOF &&
+	refs/heads/master
+	refs/heads/matching-branch
+	refs/remotes/other/branch-in-other
+	refs/remotes/other/master-in-other
+	refs/tags/matching-tag
+	EOF
+	(
+		cur="--opt=refs/" &&
+		__git_refs "" "" "" refs/ >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
 test_expect_success '__git_complete_refs - simple' '
 	sed -e "s/Z$//" >expected <<-EOF &&
 	HEAD Z
-- 
2.12.1.485.g1616aa492


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCHv2 04/14] completion: support completing fully qualified non-fast-forward refspecs
  2017-03-23 15:29 [PATCHv2 00/14] completion: speed up refs completion SZEDER Gábor
                   ` (2 preceding siblings ...)
  2017-03-23 15:29 ` [PATCHv2 03/14] completion: support completing full refs after '--option=refs/<TAB>' SZEDER Gábor
@ 2017-03-23 15:29 ` SZEDER Gábor
  2017-03-23 15:29 ` [PATCHv2 05/14] completion: support excluding full refs SZEDER Gábor
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: SZEDER Gábor @ 2017-03-23 15:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor

After 'git fetch <remote> <TAB>' our completion script offers refspecs
that will fetch to a local branch with the same name as in the remote
repository, e.g. 'master:master'.  This also completes
non-fast-forward refspecs, i.e. after a '+' prefix like
'+master:master', and fully qualified refspecs, e.g.
'refs/heads/master:refs/heads/master'.  However, it does not complete
non-fast-forward fully qualified refspecs (or fully qualified refspecs
following any other prefix, e.g. '--option=', though currently no git
command supports such an option, but third party git commands might).

These refspecs are listed by the __git_refs2() function, which is just
a thin wrapper iterating over __git_refs()'s output, turning each
listed ref into a refspec.  Now, it's certainly possible to modify
__git_refs2() and its callsite to pass an extra parameter containing
only the ref part of the current word to be completed (to follow suit
of the previous commit) to deal with prefixed fully qualified refspecs
as well.  Unfortunately, keeping the current behavior unchanged in the
"no extra parameter" case brings in a bit of subtlety, which makes the
resulting code ugly and compelled me to write a 8-line long comment in
the proof of concept.  Not good.  However, since the callsite has to
be modified for proper functioning anyway, we might as well leave
__git_refs2() as is and introduce a new helper function without
backwards compatibility concerns.

Add the new function __git_complete_fetch_refspecs() that has all the
necessary parameters to do the right thing in all cases mentioned
above, including non-fast-forward fully qualified refspecs.  This new
function can also easier benefit from optimizations coming later in
this patch series.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 contrib/completion/git-completion.bash | 21 ++++++++++-
 t/t9902-completion.sh                  | 68 ++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index b897cba4b..067dff823 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -486,6 +486,7 @@ __git_complete_refs ()
 }
 
 # __git_refs2 requires 1 argument (to pass to __git_refs)
+# Deprecated: use __git_complete_fetch_refspecs() instead.
 __git_refs2 ()
 {
 	local i
@@ -494,6 +495,24 @@ __git_refs2 ()
 	done
 }
 
+# Completes refspecs for fetching from a remote repository.
+# 1: The remote repository.
+# 2: A prefix to be added to each listed refspec (optional).
+# 3: The ref to be completed as a refspec instead of the current word to be
+#    completed (optional)
+# 4: A suffix to be appended to each listed refspec instead of the default
+#    space (optional).
+__git_complete_fetch_refspecs ()
+{
+	local i remote="$1" pfx="${2-}" cur_="${3-$cur}" sfx="${4- }"
+
+	__gitcomp_nl "$(
+		for i in $(__git_refs "$remote" "" "" "$cur_") ; do
+			echo "$i:$i"
+		done
+		)" "$pfx" "$cur_" "$sfx"
+}
+
 # __git_refs_remotes requires 1 argument (to pass to ls-remote)
 __git_refs_remotes ()
 {
@@ -686,7 +705,7 @@ __git_complete_remote_or_refspec ()
 	case "$cmd" in
 	fetch)
 		if [ $lhs = 1 ]; then
-			__gitcomp_nl "$(__git_refs2 "$remote")" "$pfx" "$cur_"
+			__git_complete_fetch_refspecs "$remote" "$pfx" "$cur_"
 		else
 			__git_complete_refs --pfx="$pfx" --cur="$cur_"
 		fi
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 0a41ee1ea..f641d99ec 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -912,6 +912,74 @@ test_expect_success '__git_complete_refs - suffix' '
 	test_cmp expected out
 '
 
+test_expect_success '__git_complete_fetch_refspecs - simple' '
+	sed -e "s/Z$//" >expected <<-EOF &&
+	HEAD:HEAD Z
+	branch-in-other:branch-in-other Z
+	master-in-other:master-in-other Z
+	EOF
+	(
+		cur= &&
+		__git_complete_fetch_refspecs other &&
+		print_comp
+	) &&
+	test_cmp expected out
+'
+
+test_expect_success '__git_complete_fetch_refspecs - matching' '
+	sed -e "s/Z$//" >expected <<-EOF &&
+	branch-in-other:branch-in-other Z
+	EOF
+	(
+		cur=br &&
+		__git_complete_fetch_refspecs other "" br &&
+		print_comp
+	) &&
+	test_cmp expected out
+'
+
+test_expect_success '__git_complete_fetch_refspecs - prefix' '
+	sed -e "s/Z$//" >expected <<-EOF &&
+	+HEAD:HEAD Z
+	+branch-in-other:branch-in-other Z
+	+master-in-other:master-in-other Z
+	EOF
+	(
+		cur="+" &&
+		__git_complete_fetch_refspecs other "+" ""  &&
+		print_comp
+	) &&
+	test_cmp expected out
+'
+
+test_expect_success '__git_complete_fetch_refspecs - fully qualified' '
+	sed -e "s/Z$//" >expected <<-EOF &&
+	refs/heads/branch-in-other:refs/heads/branch-in-other Z
+	refs/heads/master-in-other:refs/heads/master-in-other Z
+	refs/tags/tag-in-other:refs/tags/tag-in-other Z
+	EOF
+	(
+		cur=refs/ &&
+		__git_complete_fetch_refspecs other "" refs/ &&
+		print_comp
+	) &&
+	test_cmp expected out
+'
+
+test_expect_success '__git_complete_fetch_refspecs - fully qualified & prefix' '
+	sed -e "s/Z$//" >expected <<-EOF &&
+	+refs/heads/branch-in-other:refs/heads/branch-in-other Z
+	+refs/heads/master-in-other:refs/heads/master-in-other Z
+	+refs/tags/tag-in-other:refs/tags/tag-in-other Z
+	EOF
+	(
+		cur=+refs/ &&
+		__git_complete_fetch_refspecs other + refs/ &&
+		print_comp
+	) &&
+	test_cmp expected out
+'
+
 test_expect_success 'teardown after ref completion' '
 	git branch -d matching-branch &&
 	git tag -d matching-tag &&
-- 
2.12.1.485.g1616aa492


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCHv2 05/14] completion: support excluding full refs
  2017-03-23 15:29 [PATCHv2 00/14] completion: speed up refs completion SZEDER Gábor
                   ` (3 preceding siblings ...)
  2017-03-23 15:29 ` [PATCHv2 04/14] completion: support completing fully qualified non-fast-forward refspecs SZEDER Gábor
@ 2017-03-23 15:29 ` SZEDER Gábor
  2017-03-23 15:29 ` [PATCHv2 06/14] completion: don't disambiguate tags and branches SZEDER Gábor
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: SZEDER Gábor @ 2017-03-23 15:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor

Commit 49416ad22 (completion: support excluding refs, 2016-08-24) made
possible to complete short refs with a '^' prefix.

Extend the support to full refs to make completing '^refs/...' work.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 contrib/completion/git-completion.bash |  8 ++++----
 t/t9902-completion.sh                  | 31 +++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 067dff823..882635f97 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -387,6 +387,10 @@ __git_refs ()
 	fi
 
 	if [ "$list_refs_from" = path ]; then
+		if [[ "$cur_" == ^* ]]; then
+			pfx="^"
+			cur_=${cur_#^}
+		fi
 		case "$cur_" in
 		refs|refs/*)
 			format="refname"
@@ -394,10 +398,6 @@ __git_refs ()
 			track=""
 			;;
 		*)
-			if [[ "$cur_" == ^* ]]; then
-				pfx="^"
-				cur_=${cur_#^}
-			fi
 			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
 				if [ -e "$dir/$i" ]; then echo $pfx$i; fi
 			done
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index f641d99ec..e2b45f625 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -806,6 +806,37 @@ test_expect_success '__git_refs - after --opt= - full refs' '
 	test_cmp expected "$actual"
 '
 
+test_expect_success '__git refs - exluding refs' '
+	cat >expected <<-EOF &&
+	^HEAD
+	^master
+	^matching-branch
+	^other/branch-in-other
+	^other/master-in-other
+	^matching-tag
+	EOF
+	(
+		cur=^ &&
+		__git_refs >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__git refs - exluding full refs' '
+	cat >expected <<-EOF &&
+	^refs/heads/master
+	^refs/heads/matching-branch
+	^refs/remotes/other/branch-in-other
+	^refs/remotes/other/master-in-other
+	^refs/tags/matching-tag
+	EOF
+	(
+		cur=^refs/ &&
+		__git_refs >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
 test_expect_success '__git_complete_refs - simple' '
 	sed -e "s/Z$//" >expected <<-EOF &&
 	HEAD Z
-- 
2.12.1.485.g1616aa492


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCHv2 06/14] completion: don't disambiguate tags and branches
  2017-03-23 15:29 [PATCHv2 00/14] completion: speed up refs completion SZEDER Gábor
                   ` (4 preceding siblings ...)
  2017-03-23 15:29 ` [PATCHv2 05/14] completion: support excluding full refs SZEDER Gábor
@ 2017-03-23 15:29 ` SZEDER Gábor
  2017-03-23 15:29 ` [PATCHv2 07/14] completion: don't disambiguate short refs SZEDER Gábor
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: SZEDER Gábor @ 2017-03-23 15:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor

When the completion script has to list only tags or only branches, it
uses the 'git for-each-ref' format 'refname:short', which makes sure
that all listed tags and branches are unambiguous.  However,
disambiguating tags and branches in these cases is wrong, because:

  - __git_tags(), the helper function listing possible tagname
    arguments for 'git tag', lists an ambiguous tag
    'refs/tags/ambiguous' as 'tags/ambiguous'.  Its only consumer,
    'git tag' expects its tagname argument to be under 'refs/tags/',
    thus it interprets that abgiguous tag as
    'refs/tags/tags/ambiguous'.  Clearly wrong.

  - __git_heads() lists possible branchname arguments for 'git branch'
    and possible 'branch.<branchname>' configuration subsections.
    Both of these expect branchnames to be under 'refs/heads/' and
    misinterpret a disambiguated branchname like 'heads/ambiguous'.

Furthermore, disambiguation involves several stat() syscalls for each
tag or branch, thus comes at a steep cost especially on Windows and/or
when there are a lot of tags or branches to be listed.

Use the 'git for-each-ref' format 'refname:strip=2' instead of
'refname:short' to avoid harmful disambiguation of tags and branches
in __git_tags() and __git_heads().

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 contrib/completion/git-completion.bash | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 882635f97..e129f674e 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -340,12 +340,12 @@ __git_index_files ()
 
 __git_heads ()
 {
-	__git for-each-ref --format='%(refname:short)' refs/heads
+	__git for-each-ref --format='%(refname:strip=2)' refs/heads
 }
 
 __git_tags ()
 {
-	__git for-each-ref --format='%(refname:short)' refs/tags
+	__git for-each-ref --format='%(refname:strip=2)' refs/tags
 }
 
 # Lists refs from the local (by default) or from a remote repository.
-- 
2.12.1.485.g1616aa492


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCHv2 07/14] completion: don't disambiguate short refs
  2017-03-23 15:29 [PATCHv2 00/14] completion: speed up refs completion SZEDER Gábor
                   ` (5 preceding siblings ...)
  2017-03-23 15:29 ` [PATCHv2 06/14] completion: don't disambiguate tags and branches SZEDER Gábor
@ 2017-03-23 15:29 ` SZEDER Gábor
  2017-03-24 19:31   ` Jeff King
  2017-03-23 15:29 ` [PATCHv2 08/14] completion: let 'for-each-ref' and 'ls-remote' filter matching refs SZEDER Gábor
                   ` (8 subsequent siblings)
  15 siblings, 1 reply; 22+ messages in thread
From: SZEDER Gábor @ 2017-03-23 15:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor

When the completion script lists short refs it does so using the 'git
for-each-ref' format 'refname:short', which makes sure that all listed
refs are unambiguous.  While disambiguating refs is technically
correct in this case, as opposed to the cases discussed in the
previous patch, this disambiguation involves several stat() syscalls
for each ref, thus, unfortunately, comes at a steep cost especially on
Windows and/or when there are a lot of refs to be listed.  A user of
Git for Windows reported[1] 'git checkout <TAB>' taking ~11 seconds in
a repository with just about 4000 refs.

However, it's questionable whether ambiguous refs are really that bad
to justify that much extra cost:

  - Ambiguous refs are not that common,
  - even if a repository contains ambiguous refs, they only hurt when
    the user actually happens to want to do something with one of the
    ambiguous refs, and
  - the issue can be easily circumvented by renaming those ambiguous
    refs.

  - On the other hand, apparently not that many refs are needed to
    make refs completion unacceptably slow on Windows,
  - and this slowness bites each and every time the user attempts refs
    completion, even when the repository doesn't contain any ambiguous
    refs.
  - Furthermore, circumventing the issue might not be possible or
    might be considerably more difficult and requires various
    trade-offs (e.g. working in a repository with only a few selected
    important refs while keeping a separate repository with all refs
    for reference).

Arguably, in this case the benefits of technical correctness are
rather minor compared to the price we pay for it, and we are better
off opting for performance over correctness.

Use the 'git for-each-ref' format 'refname:strip=2' to list short refs
to spare the substantial cost of disambiguating.

This speeds up refs completion considerably.  Uniquely completing a
branch in a repository with 100k local branches, all packed, best of
five:

  On Linux, before:

    $ time __git_complete_refs --cur=maste

    real    0m1.662s
    user    0m1.368s
    sys     0m0.296s

  After:

    real    0m0.831s
    user    0m0.808s
    sys     0m0.028s

  On Windows, before:

    real    0m12.457s
    user    0m1.016s
    sys     0m0.092s

  After:

    real    0m1.480s
    user    0m1.031s
    sys     0m0.060s

[1] - https://github.com/git-for-windows/git/issues/524

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 contrib/completion/git-completion.bash | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index e129f674e..5ee35d530 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -401,7 +401,7 @@ __git_refs ()
 			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
 				if [ -e "$dir/$i" ]; then echo $pfx$i; fi
 			done
-			format="refname:short"
+			format="refname:strip=2"
 			refs="refs/tags refs/heads refs/remotes"
 			;;
 		esac
@@ -412,7 +412,7 @@ __git_refs ()
 			# Try to find a remote branch that matches the completion word
 			# but only output if the branch name is unique
 			local ref entry
-			__git for-each-ref --shell --format="ref=%(refname:short)" \
+			__git for-each-ref --shell --format="ref=%(refname:strip=2)" \
 				"refs/remotes/" | \
 			while read -r entry; do
 				eval "$entry"
@@ -437,7 +437,7 @@ __git_refs ()
 	*)
 		if [ "$list_refs_from" = remote ]; then
 			echo "HEAD"
-			__git for-each-ref --format="%(refname:short)" \
+			__git for-each-ref --format="%(refname:strip=2)" \
 				"refs/remotes/$remote/" | sed -e "s#^$remote/##"
 		else
 			__git ls-remote "$remote" HEAD \
-- 
2.12.1.485.g1616aa492


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCHv2 08/14] completion: let 'for-each-ref' and 'ls-remote' filter matching refs
  2017-03-23 15:29 [PATCHv2 00/14] completion: speed up refs completion SZEDER Gábor
                   ` (6 preceding siblings ...)
  2017-03-23 15:29 ` [PATCHv2 07/14] completion: don't disambiguate short refs SZEDER Gábor
@ 2017-03-23 15:29 ` SZEDER Gábor
  2017-03-24 19:42   ` Jeff King
  2017-03-23 15:29 ` [PATCHv2 09/14] completion: let 'for-each-ref' strip the remote name from remote branches SZEDER Gábor
                   ` (7 subsequent siblings)
  15 siblings, 1 reply; 22+ messages in thread
From: SZEDER Gábor @ 2017-03-23 15:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor

When completing refs, several __git_refs() code paths list all the
refs from the refs/{heads,tags,remotes}/ hierarchy and then
__gitcomp_nl() iterates over those refs in a shell loop to filter out
refs not matching the current ref to be completed.  This comes with a
considerable performance penalty when a repository contains a lot of
refs but the current ref can be uniquely completed or when only a
handful of refs match the current ref.

Reduce the number of iterations in __gitcomp_nl() from the number of
refs to the number of matching refs by specifying appropriate globbing
patterns to 'git for-each-ref' and 'git ls-remote' to list only those
refs that match the current ref to be completed.  However, do so only
when the ref to match is explicitly given as parameter, because the
current word on the command line might contain a prefix like
'--option=' or 'branch..'.  The __git_complete_refs() and
__git_complete_fetch_refspecs() helpers introduced previously in this
patch series already call __git_refs() specifying this current ref
parameter, so all their callsites, i.e. all places in the completion
script doing refs completion, can benefit from this optimization.

Furthermore, list only those symbolic and pseudo refs that match the
current ref to be completed.  Though it doesn't matter at all in
itself performance-wise, it will allow us further significant
optimizations later in this series.

This speeds up refs completion considerably when there are a lot of
non-matching refs to be filtered out.  Uniquely completing a branch in
a repository with 100k local branches, all packed, best of five:

  On Linux, before:

    $ time __git_complete_refs --cur=maste

    real    0m0.831s
    user    0m0.808s
    sys     0m0.028s

  After:

    real    0m0.119s
    user    0m0.104s
    sys     0m0.008s

  On Windows, before:

    real    0m1.480s
    user    0m1.031s
    sys     0m0.060s

  After:

    real    0m0.377s
    user    0m0.015s
    sys     0m0.030s

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 contrib/completion/git-completion.bash |  41 ++++++++---
 t/t9902-completion.sh                  | 124 +++++++++++++++++++++++++++++++++
 2 files changed, 154 insertions(+), 11 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 5ee35d530..976f80598 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -355,7 +355,8 @@ __git_tags ()
 # 2: In addition to local refs, list unique branches from refs/remotes/ for
 #    'git checkout's tracking DWIMery (optional; ignored, if set but empty).
 # 3: Currently ignored.
-# 4: The current ref to be completed (optional).
+# 4: List only refs matching this word (optional; list all refs if unset or
+#    empty).
 #
 # Use __git_complete_refs() instead.
 __git_refs ()
@@ -364,6 +365,7 @@ __git_refs ()
 	local list_refs_from=path remote="${1-}"
 	local format refs pfx
 	local cur_="${4-$cur}"
+	local match="${4-}"
 
 	__git_find_repo_path
 	dir="$__git_repo_path"
@@ -390,23 +392,32 @@ __git_refs ()
 		if [[ "$cur_" == ^* ]]; then
 			pfx="^"
 			cur_=${cur_#^}
+			match=${match#^}
 		fi
 		case "$cur_" in
 		refs|refs/*)
 			format="refname"
-			refs="${cur_%/*}"
+			refs=("$match*" "$match*/**")
 			track=""
 			;;
 		*)
 			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
-				if [ -e "$dir/$i" ]; then echo $pfx$i; fi
+				case "$i" in
+				$match*)
+					if [ -e "$dir/$i" ]; then
+						echo $pfx$i
+					fi
+					;;
+				esac
 			done
 			format="refname:strip=2"
-			refs="refs/tags refs/heads refs/remotes"
+			refs=("refs/tags/$match*" "refs/tags/$match*/**"
+				"refs/heads/$match*" "refs/heads/$match*/**"
+				"refs/remotes/$match*" "refs/remotes/$match*/**")
 			;;
 		esac
 		__git_dir="$dir" __git for-each-ref --format="$pfx%($format)" \
-			$refs
+			"${refs[@]}"
 		if [ -n "$track" ]; then
 			# employ the heuristic used by git checkout
 			# Try to find a remote branch that matches the completion word
@@ -417,7 +428,7 @@ __git_refs ()
 			while read -r entry; do
 				eval "$entry"
 				ref="${ref#*/}"
-				if [[ "$ref" == "$cur_"* ]]; then
+				if [[ "$ref" == "$match"* ]]; then
 					echo "$ref"
 				fi
 			done | sort | uniq -u
@@ -426,7 +437,7 @@ __git_refs ()
 	fi
 	case "$cur_" in
 	refs|refs/*)
-		__git ls-remote "$remote" "$cur_*" | \
+		__git ls-remote "$remote" "$match*" | \
 		while read -r hash i; do
 			case "$i" in
 			*^{}) ;;
@@ -436,12 +447,20 @@ __git_refs ()
 		;;
 	*)
 		if [ "$list_refs_from" = remote ]; then
-			echo "HEAD"
+			case "HEAD" in
+			$match*)	echo "HEAD" ;;
+			esac
 			__git for-each-ref --format="%(refname:strip=2)" \
-				"refs/remotes/$remote/" | sed -e "s#^$remote/##"
+				"refs/remotes/$remote/$match*" \
+				"refs/remotes/$remote/$match*/**" | sed -e "s#^$remote/##"
 		else
-			__git ls-remote "$remote" HEAD \
-				"refs/tags/*" "refs/heads/*" "refs/remotes/*" |
+			local query_symref
+			case "HEAD" in
+			$match*)	query_symref="HEAD" ;;
+			esac
+			__git ls-remote "$remote" $query_symref \
+				"refs/tags/$match*" "refs/heads/$match*" \
+				"refs/remotes/$match*" |
 			while read -r hash i; do
 				case "$i" in
 				*^{})	;;
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index e2b45f625..cc9e741f9 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -555,6 +555,9 @@ test_expect_success '__git_refs - full refs' '
 	cat >expected <<-EOF &&
 	refs/heads/master
 	refs/heads/matching-branch
+	refs/remotes/other/branch-in-other
+	refs/remotes/other/master-in-other
+	refs/tags/matching-tag
 	EOF
 	(
 		cur=refs/heads/ &&
@@ -620,6 +623,7 @@ test_expect_success '__git_refs - configured remote' '
 
 test_expect_success '__git_refs - configured remote - full refs' '
 	cat >expected <<-EOF &&
+	HEAD
 	refs/heads/branch-in-other
 	refs/heads/master-in-other
 	refs/tags/tag-in-other
@@ -648,6 +652,7 @@ test_expect_success '__git_refs - configured remote - repo given on the command
 
 test_expect_success '__git_refs - configured remote - full refs - repo given on the command line' '
 	cat >expected <<-EOF &&
+	HEAD
 	refs/heads/branch-in-other
 	refs/heads/master-in-other
 	refs/tags/tag-in-other
@@ -692,6 +697,7 @@ test_expect_success '__git_refs - URL remote' '
 
 test_expect_success '__git_refs - URL remote - full refs' '
 	cat >expected <<-EOF &&
+	HEAD
 	refs/heads/branch-in-other
 	refs/heads/master-in-other
 	refs/tags/tag-in-other
@@ -837,6 +843,124 @@ test_expect_success '__git refs - exluding full refs' '
 	test_cmp expected "$actual"
 '
 
+test_expect_success 'setup for filtering matching refs' '
+	git branch matching/branch &&
+	git tag matching/tag &&
+	git -C otherrepo branch matching/branch-in-other &&
+	git fetch --no-tags other &&
+	rm -f .git/FETCH_HEAD
+'
+
+test_expect_success '__git_refs - dont filter refs unless told so' '
+	cat >expected <<-EOF &&
+	HEAD
+	master
+	matching-branch
+	matching/branch
+	other/branch-in-other
+	other/master-in-other
+	other/matching/branch-in-other
+	matching-tag
+	matching/tag
+	EOF
+	(
+		cur=master &&
+		__git_refs >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - only matching refs' '
+	cat >expected <<-EOF &&
+	matching-branch
+	matching/branch
+	matching-tag
+	matching/tag
+	EOF
+	(
+		cur=mat &&
+		__git_refs "" "" "" "$cur" >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - only matching refs - full refs' '
+	cat >expected <<-EOF &&
+	refs/heads/matching-branch
+	refs/heads/matching/branch
+	EOF
+	(
+		cur=refs/heads/mat &&
+		__git_refs "" "" "" "$cur" >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - only matching refs - remote on local file system' '
+	cat >expected <<-EOF &&
+	master-in-other
+	matching/branch-in-other
+	EOF
+	(
+		cur=ma &&
+		__git_refs otherrepo "" "" "$cur" >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - only matching refs - configured remote' '
+	cat >expected <<-EOF &&
+	master-in-other
+	matching/branch-in-other
+	EOF
+	(
+		cur=ma &&
+		__git_refs other "" "" "$cur" >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - only matching refs - remote - full refs' '
+	cat >expected <<-EOF &&
+	refs/heads/master-in-other
+	refs/heads/matching/branch-in-other
+	EOF
+	(
+		cur=refs/heads/ma &&
+		__git_refs other "" "" "$cur" >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - only matching refs - checkout DWIMery' '
+	cat >expected <<-EOF &&
+	matching-branch
+	matching/branch
+	matching-tag
+	matching/tag
+	matching/branch-in-other
+	EOF
+	for remote_ref in refs/remotes/other/ambiguous \
+		refs/remotes/remote/ambiguous \
+		refs/remotes/remote/branch-in-remote
+	do
+		git update-ref $remote_ref master &&
+		test_when_finished "git update-ref -d $remote_ref"
+	done &&
+	(
+		cur=mat &&
+		__git_refs "" 1 "" "$cur" >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'teardown after filtering matching refs' '
+	git branch -d matching/branch &&
+	git tag -d matching/tag &&
+	git update-ref -d refs/remotes/other/matching/branch-in-other &&
+	git -C otherrepo branch -D matching/branch-in-other
+'
+
 test_expect_success '__git_complete_refs - simple' '
 	sed -e "s/Z$//" >expected <<-EOF &&
 	HEAD Z
-- 
2.12.1.485.g1616aa492


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCHv2 09/14] completion: let 'for-each-ref' strip the remote name from remote branches
  2017-03-23 15:29 [PATCHv2 00/14] completion: speed up refs completion SZEDER Gábor
                   ` (7 preceding siblings ...)
  2017-03-23 15:29 ` [PATCHv2 08/14] completion: let 'for-each-ref' and 'ls-remote' filter matching refs SZEDER Gábor
@ 2017-03-23 15:29 ` SZEDER Gábor
  2017-03-23 15:29 ` [PATCHv2 10/14] completion: let 'for-each-ref' filter remote branches for 'checkout' DWIMery SZEDER Gábor
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: SZEDER Gábor @ 2017-03-23 15:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor

The code listing unique remote branches for 'git checkout's tracking
DWIMery uses a shell parameter expansion in a loop iterating over each
listed ref to remove the remote's name from the remote branches, i.e.
the leading path component from the short ref.  When listing refs from
a configured remote repository, '| sed s///' is used for the same
purpose.

Let 'git for-each-ref' strip one more leading path component from the
refs, i.e. use the format 'refname:strip=3' instead of '=2', making
that parameter expansion and 'sed' execution unnecessary.

This speeds up refs completion for 'git checkout'.  Uniquely
completing a branch for 'git checkout maste<TAB>' in a repo with 100k
remote branches, all packed, best of five:

  On Linux, near the beginning of this series, for reference:

    $ time __git_complete_refs --cur=maste --track

    real    0m8.185s
    user    0m6.896s
    sys     0m1.616s

  Before this patch:

    real    0m2.714s
    user    0m2.344s
    sys     0m0.436s

  After:

    real    0m1.993s
    user    0m1.740s
    sys     0m0.304s

  On Windows, near the beginning:

    real    1m8.421s
    user    0m7.591s
    sys     0m3.557s

  Before this patch:

    real    0m8.191s
    user    0m4.638s
    sys     0m2.918s

  After:

    real    0m6.187s
    user    0m3.358s
    sys     0m2.121s

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 contrib/completion/git-completion.bash | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 976f80598..206eaf0ca 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -423,11 +423,10 @@ __git_refs ()
 			# Try to find a remote branch that matches the completion word
 			# but only output if the branch name is unique
 			local ref entry
-			__git for-each-ref --shell --format="ref=%(refname:strip=2)" \
+			__git for-each-ref --shell --format="ref=%(refname:strip=3)" \
 				"refs/remotes/" | \
 			while read -r entry; do
 				eval "$entry"
-				ref="${ref#*/}"
 				if [[ "$ref" == "$match"* ]]; then
 					echo "$ref"
 				fi
@@ -450,9 +449,9 @@ __git_refs ()
 			case "HEAD" in
 			$match*)	echo "HEAD" ;;
 			esac
-			__git for-each-ref --format="%(refname:strip=2)" \
+			__git for-each-ref --format="%(refname:strip=3)" \
 				"refs/remotes/$remote/$match*" \
-				"refs/remotes/$remote/$match*/**" | sed -e "s#^$remote/##"
+				"refs/remotes/$remote/$match*/**"
 		else
 			local query_symref
 			case "HEAD" in
-- 
2.12.1.485.g1616aa492


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCHv2 10/14] completion: let 'for-each-ref' filter remote branches for 'checkout' DWIMery
  2017-03-23 15:29 [PATCHv2 00/14] completion: speed up refs completion SZEDER Gábor
                   ` (8 preceding siblings ...)
  2017-03-23 15:29 ` [PATCHv2 09/14] completion: let 'for-each-ref' strip the remote name from remote branches SZEDER Gábor
@ 2017-03-23 15:29 ` SZEDER Gábor
  2017-03-23 15:29 ` [PATCHv2 11/14] completion: let 'for-each-ref' sort " SZEDER Gábor
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: SZEDER Gábor @ 2017-03-23 15:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor

The code listing unique remote branches for 'git checkout's tracking
DWIMery outputs only remote branches that match the current word to be
completed, but the filtering is done in a shell loop iterating over
all remote refs.

Let 'git for-each-ref' do the filtering, as it can do so much more
efficiently and we can remove that shell loop entirely.

This speeds up refs completion for 'git checkout' considerably when
there are a lot of non-matching remote refs to be filtered out.
Uniquely completing a branch in a repository with 100k remote
branches, all packed, best of five:

  On Linux, before:

    $ time __git_complete_refs --cur=maste --track

    real    0m1.993s
    user    0m1.740s
    sys     0m0.304s

  After:

    real    0m0.266s
    user    0m0.248s
    sys     0m0.012s

  On Windows, before:

    real    0m6.187s
    user    0m3.358s
    sys     0m2.121s

  After:

    real    0m0.750s
    user    0m0.015s
    sys     0m0.090s

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 contrib/completion/git-completion.bash | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 206eaf0ca..394dcece6 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -422,15 +422,9 @@ __git_refs ()
 			# employ the heuristic used by git checkout
 			# Try to find a remote branch that matches the completion word
 			# but only output if the branch name is unique
-			local ref entry
-			__git for-each-ref --shell --format="ref=%(refname:strip=3)" \
-				"refs/remotes/" | \
-			while read -r entry; do
-				eval "$entry"
-				if [[ "$ref" == "$match"* ]]; then
-					echo "$ref"
-				fi
-			done | sort | uniq -u
+			__git for-each-ref --format="%(refname:strip=3)" \
+				"refs/remotes/*/$match*" "refs/remotes/*/$match*/**" | \
+			sort | uniq -u
 		fi
 		return
 	fi
-- 
2.12.1.485.g1616aa492


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCHv2 11/14] completion: let 'for-each-ref' sort remote branches for 'checkout' DWIMery
  2017-03-23 15:29 [PATCHv2 00/14] completion: speed up refs completion SZEDER Gábor
                   ` (9 preceding siblings ...)
  2017-03-23 15:29 ` [PATCHv2 10/14] completion: let 'for-each-ref' filter remote branches for 'checkout' DWIMery SZEDER Gábor
@ 2017-03-23 15:29 ` SZEDER Gábor
  2017-03-24 19:53   ` Jeff King
  2017-03-23 15:29 ` [PATCHv2 12/14] completion: fill COMPREPLY directly when completing refs SZEDER Gábor
                   ` (4 subsequent siblings)
  15 siblings, 1 reply; 22+ messages in thread
From: SZEDER Gábor @ 2017-03-23 15:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor

When listing unique remote branches for 'git checkout's tracking
DWIMery, __git_refs() runs the classic '... |sort |uniq -u' pattern to
filter out duplicate remote branches.

Let 'git for-each-ref' do the sorting, sparing the overhead of
fork()+exec()ing 'sort' and a stage in the pipeline where potentially
relatively large amount of data can be passed between two subsequent
pipeline stages.

This speeds up refs completion for 'git checkout' a bit when a lot of
remote branches match the current word to be completed.  Listing a
single local and 100k remote branches, all packed, best of five:

  On Linux, before:

    $ time __git_complete_refs --track

    real    0m1.856s
    user    0m1.816s
    sys     0m0.060s

  After:

    real    0m1.550s
    user    0m1.512s
    sys     0m0.060s

  On Windows, before:

    real    0m3.128s
    user    0m2.155s
    sys     0m0.183s

  After:

    real    0m2.781s
    user    0m1.826s
    sys     0m0.136s

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 contrib/completion/git-completion.bash | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 394dcece6..d26312899 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -423,8 +423,9 @@ __git_refs ()
 			# Try to find a remote branch that matches the completion word
 			# but only output if the branch name is unique
 			__git for-each-ref --format="%(refname:strip=3)" \
+				--sort="refname:strip=3" \
 				"refs/remotes/*/$match*" "refs/remotes/*/$match*/**" | \
-			sort | uniq -u
+			uniq -u
 		fi
 		return
 	fi
-- 
2.12.1.485.g1616aa492


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCHv2 12/14] completion: fill COMPREPLY directly when completing refs
  2017-03-23 15:29 [PATCHv2 00/14] completion: speed up refs completion SZEDER Gábor
                   ` (10 preceding siblings ...)
  2017-03-23 15:29 ` [PATCHv2 11/14] completion: let 'for-each-ref' sort " SZEDER Gábor
@ 2017-03-23 15:29 ` SZEDER Gábor
  2017-03-23 15:29 ` [PATCHv2 13/14] completion: fill COMPREPLY directly when completing fetch refspecs SZEDER Gábor
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: SZEDER Gábor @ 2017-03-23 15:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor

__gitcomp_nl() iterates over all the possible completion words it gets
as argument

  - filtering matching words,
  - appending a trailing space to each matching word (in all but two
    cases),
  - prepending a prefix to each matching word (when completing words
    after e.g. '--option=<TAB>' or 'master..<TAB>'), and
  - adding each matching word to the COMPREPLY array.

This takes a while when a lot of refs are passed to __gitcomp_nl().

The previous changes in this series ensure that __git_refs() lists
only refs matching the current word to be completed, making a second
filtering in __gitcomp_nl() redundant.

Adding the necessary prefix and suffix could be done in __git_refs()
as well:

  - When refs come from 'git for-each-ref', then that prefix and
    suffix could be added much more efficiently using a 'git
    for-each-ref' format containing said prefix and suffix.  Care
    should be taken, though, because that prefix might contain
    'for-each-ref' format specifiers as part of the left hand side of
    a '..' range or '...' symmetric difference notation or
    fetch/push/etc. refspec, e.g. 'git log "evil-%(refname)..br<TAB>'.
    Doubling every '%' in the prefix will prevent 'git for-each-ref'
    from interpolating any of those contained specifiers.
  - When refs come from 'git ls-remote', then that prefix and suffix
    can be added in the shell loop that has to process 'git
    ls-remote's output anyway.
  - Finally, the prefix and suffix can be added to that handful of
    potentially matching symbolic and pseudo refs right away in the
    shell loop listing them.

And then all what is still left to do is to assign a bunch of
newline-separated words to a shell array, which can be done without a
shell loop iterating over each word, basically making all of
__gitcomp_nl() unnecessary for refs completion.

Add the helper function __gitcomp_direct() to fill the COMPREPLY array
with prefiltered and preprocessed words without any additional
processing, without a shell loop, with just one single compound
assignment.  Modify __git_refs() to accept prefix and suffix
parameters and add them to each and every listed ref as described
above.  Modify __git_complete_refs() to pass the prefix and suffix
parameters to __git_refs() and to feed __git_refs()'s output to
__gitcomp_direct() instead of __gitcomp_nl().

This speeds up refs completion when there are a lot of refs matching
the current word to be completed.  Listing all branches for completion
in a repo with 100k local branches, all packed, best of five:

  On Linux, near the beginning of this series, for reference:

    $ time __git_complete_refs

    real    0m2.028s
    user    0m1.692s
    sys     0m0.344s

  Before this patch:

    real    0m1.135s
    user    0m1.112s
    sys     0m0.024s

  After:

    real    0m0.367s
    user    0m0.352s
    sys     0m0.020s

  On Windows, near the beginning:

    real    0m13.078s
    user    0m1.609s
    sys     0m0.060s

  Before this patch:

    real    0m2.093s
    user    0m1.641s
    sys     0m0.060s

  After:

    real    0m0.683s
    user    0m0.203s
    sys     0m0.076s

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 contrib/completion/git-completion.bash | 54 +++++++++++++++++++++++++---------
 contrib/completion/git-completion.zsh  |  9 ++++++
 t/t9902-completion.sh                  | 27 +++++++++++++++++
 3 files changed, 76 insertions(+), 14 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index d26312899..41e658931 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -213,6 +213,20 @@ _get_comp_words_by_ref ()
 }
 fi
 
+# Fills the COMPREPLY array with prefiltered words without any additional
+# processing.
+# Callers must take care of providing only words that match the current word
+# to be completed and adding any prefix and/or suffix (trailing space!), if
+# necessary.
+# 1: List of newline-separated matching completion words, complete with
+#    prefix and suffix.
+__gitcomp_direct ()
+{
+	local IFS=$'\n'
+
+	COMPREPLY=($1)
+}
+
 __gitcompappend ()
 {
 	local x i=${#COMPREPLY[@]}
@@ -354,18 +368,21 @@ __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).
-# 3: Currently ignored.
+# 3: A prefix to be added to each listed ref (optional).
 # 4: List only refs matching this word (optional; list all refs if unset or
 #    empty).
+# 5: A suffix to be appended to each listed ref (optional; ignored, if set
+#    but empty).
 #
 # Use __git_complete_refs() instead.
 __git_refs ()
 {
 	local i hash dir track="${2-}"
 	local list_refs_from=path remote="${1-}"
-	local format refs pfx
-	local cur_="${4-$cur}"
+	local format refs
+	local pfx="${3-}" cur_="${4-$cur}" sfx="${5-}"
 	local match="${4-}"
+	local fer_pfx="${pfx//\%/%%}" # "escape" for-each-ref format specifiers
 
 	__git_find_repo_path
 	dir="$__git_repo_path"
@@ -390,7 +407,8 @@ __git_refs ()
 
 	if [ "$list_refs_from" = path ]; then
 		if [[ "$cur_" == ^* ]]; then
-			pfx="^"
+			pfx="$pfx^"
+			fer_pfx="$fer_pfx^"
 			cur_=${cur_#^}
 			match=${match#^}
 		fi
@@ -405,7 +423,7 @@ __git_refs ()
 				case "$i" in
 				$match*)
 					if [ -e "$dir/$i" ]; then
-						echo $pfx$i
+						echo "$pfx$i$sfx"
 					fi
 					;;
 				esac
@@ -416,13 +434,13 @@ __git_refs ()
 				"refs/remotes/$match*" "refs/remotes/$match*/**")
 			;;
 		esac
-		__git_dir="$dir" __git for-each-ref --format="$pfx%($format)" \
+		__git_dir="$dir" __git for-each-ref --format="$fer_pfx%($format)$sfx" \
 			"${refs[@]}"
 		if [ -n "$track" ]; then
 			# employ the heuristic used by git checkout
 			# Try to find a remote branch that matches the completion word
 			# but only output if the branch name is unique
-			__git for-each-ref --format="%(refname:strip=3)" \
+			__git for-each-ref --format="$fer_pfx%(refname:strip=3)$sfx" \
 				--sort="refname:strip=3" \
 				"refs/remotes/*/$match*" "refs/remotes/*/$match*/**" | \
 			uniq -u
@@ -435,16 +453,16 @@ __git_refs ()
 		while read -r hash i; do
 			case "$i" in
 			*^{}) ;;
-			*) echo "$i" ;;
+			*) echo "$pfx$i$sfx" ;;
 			esac
 		done
 		;;
 	*)
 		if [ "$list_refs_from" = remote ]; then
 			case "HEAD" in
-			$match*)	echo "HEAD" ;;
+			$match*)	echo "${pfx}HEAD$sfx" ;;
 			esac
-			__git for-each-ref --format="%(refname:strip=3)" \
+			__git for-each-ref --format="$fer_pfx%(refname:strip=3)$sfx" \
 				"refs/remotes/$remote/$match*" \
 				"refs/remotes/$remote/$match*/**"
 		else
@@ -458,8 +476,8 @@ __git_refs ()
 			while read -r hash i; do
 				case "$i" in
 				*^{})	;;
-				refs/*)	echo "${i#refs/*/}" ;;
-				*)	echo "$i" ;;  # symbolic refs
+				refs/*)	echo "$pfx${i#refs/*/}$sfx" ;;
+				*)	echo "$pfx$i$sfx" ;;  # symbolic refs
 				esac
 			done
 		fi
@@ -494,8 +512,7 @@ __git_complete_refs ()
 		shift
 	done
 
-	__gitcomp_nl "$(__git_refs "$remote" "$track" "" "$cur_")" \
-		"$pfx" "$cur_" "$sfx"
+	__gitcomp_direct "$(__git_refs "$remote" "$track" "$pfx" "$cur_" "$sfx")"
 }
 
 # __git_refs2 requires 1 argument (to pass to __git_refs)
@@ -2997,6 +3014,15 @@ if [[ -n ${ZSH_VERSION-} ]]; then
 		esac
 	}
 
+	__gitcomp_direct ()
+	{
+		emulate -L zsh
+
+		local IFS=$'\n'
+		compset -P '*[=:]'
+		compadd -Q -- ${=1} && _ret=0
+	}
+
 	__gitcomp_nl ()
 	{
 		emulate -L zsh
diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
index e25541308..c3521fbfc 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -67,6 +67,15 @@ __gitcomp ()
 	esac
 }
 
+__gitcomp_direct ()
+{
+	emulate -L zsh
+
+	local IFS=$'\n'
+	compset -P '*[=:]'
+	compadd -Q -- ${=1} && _ret=0
+}
+
 __gitcomp_nl ()
 {
 	emulate -L zsh
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index cc9e741f9..5ed28135b 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -400,6 +400,22 @@ test_expect_success '__gitdir - remote as argument' '
 	test_cmp expected "$actual"
 '
 
+test_expect_success '__gitcomp_direct - puts everything into COMPREPLY as-is' '
+	sed -e "s/Z$//g" >expected <<-EOF &&
+	with-trailing-space Z
+	without-trailing-spaceZ
+	--option Z
+	--option=Z
+	$invalid_variable_name Z
+	EOF
+	(
+		cur=should_be_ignored &&
+		__gitcomp_direct "$(cat expected)" &&
+		print_comp
+	) &&
+	test_cmp expected out
+'
+
 test_expect_success '__gitcomp - trailing space - options' '
 	test_gitcomp "--re" "--dry-run --reuse-message= --reedit-message=
 		--reset-author" <<-EOF
@@ -961,6 +977,17 @@ test_expect_success 'teardown after filtering matching refs' '
 	git -C otherrepo branch -D matching/branch-in-other
 '
 
+test_expect_success '__git_refs - for-each-ref format specifiers in prefix' '
+	cat >expected <<-EOF &&
+	evil-%%-%42-%(refname)..master
+	EOF
+	(
+		cur="evil-%%-%42-%(refname)..mas" &&
+		__git_refs "" "" "evil-%%-%42-%(refname).." mas >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
 test_expect_success '__git_complete_refs - simple' '
 	sed -e "s/Z$//" >expected <<-EOF &&
 	HEAD Z
-- 
2.12.1.485.g1616aa492


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCHv2 13/14] completion: fill COMPREPLY directly when completing fetch refspecs
  2017-03-23 15:29 [PATCHv2 00/14] completion: speed up refs completion SZEDER Gábor
                   ` (11 preceding siblings ...)
  2017-03-23 15:29 ` [PATCHv2 12/14] completion: fill COMPREPLY directly when completing refs SZEDER Gábor
@ 2017-03-23 15:29 ` SZEDER Gábor
  2017-03-23 15:29 ` [PATCHv2 14/14] completion: speed up branch and tag completion SZEDER Gábor
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: SZEDER Gábor @ 2017-03-23 15:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor

The __git_complete_fetch_refspecs() has to iterate over __git_refs()'s
output anyway to turn the listed matching refs into refspecs, and it
knows about the prefix and suffix that has to be added to each
refspec.

Modify this function to add the prefix and suffix to each refspec
while iterating and feed the result, since it doesn't need further
processing, to the new __gitcomp_direct() helper added in the previous
commit, because it should be faster when there are a lot of refspecs
to list.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 contrib/completion/git-completion.bash | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 41e658931..86c7d93b8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -536,11 +536,11 @@ __git_complete_fetch_refspecs ()
 {
 	local i remote="$1" pfx="${2-}" cur_="${3-$cur}" sfx="${4- }"
 
-	__gitcomp_nl "$(
+	__gitcomp_direct "$(
 		for i in $(__git_refs "$remote" "" "" "$cur_") ; do
-			echo "$i:$i"
+			echo "$pfx$i:$i$sfx"
 		done
-		)" "$pfx" "$cur_" "$sfx"
+		)"
 }
 
 # __git_refs_remotes requires 1 argument (to pass to ls-remote)
-- 
2.12.1.485.g1616aa492


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCHv2 14/14] completion: speed up branch and tag completion
  2017-03-23 15:29 [PATCHv2 00/14] completion: speed up refs completion SZEDER Gábor
                   ` (12 preceding siblings ...)
  2017-03-23 15:29 ` [PATCHv2 13/14] completion: fill COMPREPLY directly when completing fetch refspecs SZEDER Gábor
@ 2017-03-23 15:29 ` SZEDER Gábor
  2017-03-23 15:33 ` [PATCHv2 00/14] completion: speed up refs completion SZEDER Gábor
  2017-03-23 18:28 ` Junio C Hamano
  15 siblings, 0 replies; 22+ messages in thread
From: SZEDER Gábor @ 2017-03-23 15:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor

Modify __git_heads() and __git_tags() and the few callsites they have,
so we can let 'git for-each-ref' do all the hard work and these
functions' output won't need any further processing or filtering
before being handed over to Bash, resulting in faster branch and tag
completion.  These are some of the same tricks used in the previous
commits to speed up refs completion, namely:

  - Extend both functions to accept prefix, current word and suffix
    positional parameters, all optional and all empty by default to
    keep the parameterless behavior unaltered.

  - Specify appropriate globbing patterns to 'git for-each-ref' to
    list only branches or tags matching the given current word
    parameter.

  - Modify the 'git for-each-ref --format=<...>' to include the given
    prefix and suffix.

  - Adjust all callsites to specify the proper prefix, current word
    and suffix parameters, and to fill COMPREPLY using
    __gitcomp_direct().

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 contrib/completion/git-completion.bash | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 86c7d93b8..bd07d9a74 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -352,14 +352,27 @@ __git_index_files ()
 	done | sort | uniq
 }
 
+# Lists branches from the local repository.
+# 1: A prefix to be added to each listed branch (optional).
+# 2: List only branches matching this word (optional; list all branches if
+#    unset or empty).
+# 3: A suffix to be appended to each listed branch (optional).
 __git_heads ()
 {
-	__git for-each-ref --format='%(refname:strip=2)' refs/heads
+	local pfx="${1-}" cur_="${2-}" sfx="${3-}"
+
+	__git for-each-ref --format="${pfx//\%/%%}%(refname:strip=2)$sfx" \
+			"refs/heads/$cur_*" "refs/heads/$cur_*/**"
 }
 
+# Lists tags from the local repository.
+# Accepts the same positional parameters as __git_heads() above.
 __git_tags ()
 {
-	__git for-each-ref --format='%(refname:strip=2)' refs/tags
+	local pfx="${1-}" cur_="${2-}" sfx="${3-}"
+
+	__git for-each-ref --format="${pfx//\%/%%}%(refname:strip=2)$sfx" \
+			"refs/tags/$cur_*" "refs/tags/$cur_*/**"
 }
 
 # Lists refs from the local (by default) or from a remote repository.
@@ -1183,7 +1196,7 @@ _git_branch ()
 		;;
 	*)
 		if [ $only_local_ref = "y" -a $has_r = "n" ]; then
-			__gitcomp_nl "$(__git_heads)"
+			__gitcomp_direct "$(__git_heads "" "$cur" " ")"
 		else
 			__git_complete_refs
 		fi
@@ -2178,7 +2191,7 @@ _git_config ()
 		;;
 	branch.*)
 		local pfx="${cur%.*}." cur_="${cur#*.}"
-		__gitcomp_nl "$(__git_heads)" "$pfx" "$cur_" "."
+		__gitcomp_direct "$(__git_heads "$pfx" "$cur_" ".")"
 		__gitcomp_nl_append $'autosetupmerge\nautosetuprebase\n' "$pfx" "$cur_"
 		return
 		;;
@@ -2824,7 +2837,7 @@ _git_tag ()
 		i="${words[c]}"
 		case "$i" in
 		-d|-v)
-			__gitcomp_nl "$(__git_tags)"
+			__gitcomp_direct "$(__git_tags "" "$cur" " ")"
 			return
 			;;
 		-f)
@@ -2839,7 +2852,7 @@ _git_tag ()
 		;;
 	-*|tag)
 		if [ $f = 1 ]; then
-			__gitcomp_nl "$(__git_tags)"
+			__gitcomp_direct "$(__git_tags "" "$cur" " ")"
 		fi
 		;;
 	*)
-- 
2.12.1.485.g1616aa492


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCHv2 00/14] completion: speed up refs completion
  2017-03-23 15:29 [PATCHv2 00/14] completion: speed up refs completion SZEDER Gábor
                   ` (13 preceding siblings ...)
  2017-03-23 15:29 ` [PATCHv2 14/14] completion: speed up branch and tag completion SZEDER Gábor
@ 2017-03-23 15:33 ` SZEDER Gábor
  2017-03-23 18:28 ` Junio C Hamano
  15 siblings, 0 replies; 22+ messages in thread
From: SZEDER Gábor @ 2017-03-23 15:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git mailing list, SZEDER Gábor

On Thu, Mar 23, 2017 at 4:29 PM, SZEDER Gábor <szeder.dev@gmail.com> wrote:
> This series is the updated version of 'sg/completion-refs-speedup'.

Forgot to mention that v1 can be found here:
http://public-inbox.org/git/20170203025405.8242-1-szeder.dev@gmail.com/T/

Gábor

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCHv2 00/14] completion: speed up refs completion
  2017-03-23 15:29 [PATCHv2 00/14] completion: speed up refs completion SZEDER Gábor
                   ` (14 preceding siblings ...)
  2017-03-23 15:33 ` [PATCHv2 00/14] completion: speed up refs completion SZEDER Gábor
@ 2017-03-23 18:28 ` Junio C Hamano
  2017-03-24 20:01   ` Jeff King
  15 siblings, 1 reply; 22+ messages in thread
From: Junio C Hamano @ 2017-03-23 18:28 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: git, Jacob Keller

SZEDER Gábor <szeder.dev@gmail.com> writes:

> This series is the updated version of 'sg/completion-refs-speedup'.
> It speeds up refs completion for large number of refs, partly by
> giving up disambiguating ambiguous refs and partly by eliminating most
> of the shell processing between 'git for-each-ref' and 'ls-remote' and
> Bash's completion facility.  The rest is a bit of preparatory
> reorganization, cleanup and bugfixes.
>
> Changes since v1:
> ...
> [1] - http://public-inbox.org/git/20170206181545.12869-1-szeder.dev@gmail.com/

It seems Jacob Keller was the only person who was excited about
these changes when v1 was posted?  It would be nice to see a bit
more enthusiasm from other folks who are invested in the completion
script, but you are the de-facto go-to person on the completion
already, so ... ;-)

Will replace.  Let's advance this to 'next' soonish (say, by early
next week).

Thanks.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCHv2 07/14] completion: don't disambiguate short refs
  2017-03-23 15:29 ` [PATCHv2 07/14] completion: don't disambiguate short refs SZEDER Gábor
@ 2017-03-24 19:31   ` Jeff King
  0 siblings, 0 replies; 22+ messages in thread
From: Jeff King @ 2017-03-24 19:31 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Junio C Hamano, git

On Thu, Mar 23, 2017 at 04:29:17PM +0100, SZEDER Gábor wrote:

> However, it's questionable whether ambiguous refs are really that bad
> to justify that much extra cost:

It's not clear to me that the existing completion actually does a good
job with disambiguation anyway.

If I have a tag and a branch named "foo", then in theory doing:

  git log fo<Tab>

should present me with "heads/foo" and "tags/foo" as options. But it
doesn't seem to; it just completes "foo".

But even if it did, those don't _start_ with foo, I have to go to some
work to back up anyway. I think we are better off just completing "foo"
and letting the command complain that it's ambiguous.

So even leaving aside the performance tradeoff, that seems like a more
sensible behavior anyway. And AFAICT, that's the behavior you'd get with
your patch (we'd get two "foo"s, but the completion is presumably smart
enough to handle that.

> This speeds up refs completion considerably.  Uniquely completing a
> branch in a repository with 100k local branches, all packed, best of
> five:
> 
>   On Linux, before:
> 
>     $ time __git_complete_refs --cur=maste
> 
>     real    0m1.662s
>     user    0m1.368s
>     sys     0m0.296s
> 
>   After:
> 
>     real    0m0.831s
>     user    0m0.808s
>     sys     0m0.028s

This is nice, though I think faster ref storage is another way to attack
the problem. This is much simpler, though. :)

-Peff

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCHv2 08/14] completion: let 'for-each-ref' and 'ls-remote' filter matching refs
  2017-03-23 15:29 ` [PATCHv2 08/14] completion: let 'for-each-ref' and 'ls-remote' filter matching refs SZEDER Gábor
@ 2017-03-24 19:42   ` Jeff King
  2017-03-28 15:34     ` SZEDER Gábor
  0 siblings, 1 reply; 22+ messages in thread
From: Jeff King @ 2017-03-24 19:42 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Junio C Hamano, git

On Thu, Mar 23, 2017 at 04:29:18PM +0100, SZEDER Gábor wrote:

> When completing refs, several __git_refs() code paths list all the
> refs from the refs/{heads,tags,remotes}/ hierarchy and then
> __gitcomp_nl() iterates over those refs in a shell loop to filter out
> refs not matching the current ref to be completed.  This comes with a
> considerable performance penalty when a repository contains a lot of
> refs but the current ref can be uniquely completed or when only a
> handful of refs match the current ref.

Part of this is due to outputting too many refs from for-each-ref, but
part of it is that for-each-ref is not good at limiting the refs it
looks at internally. I have a patch for that, but it's not all that
helpful without Michael Haggerty's mmap-packed-refs work, so I'll get it
included there.

So that makes this change doubly important. You get the immediate
speedups, and then you'll get another one when that work is merged
later.

>  		case "$cur_" in
>  		refs|refs/*)
>  			format="refname"
> -			refs="${cur_%/*}"
> +			refs=("$match*" "$match*/**")
>  			track=""

Working on the aforementioned patch, I noticed that for-each-ref's
matching is a little tricky due to its path semantics. So I wanted to
double-check your patterns. :) I think these should do the right thing.

-Peff

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCHv2 11/14] completion: let 'for-each-ref' sort remote branches for 'checkout' DWIMery
  2017-03-23 15:29 ` [PATCHv2 11/14] completion: let 'for-each-ref' sort " SZEDER Gábor
@ 2017-03-24 19:53   ` Jeff King
  0 siblings, 0 replies; 22+ messages in thread
From: Jeff King @ 2017-03-24 19:53 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Junio C Hamano, git

On Thu, Mar 23, 2017 at 04:29:21PM +0100, SZEDER Gábor wrote:

> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 394dcece6..d26312899 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -423,8 +423,9 @@ __git_refs ()
>  			# Try to find a remote branch that matches the completion word
>  			# but only output if the branch name is unique
>  			__git for-each-ref --format="%(refname:strip=3)" \
> +				--sort="refname:strip=3" \
>  				"refs/remotes/*/$match*" "refs/remotes/*/$match*/**" | \
> -			sort | uniq -u
> +			uniq -u

I wonder if it would be worth adding a "--unique" option to
for-each-ref. I guess the definition of "unique" may change though (here
you'd want to do it on the sort key, but other cases may want unique
branch names sorted by time, which is less trivial).

-Peff

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCHv2 00/14] completion: speed up refs completion
  2017-03-23 18:28 ` Junio C Hamano
@ 2017-03-24 20:01   ` Jeff King
  0 siblings, 0 replies; 22+ messages in thread
From: Jeff King @ 2017-03-24 20:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: SZEDER Gábor, git, Jacob Keller

On Thu, Mar 23, 2017 at 11:28:52AM -0700, Junio C Hamano wrote:

> SZEDER Gábor <szeder.dev@gmail.com> writes:
> 
> > This series is the updated version of 'sg/completion-refs-speedup'.
> > It speeds up refs completion for large number of refs, partly by
> > giving up disambiguating ambiguous refs and partly by eliminating most
> > of the shell processing between 'git for-each-ref' and 'ls-remote' and
> > Bash's completion facility.  The rest is a bit of preparatory
> > reorganization, cleanup and bugfixes.
> >
> > Changes since v1:
> > ...
> > [1] - http://public-inbox.org/git/20170206181545.12869-1-szeder.dev@gmail.com/
> 
> It seems Jacob Keller was the only person who was excited about
> these changes when v1 was posted?  It would be nice to see a bit
> more enthusiasm from other folks who are invested in the completion
> script, but you are the de-facto go-to person on the completion
> already, so ... ;-)
> 
> Will replace.  Let's advance this to 'next' soonish (say, by early
> next week).

I'm far from an expert in the completion scripts, but I read over the
whole thing and it looked good to me. As usual with bash, the
optimizations can make the code a bit non-intuitive, but I think the
changes are clearly explained and the performance results speak for
themselves.

-Peff

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCHv2 08/14] completion: let 'for-each-ref' and 'ls-remote' filter matching refs
  2017-03-24 19:42   ` Jeff King
@ 2017-03-28 15:34     ` SZEDER Gábor
  0 siblings, 0 replies; 22+ messages in thread
From: SZEDER Gábor @ 2017-03-28 15:34 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Git mailing list

On Fri, Mar 24, 2017 at 8:42 PM, Jeff King <peff@peff.net> wrote:
> On Thu, Mar 23, 2017 at 04:29:18PM +0100, SZEDER Gábor wrote:
>>               case "$cur_" in
>>               refs|refs/*)
>>                       format="refname"
>> -                     refs="${cur_%/*}"
>> +                     refs=("$match*" "$match*/**")
>>                       track=""
>
> Working on the aforementioned patch, I noticed that for-each-ref's
> matching is a little tricky due to its path semantics. So I wanted to
> double-check your patterns. :) I think these should do the right thing.

Yeah, I always thought that it's weird that globbing in for-each-ref
behaves differently from globbing in ls-remote or refspecs, but there
is nothing we can do about it now.

Anyway, this is why the tests added in this patch include e.g. both
'matching-branch' and 'matching/branch'.

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2017-03-28 15:35 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-23 15:29 [PATCHv2 00/14] completion: speed up refs completion SZEDER Gábor
2017-03-23 15:29 ` [PATCHv2 01/14] completion: remove redundant __gitcomp_nl() options from _git_commit() SZEDER Gábor
2017-03-23 15:29 ` [PATCHv2 02/14] completion: wrap __git_refs() for better option parsing SZEDER Gábor
2017-03-23 15:29 ` [PATCHv2 03/14] completion: support completing full refs after '--option=refs/<TAB>' SZEDER Gábor
2017-03-23 15:29 ` [PATCHv2 04/14] completion: support completing fully qualified non-fast-forward refspecs SZEDER Gábor
2017-03-23 15:29 ` [PATCHv2 05/14] completion: support excluding full refs SZEDER Gábor
2017-03-23 15:29 ` [PATCHv2 06/14] completion: don't disambiguate tags and branches SZEDER Gábor
2017-03-23 15:29 ` [PATCHv2 07/14] completion: don't disambiguate short refs SZEDER Gábor
2017-03-24 19:31   ` Jeff King
2017-03-23 15:29 ` [PATCHv2 08/14] completion: let 'for-each-ref' and 'ls-remote' filter matching refs SZEDER Gábor
2017-03-24 19:42   ` Jeff King
2017-03-28 15:34     ` SZEDER Gábor
2017-03-23 15:29 ` [PATCHv2 09/14] completion: let 'for-each-ref' strip the remote name from remote branches SZEDER Gábor
2017-03-23 15:29 ` [PATCHv2 10/14] completion: let 'for-each-ref' filter remote branches for 'checkout' DWIMery SZEDER Gábor
2017-03-23 15:29 ` [PATCHv2 11/14] completion: let 'for-each-ref' sort " SZEDER Gábor
2017-03-24 19:53   ` Jeff King
2017-03-23 15:29 ` [PATCHv2 12/14] completion: fill COMPREPLY directly when completing refs SZEDER Gábor
2017-03-23 15:29 ` [PATCHv2 13/14] completion: fill COMPREPLY directly when completing fetch refspecs SZEDER Gábor
2017-03-23 15:29 ` [PATCHv2 14/14] completion: speed up branch and tag completion SZEDER Gábor
2017-03-23 15:33 ` [PATCHv2 00/14] completion: speed up refs completion SZEDER Gábor
2017-03-23 18:28 ` Junio C Hamano
2017-03-24 20:01   ` Jeff King

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.