Git development
 help / color / mirror / Atom feed
* [PATCHv2 21/21] completion: cache the path to the repository
From: SZEDER Gábor @ 2017-02-03  2:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor
In-Reply-To: <20170203024829.8071-1-szeder.dev@gmail.com>

After the previous changes in this series there are only a handful of
$(__gitdir) command substitutions left in the completion script, but
there is still a bit of room for improvements:

  1. The command substitution involves the forking of a subshell,
     which has considerable overhead on some platforms.

  2. There are a few cases, where this command substitution is
     executed more than once during a single completion, which means
     multiple subshells and possibly multiple 'git rev-parse'
     executions.  __gitdir() is invoked twice while completing refs
     for e.g. 'git log', 'git rebase', 'gitk', or while completing
     remote refs for 'git fetch' or 'git push'.

Both of these points can be addressed by using the
__git_find_repo_path() helper function introduced in the previous
commit:

  1. __git_find_repo_path() stores the path to the repository in a
     variable instead of printing it, so the command substitution
     around the function can be avoided.  Or rather: the command
     substitution should be avoided to make the new value of the
     variable set inside the function visible to the callers.
     (Yes, there is now a command substitution inside
     __git_find_repo_path() around each 'git rev-parse', but that's
     executed only if necessary, and only once per completion, see
     point 2. below.)

  2. $__git_repo_path, the variable holding the path to the
     repository, is declared local in the toplevel completion
     functions __git_main() and __gitk_main().  Thus, once set, the
     path is visible in all completion functions, including all
     subsequent calls to __git_find_repo_path(), meaning that they
     wouldn't have to re-discover the path to the repository.

So call __git_find_repo_path() and use $__git_repo_path instead of the
$(__gitdir) command substitution to access paths in the .git
directory.  Turn tests checking __gitdir()'s repository discovery into
tests of __git_find_repo_path() such that only the tested function
changes but the expected results don't, ensuring that repo discovery
keeps working as it did before.

As __gitdir() is not used anymore in the completion script, mark it as
deprecated and direct users' attention to __git_find_repo_path() and
$__git_repo_path.  Yet keep four __gitdir() tests to ensure that it
handles success and failure of __git_find_repo_path() and that it
still handles its optional remote argument, because users' custom
completion scriptlets might depend on it.

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

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 7775411cd..ed06cb621 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -39,6 +39,11 @@ esac
 # variable.
 __git_find_repo_path ()
 {
+	if [ -n "$__git_repo_path" ]; then
+		# we already know where it is
+		return
+	fi
+
 	if [ -n "${__git_C_args-}" ]; then
 		__git_repo_path="$(git "${__git_C_args[@]}" \
 			${__git_dir:+--git-dir="$__git_dir"} \
@@ -56,6 +61,7 @@ __git_find_repo_path ()
 	fi
 }
 
+# Deprecated: use __git_find_repo_path() and $__git_repo_path instead
 # __gitdir accepts 0 or 1 arguments (i.e., location)
 # returns location of .git repo
 __gitdir ()
@@ -350,10 +356,13 @@ __git_tags ()
 #    'git checkout's tracking DWIMery (optional; ignored, if set but empty).
 __git_refs ()
 {
-	local i hash dir="$(__gitdir)" track="${2-}"
+	local i hash dir track="${2-}"
 	local list_refs_from=path remote="${1-}"
 	local format refs pfx
 
+	__git_find_repo_path
+	dir="$__git_repo_path"
+
 	if [ -z "$remote" ]; then
 		if [ -z "$dir" ]; then
 			return
@@ -458,8 +467,8 @@ __git_refs_remotes ()
 
 __git_remotes ()
 {
-	local d="$(__gitdir)"
-	test -d "$d/remotes" && ls -1 "$d/remotes"
+	__git_find_repo_path
+	test -d "$__git_repo_path/remotes" && ls -1 "$__git_repo_path/remotes"
 	__git remote
 }
 
@@ -957,8 +966,8 @@ __git_whitespacelist="nowarn warn error error-all fix"
 
 _git_am ()
 {
-	local dir="$(__gitdir)"
-	if [ -d "$dir"/rebase-apply ]; then
+	__git_find_repo_path
+	if [ -d "$__git_repo_path"/rebase-apply ]; then
 		__gitcomp "--skip --continue --resolved --abort"
 		return
 	fi
@@ -1041,7 +1050,8 @@ _git_bisect ()
 	local subcommands="start bad good skip reset visualize replay log run"
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
 	if [ -z "$subcommand" ]; then
-		if [ -f "$(__gitdir)"/BISECT_START ]; then
+		__git_find_repo_path
+		if [ -f "$__git_repo_path"/BISECT_START ]; then
 			__gitcomp "$subcommands"
 		else
 			__gitcomp "replay start"
@@ -1146,8 +1156,8 @@ _git_cherry ()
 
 _git_cherry_pick ()
 {
-	local dir="$(__gitdir)"
-	if [ -f "$dir"/CHERRY_PICK_HEAD ]; then
+	__git_find_repo_path
+	if [ -f "$__git_repo_path"/CHERRY_PICK_HEAD ]; then
 		__gitcomp "--continue --quit --abort"
 		return
 	fi
@@ -1538,10 +1548,10 @@ __git_log_date_formats="relative iso8601 rfc2822 short local default raw"
 _git_log ()
 {
 	__git_has_doubledash && return
+	__git_find_repo_path
 
-	local g="$(__gitdir)"
 	local merge=""
-	if [ -f "$g/MERGE_HEAD" ]; then
+	if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
 		merge="--merge"
 	fi
 	case "$cur" in
@@ -1788,11 +1798,12 @@ _git_push ()
 
 _git_rebase ()
 {
-	local dir="$(__gitdir)"
-	if [ -f "$dir"/rebase-merge/interactive ]; then
+	__git_find_repo_path
+	if [ -f "$__git_repo_path"/rebase-merge/interactive ]; then
 		__gitcomp "--continue --skip --abort --quit --edit-todo"
 		return
-	elif [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
+	elif [ -d "$__git_repo_path"/rebase-apply ] || \
+	     [ -d "$__git_repo_path"/rebase-merge ]; then
 		__gitcomp "--continue --skip --abort --quit"
 		return
 	fi
@@ -2467,8 +2478,8 @@ _git_reset ()
 
 _git_revert ()
 {
-	local dir="$(__gitdir)"
-	if [ -f "$dir"/REVERT_HEAD ]; then
+	__git_find_repo_path
+	if [ -f "$__git_repo_path"/REVERT_HEAD ]; then
 		__gitcomp "--continue --quit --abort"
 		return
 	fi
@@ -2865,9 +2876,10 @@ __gitk_main ()
 	__git_has_doubledash && return
 
 	local __git_repo_path
-	local g="$(__gitdir)"
+	__git_find_repo_path
+
 	local merge=""
-	if [ -f "$g/MERGE_HEAD" ]; then
+	if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
 		merge="--merge"
 	fi
 	case "$cur" in
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 1816ed2e0..d711bef24 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -131,213 +131,217 @@ else
 	ROOT="$(pwd)"
 fi
 
-test_expect_success 'setup for __gitdir tests' '
+test_expect_success 'setup for __git_find_repo_path/__gitdir tests' '
 	mkdir -p subdir/subsubdir &&
+	mkdir -p non-repo &&
 	git init otherrepo
 '
 
-test_expect_success '__gitdir - from command line (through $__git_dir)' '
+test_expect_success '__git_find_repo_path - from command line (through $__git_dir)' '
 	echo "$ROOT/otherrepo/.git" >expected &&
 	(
 		__git_dir="$ROOT/otherrepo/.git" &&
-		__gitdir >"$actual"
-	) &&
-	test_cmp expected "$actual"
-'
-
-test_expect_success '__gitdir - repo as argument' '
-	echo "otherrepo/.git" >expected &&
-	(
-		__gitdir "otherrepo" >"$actual"
+		__git_find_repo_path &&
+		echo "$__git_repo_path" >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
-test_expect_success '__gitdir - remote as argument' '
-	echo "remote" >expected &&
-	(
-		__gitdir "remote" >"$actual"
-	) &&
-	test_cmp expected "$actual"
-'
-
-test_expect_success '__gitdir - .git directory in cwd' '
+test_expect_success '__git_find_repo_path - .git directory in cwd' '
 	echo ".git" >expected &&
 	(
-		__gitdir >"$actual"
+		__git_find_repo_path &&
+		echo "$__git_repo_path" >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
-test_expect_success '__gitdir - .git directory in parent' '
+test_expect_success '__git_find_repo_path - .git directory in parent' '
 	echo "$ROOT/.git" >expected &&
 	(
 		cd subdir/subsubdir &&
-		__gitdir >"$actual"
+		__git_find_repo_path &&
+		echo "$__git_repo_path" >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
-test_expect_success '__gitdir - cwd is a .git directory' '
+test_expect_success '__git_find_repo_path - cwd is a .git directory' '
 	echo "." >expected &&
 	(
 		cd .git &&
-		__gitdir >"$actual"
+		__git_find_repo_path &&
+		echo "$__git_repo_path" >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
-test_expect_success '__gitdir - parent is a .git directory' '
+test_expect_success '__git_find_repo_path - parent is a .git directory' '
 	echo "$ROOT/.git" >expected &&
 	(
 		cd .git/refs/heads &&
-		__gitdir >"$actual"
+		__git_find_repo_path &&
+		echo "$__git_repo_path" >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
-test_expect_success '__gitdir - $GIT_DIR set while .git directory in cwd' '
+test_expect_success '__git_find_repo_path - $GIT_DIR set while .git directory in cwd' '
 	echo "$ROOT/otherrepo/.git" >expected &&
 	(
 		GIT_DIR="$ROOT/otherrepo/.git" &&
 		export GIT_DIR &&
-		__gitdir >"$actual"
+		__git_find_repo_path &&
+		echo "$__git_repo_path" >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
-test_expect_success '__gitdir - $GIT_DIR set while .git directory in parent' '
+test_expect_success '__git_find_repo_path - $GIT_DIR set while .git directory in parent' '
 	echo "$ROOT/otherrepo/.git" >expected &&
 	(
 		GIT_DIR="$ROOT/otherrepo/.git" &&
 		export GIT_DIR &&
 		cd subdir &&
-		__gitdir >"$actual"
+		__git_find_repo_path &&
+		echo "$__git_repo_path" >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
-test_expect_success '__gitdir - from command line while "git -C"' '
+test_expect_success '__git_find_repo_path - from command line while "git -C"' '
 	echo "$ROOT/.git" >expected &&
 	(
 		__git_dir="$ROOT/.git" &&
 		__git_C_args=(-C otherrepo) &&
-		__gitdir >"$actual"
+		__git_find_repo_path &&
+		echo "$__git_repo_path" >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
-test_expect_success '__gitdir - relative dir from command line and "git -C"' '
+test_expect_success '__git_find_repo_path - relative dir from command line and "git -C"' '
 	echo "$ROOT/otherrepo/.git" >expected &&
 	(
 		cd subdir &&
 		__git_dir="otherrepo/.git" &&
 		__git_C_args=(-C ..) &&
-		__gitdir >"$actual"
+		__git_find_repo_path &&
+		echo "$__git_repo_path" >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
-test_expect_success '__gitdir - $GIT_DIR set while "git -C"' '
+test_expect_success '__git_find_repo_path - $GIT_DIR set while "git -C"' '
 	echo "$ROOT/.git" >expected &&
 	(
 		GIT_DIR="$ROOT/.git" &&
 		export GIT_DIR &&
 		__git_C_args=(-C otherrepo) &&
-		__gitdir >"$actual"
+		__git_find_repo_path &&
+		echo "$__git_repo_path" >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
-test_expect_success '__gitdir - relative dir in $GIT_DIR and "git -C"' '
+test_expect_success '__git_find_repo_path - relative dir in $GIT_DIR and "git -C"' '
 	echo "$ROOT/otherrepo/.git" >expected &&
 	(
 		cd subdir &&
 		GIT_DIR="otherrepo/.git" &&
 		export GIT_DIR &&
 		__git_C_args=(-C ..) &&
-		__gitdir >"$actual"
+		__git_find_repo_path &&
+		echo "$__git_repo_path" >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
-test_expect_success '__gitdir - "git -C" while .git directory in cwd' '
+test_expect_success '__git_find_repo_path - "git -C" while .git directory in cwd' '
 	echo "$ROOT/otherrepo/.git" >expected &&
 	(
 		__git_C_args=(-C otherrepo) &&
-		__gitdir >"$actual"
+		__git_find_repo_path &&
+		echo "$__git_repo_path" >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
-test_expect_success '__gitdir - "git -C" while cwd is a .git directory' '
+test_expect_success '__git_find_repo_path - "git -C" while cwd is a .git directory' '
 	echo "$ROOT/otherrepo/.git" >expected &&
 	(
 		cd .git &&
 		__git_C_args=(-C .. -C otherrepo) &&
-		__gitdir >"$actual"
+		__git_find_repo_path &&
+		echo "$__git_repo_path" >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
-test_expect_success '__gitdir - "git -C" while .git directory in parent' '
+test_expect_success '__git_find_repo_path - "git -C" while .git directory in parent' '
 	echo "$ROOT/otherrepo/.git" >expected &&
 	(
 		cd subdir &&
 		__git_C_args=(-C .. -C otherrepo) &&
-		__gitdir >"$actual"
+		__git_find_repo_path &&
+		echo "$__git_repo_path" >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
-test_expect_success '__gitdir - non-existing path in "git -C"' '
+test_expect_success '__git_find_repo_path - non-existing path in "git -C"' '
 	(
 		__git_C_args=(-C non-existing) &&
-		test_must_fail __gitdir >"$actual"
+		test_must_fail __git_find_repo_path &&
+		printf "$__git_repo_path" >"$actual"
 	) &&
 	test_must_be_empty "$actual"
 '
 
-test_expect_success '__gitdir - non-existing path in $__git_dir' '
+test_expect_success '__git_find_repo_path - non-existing path in $__git_dir' '
 	(
 		__git_dir="non-existing" &&
-		test_must_fail __gitdir >"$actual"
+		test_must_fail __git_find_repo_path &&
+		printf "$__git_repo_path" >"$actual"
 	) &&
 	test_must_be_empty "$actual"
 '
 
-test_expect_success '__gitdir - non-existing $GIT_DIR' '
+test_expect_success '__git_find_repo_path - non-existing $GIT_DIR' '
 	(
 		GIT_DIR="$ROOT/non-existing" &&
 		export GIT_DIR &&
-		test_must_fail __gitdir >"$actual"
+		test_must_fail __git_find_repo_path &&
+		printf "$__git_repo_path" >"$actual"
 	) &&
 	test_must_be_empty "$actual"
 '
 
-test_expect_success '__gitdir - gitfile in cwd' '
+test_expect_success '__git_find_repo_path - gitfile in cwd' '
 	echo "$ROOT/otherrepo/.git" >expected &&
 	echo "gitdir: $ROOT/otherrepo/.git" >subdir/.git &&
 	test_when_finished "rm -f subdir/.git" &&
 	(
 		cd subdir &&
-		__gitdir >"$actual"
+		__git_find_repo_path &&
+		echo "$__git_repo_path" >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
-test_expect_success '__gitdir - gitfile in parent' '
+test_expect_success '__git_find_repo_path - gitfile in parent' '
 	echo "$ROOT/otherrepo/.git" >expected &&
 	echo "gitdir: $ROOT/otherrepo/.git" >subdir/.git &&
 	test_when_finished "rm -f subdir/.git" &&
 	(
 		cd subdir/subsubdir &&
-		__gitdir >"$actual"
+		__git_find_repo_path &&
+		echo "$__git_repo_path" >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
-test_expect_success SYMLINKS '__gitdir - resulting path avoids symlinks' '
+test_expect_success SYMLINKS '__git_find_repo_path - resulting path avoids symlinks' '
 	echo "$ROOT/otherrepo/.git" >expected &&
 	mkdir otherrepo/dir &&
 	test_when_finished "rm -rf otherrepo/dir" &&
@@ -345,16 +349,57 @@ test_expect_success SYMLINKS '__gitdir - resulting path avoids symlinks' '
 	test_when_finished "rm -f link" &&
 	(
 		cd link &&
+		__git_find_repo_path &&
+		echo "$__git_repo_path" >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__git_find_repo_path - not a git repository' '
+	(
+		cd non-repo &&
+		GIT_CEILING_DIRECTORIES="$ROOT" &&
+		export GIT_CEILING_DIRECTORIES &&
+		test_must_fail __git_find_repo_path &&
+		printf "$__git_repo_path" >"$actual"
+	) &&
+	test_must_be_empty "$actual"
+'
+
+test_expect_success '__gitdir - finds repo' '
+	echo "$ROOT/.git" >expected &&
+	(
+		cd subdir/subsubdir &&
 		__gitdir >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
-test_expect_success '__gitdir - not a git repository' '
-	nongit test_must_fail __gitdir >"$actual" &&
+
+test_expect_success '__gitdir - returns error when cant find repo' '
+	(
+		__git_dir="non-existing" &&
+		test_must_fail __gitdir >"$actual"
+	) &&
 	test_must_be_empty "$actual"
 '
 
+test_expect_success '__gitdir - repo as argument' '
+	echo "otherrepo/.git" >expected &&
+	(
+		__gitdir "otherrepo" >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__gitdir - remote as argument' '
+	echo "remote" >expected &&
+	(
+		__gitdir "remote" >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
 test_expect_success '__gitcomp - trailing space - options' '
 	test_gitcomp "--re" "--dry-run --reuse-message= --reedit-message=
 		--reset-author" <<-EOF
-- 
2.11.0.555.g967c1bcb3


^ permalink raw reply related

* [PATCHv2 09/21] completion: respect 'git --git-dir=<path>' when listing remote refs
From: SZEDER Gábor @ 2017-02-03  2:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor
In-Reply-To: <20170203024829.8071-1-szeder.dev@gmail.com>

In __git_refs() the git commands listing refs, both short and full,
from a given remote repository are run without giving them the path to
the git repository which might have been specified on the command line
via 'git --git-dir=<path>'.  This is bad, those git commands should
access the 'refs/remotes/<remote>/' hierarchy or the remote and
credentials configuration in that specified repository.

Use the __gitdir() helper only to find the path to the .git directory
and pass the resulting path to the 'git ls-remote' and 'for-each-ref'
executions that list remote refs.  While modifying that 'for-each-ref'
line, remove the superfluous disambiguating doubledash.

Don't use __gitdir() to check that the given remote is on the file
system: basically it performs only a single if statement for us at the
considerable cost of fork()ing a subshell for a command substitution.
We are better off to perform all the necessary checks of the remote in
__git_refs().

Though __git_refs() was the last remaining callsite that passed a
remote to __gitdir(), don't delete __gitdir()'s remote-handling part
yet, just in case some users' custom completion scriptlets depend on
it.

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

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 295f6de24..7d7e8b9d9 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -342,9 +342,21 @@ __git_tags ()
 #    'git checkout's tracking DWIMery (optional; ignored, if set but empty).
 __git_refs ()
 {
-	local i hash dir="$(__gitdir "${1-}")" track="${2-}"
+	local i hash dir="$(__gitdir)" track="${2-}"
+	local list_refs_from=path remote="${1-}"
 	local format refs pfx
-	if [ -d "$dir" ]; then
+
+	if [ -n "$remote" ]; then
+		if [ -d "$remote/.git" ]; then
+			dir="$remote/.git"
+		elif [ -d "$remote" ]; then
+			dir="$remote"
+		else
+			list_refs_from=remote
+		fi
+	fi
+
+	if [ "$list_refs_from" = path ] && [ -d "$dir" ]; then
 		case "$cur" in
 		refs|refs/*)
 			format="refname"
@@ -381,7 +393,7 @@ __git_refs ()
 	fi
 	case "$cur" in
 	refs|refs/*)
-		git ls-remote "$dir" "$cur*" 2>/dev/null | \
+		git --git-dir="$dir" ls-remote "$remote" "$cur*" 2>/dev/null | \
 		while read -r hash i; do
 			case "$i" in
 			*^{}) ;;
@@ -391,8 +403,8 @@ __git_refs ()
 		;;
 	*)
 		echo "HEAD"
-		git for-each-ref --format="%(refname:short)" -- \
-			"refs/remotes/$dir/" 2>/dev/null | sed -e "s#^$dir/##"
+		git --git-dir="$dir" for-each-ref --format="%(refname:short)" \
+			"refs/remotes/$remote/" 2>/dev/null | sed -e "s#^$remote/##"
 		;;
 	esac
 }
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 7667baabf..6e64cd6ba 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -486,7 +486,7 @@ test_expect_success '__git_refs - configured remote - full refs' '
 	test_cmp expected "$actual"
 '
 
-test_expect_failure '__git_refs - configured remote - repo given on the command line' '
+test_expect_success '__git_refs - configured remote - repo given on the command line' '
 	cat >expected <<-EOF &&
 	HEAD
 	branch-in-other
@@ -501,7 +501,7 @@ test_expect_failure '__git_refs - configured remote - repo given on the command
 	test_cmp expected "$actual"
 '
 
-test_expect_failure '__git_refs - configured remote - full refs - repo given on the command line' '
+test_expect_success '__git_refs - configured remote - full refs - repo given on the command line' '
 	cat >expected <<-EOF &&
 	refs/heads/branch-in-other
 	refs/heads/master-in-other
-- 
2.11.0.555.g967c1bcb3


^ permalink raw reply related

* [PATCHv2 14/21] completion: fix completion after 'git -C <path>'
From: SZEDER Gábor @ 2017-02-03  2:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor
In-Reply-To: <20170203024829.8071-1-szeder.dev@gmail.com>

The main completion function finds the name of the git command by
iterating through all the words on the command line in search for the
first non-option-looking word.  As it is not aware of 'git -C's
mandatory path argument, if the '-C <path>' option is present, 'path'
will be the first such word and it will be mistaken for a git command.
This breaks completion in various ways:

 - If 'path' happens to match one of the commands supported by the
   completion script, then options of that command will be offered.

 - If 'path' doesn't match a supported command and doesn't contain any
   characters not allowed in Bash identifier names, then the
   completion script does basically nothing and Bash in turn falls
   back to filename completion for all subsequent words.

 - Otherwise, if 'path' does contain such an unallowed character, then
   it leads to a more or less ugly error message in the middle of the
   command line.  The standard '/' directory separator is such a
   character, and it happens to trigger one of the uglier errors:

     $ git -C some/path <TAB>sh.exe": declare: `_git_some/path': not a valid identifier
     error: invalid key: alias.some/path

Fix this by skipping 'git -C's mandatory path argument while iterating
over the words on the command line.  Extend the relevant test with
this case and, while at it, with cases that needed similar treatment
in the past ('--git-dir', '-c', '--work-tree' and '--namespace').

Additionally, silence the standard error of the 'declare' builtins
looking for the completion function associated with the git command
and of the 'git config' query for the aliased command.  So if git ever
learns a new option with a mandatory argument in the future, then,
though the completion script will again misbehave, at least the
command line will not be utterly disrupted by those error messages.

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

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 7d25b33b8..ac5eb9ced 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -816,7 +816,7 @@ __git_aliases ()
 __git_aliased_command ()
 {
 	local word cmdline=$(git --git-dir="$(__gitdir)" \
-		config --get "alias.$1")
+		config --get "alias.$1" 2>/dev/null)
 	for word in $cmdline; do
 		case "$word" in
 		\!gitk|gitk)
@@ -2800,7 +2800,7 @@ __git_main ()
 		--git-dir)   ((c++)) ; __git_dir="${words[c]}" ;;
 		--bare)      __git_dir="." ;;
 		--help) command="help"; break ;;
-		-c|--work-tree|--namespace) ((c++)) ;;
+		-c|-C|--work-tree|--namespace) ((c++)) ;;
 		-*) ;;
 		*) command="$i"; break ;;
 		esac
@@ -2844,13 +2844,13 @@ __git_main ()
 	fi
 
 	local completion_func="_git_${command//-/_}"
-	declare -f $completion_func >/dev/null && $completion_func && return
+	declare -f $completion_func >/dev/null 2>/dev/null && $completion_func && return
 
 	local expansion=$(__git_aliased_command "$command")
 	if [ -n "$expansion" ]; then
 		words[1]=$expansion
 		completion_func="_git_${expansion//-/_}"
-		declare -f $completion_func >/dev/null && $completion_func
+		declare -f $completion_func >/dev/null 2>/dev/null && $completion_func
 	fi
 }
 
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 500505dca..be2ed8704 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -755,7 +755,12 @@ test_expect_success 'general options plus command' '
 	test_completion "git --namespace=foo check" "checkout " &&
 	test_completion "git --paginate check" "checkout " &&
 	test_completion "git --info-path check" "checkout " &&
-	test_completion "git --no-replace-objects check" "checkout "
+	test_completion "git --no-replace-objects check" "checkout " &&
+	test_completion "git --git-dir some/path check" "checkout " &&
+	test_completion "git -c conf.var=value check" "checkout " &&
+	test_completion "git -C some/path check" "checkout " &&
+	test_completion "git --work-tree some/path check" "checkout " &&
+	test_completion "git --namespace name/space check" "checkout "
 '
 
 test_expect_success 'git --help completion' '
-- 
2.11.0.555.g967c1bcb3


^ permalink raw reply related

* [PATCHv2 06/21] completion tests: add tests for the __git_refs() helper function
From: SZEDER Gábor @ 2017-02-03  2:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor
In-Reply-To: <20170203024829.8071-1-szeder.dev@gmail.com>

Check how __git_refs() lists refs in different scenarios, i.e.

  - short and full refs,
  - from a local or from a remote repository,
  - remote specified via path, name or URL,
  - with or without a repository specified on the command line,
  - non-existing remote,
  - unique remote branches for 'git checkout's tracking DWIMery,
  - not in a git repository, and
  - interesting combinations of the above.

Seven of these tests expect failure, mostly demonstrating bugs related
to listing refs from a remote repository:

  - ignoring the repository specified on the command line (2 tests),
  - listing refs from the wrong place when the name of a configured
    remote happens to match a directory,
  - listing only 'HEAD' but no short refs from a remote given as URL,
  - listing 'HEAD' even from non-existing remotes (2 tests), and
  - listing 'HEAD' when not in a repository.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 t/t9902-completion.sh | 265 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 264 insertions(+), 1 deletion(-)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index f7f7d49fb..7956cb9b1 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -365,6 +365,269 @@ test_expect_success '__git_remotes - list remotes from $GIT_DIR/remotes and from
 	test_cmp expect actual
 '
 
+test_expect_success 'setup for ref completion' '
+	git commit --allow-empty -m initial &&
+	git branch matching-branch &&
+	git tag matching-tag &&
+	(
+		cd otherrepo &&
+		git commit --allow-empty -m initial &&
+		git branch -m master master-in-other &&
+		git branch branch-in-other &&
+		git tag tag-in-other
+	) &&
+	git remote add other "$ROOT/otherrepo/.git" &&
+	git fetch --no-tags other &&
+	rm -f .git/FETCH_HEAD &&
+	git init thirdrepo
+'
+
+test_expect_success '__git_refs - simple' '
+	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 - full refs' '
+	cat >expected <<-EOF &&
+	refs/heads/master
+	refs/heads/matching-branch
+	EOF
+	(
+		cur=refs/heads/ &&
+		__git_refs >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - repo given on the command line' '
+	cat >expected <<-EOF &&
+	HEAD
+	branch-in-other
+	master-in-other
+	tag-in-other
+	EOF
+	(
+		__git_dir="$ROOT/otherrepo/.git" &&
+		cur= &&
+		__git_refs >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - remote on local file system' '
+	cat >expected <<-EOF &&
+	HEAD
+	branch-in-other
+	master-in-other
+	tag-in-other
+	EOF
+	(
+		cur= &&
+		__git_refs otherrepo >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - remote on local file system - full refs' '
+	cat >expected <<-EOF &&
+	refs/heads/branch-in-other
+	refs/heads/master-in-other
+	refs/tags/tag-in-other
+	EOF
+	(
+		cur=refs/ &&
+		__git_refs otherrepo >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - configured remote' '
+	cat >expected <<-EOF &&
+	HEAD
+	branch-in-other
+	master-in-other
+	EOF
+	(
+		cur= &&
+		__git_refs other >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - configured remote - full refs' '
+	cat >expected <<-EOF &&
+	refs/heads/branch-in-other
+	refs/heads/master-in-other
+	refs/tags/tag-in-other
+	EOF
+	(
+		cur=refs/ &&
+		__git_refs other >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_failure '__git_refs - configured remote - repo given on the command line' '
+	cat >expected <<-EOF &&
+	HEAD
+	branch-in-other
+	master-in-other
+	EOF
+	(
+		cd thirdrepo &&
+		__git_dir="$ROOT/.git" &&
+		cur= &&
+		__git_refs other >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_failure '__git_refs - configured remote - full refs - repo given on the command line' '
+	cat >expected <<-EOF &&
+	refs/heads/branch-in-other
+	refs/heads/master-in-other
+	refs/tags/tag-in-other
+	EOF
+	(
+		cd thirdrepo &&
+		__git_dir="$ROOT/.git" &&
+		cur=refs/ &&
+		__git_refs other >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_failure '__git_refs - configured remote - remote name matches a directory' '
+	cat >expected <<-EOF &&
+	HEAD
+	branch-in-other
+	master-in-other
+	EOF
+	mkdir other &&
+	test_when_finished "rm -rf other" &&
+	(
+		cur= &&
+		__git_refs other >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_failure '__git_refs - URL remote' '
+	cat >expected <<-EOF &&
+	HEAD
+	branch-in-other
+	master-in-other
+	tag-in-other
+	EOF
+	(
+		cur= &&
+		__git_refs "file://$ROOT/otherrepo/.git" >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - URL remote - full refs' '
+	cat >expected <<-EOF &&
+	refs/heads/branch-in-other
+	refs/heads/master-in-other
+	refs/tags/tag-in-other
+	EOF
+	(
+		cur=refs/ &&
+		__git_refs "file://$ROOT/otherrepo/.git" >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_failure '__git_refs - non-existing remote' '
+	(
+		cur= &&
+		__git_refs non-existing >"$actual"
+	) &&
+	test_must_be_empty "$actual"
+'
+
+test_expect_success '__git_refs - non-existing remote - full refs' '
+	(
+		cur=refs/ &&
+		__git_refs non-existing >"$actual"
+	) &&
+	test_must_be_empty "$actual"
+'
+
+test_expect_failure '__git_refs - non-existing URL remote' '
+	(
+		cur= &&
+		__git_refs "file://$ROOT/non-existing" >"$actual"
+	) &&
+	test_must_be_empty "$actual"
+'
+
+test_expect_success '__git_refs - non-existing URL remote - full refs' '
+	(
+		cur=refs/ &&
+		__git_refs "file://$ROOT/non-existing" >"$actual"
+	) &&
+	test_must_be_empty "$actual"
+'
+
+test_expect_failure '__git_refs - not in a git repository' '
+	(
+		GIT_CEILING_DIRECTORIES="$ROOT" &&
+		export GIT_CEILING_DIRECTORIES &&
+		cd subdir &&
+		cur= &&
+		__git_refs >"$actual"
+	) &&
+	test_must_be_empty "$actual"
+'
+
+test_expect_success '__git_refs - unique remote branches for git checkout DWIMery' '
+	cat >expected <<-EOF &&
+	HEAD
+	master
+	matching-branch
+	other/ambiguous
+	other/branch-in-other
+	other/master-in-other
+	remote/ambiguous
+	remote/branch-in-remote
+	matching-tag
+	branch-in-other
+	branch-in-remote
+	master-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= &&
+		__git_refs "" 1 >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'teardown after ref completion' '
+	git branch -d matching-branch &&
+	git tag -d matching-tag &&
+	git remote remove other
+'
+
 test_expect_success '__git_get_config_variables' '
 	cat >expect <<-EOF &&
 	name-1
@@ -483,7 +746,7 @@ test_expect_success 'git --help completion' '
 	test_completion "git --help core" "core-tutorial "
 '
 
-test_expect_success 'setup for ref completion' '
+test_expect_success 'setup for integration tests' '
 	echo content >file1 &&
 	echo more >file2 &&
 	git add file1 file2 &&
-- 
2.11.0.555.g967c1bcb3


^ permalink raw reply related

* [PATCHv2 01/21] completion: improve __git_refs()'s in-code documentation
From: SZEDER Gábor @ 2017-02-03  2:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor
In-Reply-To: <20170203024829.8071-1-szeder.dev@gmail.com>

That "first argument is passed to __gitdir()" statement in particular
is not really helpful, and after this series it won't be the case
anyway.

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

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6721ff80f..ee6fb2259 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -332,9 +332,11 @@ __git_tags ()
 	fi
 }
 
-# __git_refs accepts 0, 1 (to pass to __gitdir), or 2 arguments
-# presence of 2nd argument means use the guess heuristic employed
-# by checkout for tracking branches
+# Lists refs from the local (by default) or from a remote repository.
+# It accepts 0, 1 or 2 arguments:
+# 1: The remote to list refs from (optional; ignored, if set but empty).
+# 2: In addition to local refs, list unique branches from refs/remotes/ for
+#    'git checkout's tracking DWIMery (optional; ignored, if set but empty).
 __git_refs ()
 {
 	local i hash dir="$(__gitdir "${1-}")" track="${2-}"
-- 
2.11.0.555.g967c1bcb3


^ permalink raw reply related

* [PATCH 00/12] completion: speed up refs completion
From: SZEDER Gábor @ 2017-02-03  2:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor

This series speeds up refs completion for large number of refs, partly
by giving up disambiguating ambiguous refs (patch 6) 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.

The last patch touches the ZSH wrapper, too.  By a lucky educated
guess I managed to get it work on the first try, but I don't really
know what I've actually done, so...  ZSH users, please have a closer
look.

At the end of this series refs completion from a local repository is
as fast as it can possibly get, at least as far as the completion
script is concerned, because it basically does nothing anymore :)  All
it does is run 'git for-each-ref' with assorted options to do all the
work, and feed its output directly, without any processing into Bash's
COMPREPLY array.  There is still room for improvements in the code
paths using 'git ls-remote', but for that we would need enhancements
to 'ls-remote'.

It goes on top of the __gitdir() improvements series I just posted at:

  http://public-inbox.org/git/20170203024829.8071-1-szeder.dev@gmail.com/T/

This series is also available at:

  https://github.com/szeder/git completion-refs-speedup


SZEDER Gábor (12):
  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 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: list only matching symbolic and pseudorefs when completing
    refs
  completion: fill COMPREPLY directly when completing refs

 contrib/completion/git-completion.bash | 205 ++++++++++++++++--------
 contrib/completion/git-completion.zsh  |   9 ++
 t/t9902-completion.sh                  | 282 +++++++++++++++++++++++++++++++++
 3 files changed, 430 insertions(+), 66 deletions(-)

-- 
2.11.0.555.g967c1bcb3


^ permalink raw reply

* [PATCH 01/12] completion: remove redundant __gitcomp_nl() options from _git_commit()
From: SZEDER Gábor @ 2017-02-03  2:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor
In-Reply-To: <20170203025405.8242-1-szeder.dev@gmail.com>

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 ed06cb621..f20d4600c 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.11.0.555.g967c1bcb3


^ permalink raw reply related

* [PATCH 02/12] completion: wrap __git_refs() for better option parsing
From: SZEDER Gábor @ 2017-02-03  2:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor
In-Reply-To: <20170203025405.8242-1-szeder.dev@gmail.com>

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

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

  - we can't change __git_refs()'s default output format, i.e. we
    can't by default append a trailing space to every listed ref,
    meaning that the suffix parameter containing the default trailing
    space would have to be specified on every invocation, and

  - we can't change the position of existing positional parameters
    either, so there would have to be plenty of set-but-empty
    placeholder positional parameters all over the completion script.

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

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

  - instead of positional parameters accepts real '--opt=val'-style
    options and with minimalistic option parsing translates them to
    __git_refs()'s and __gitcomp_nl()'s positional parameters, and

  - includes the '__gitcomp_nl "$(__git_refs ...)" ...' command
    substitution to make its behavior match its name and the behavior
    of other __git_complete_* functions, and to limit future changes
    in this series to __git_refs() and this new wrapper function.

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

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

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


^ permalink raw reply related

* [PATCH 07/12] completion: let 'for-each-ref' and 'ls-remote' filter matching refs
From: SZEDER Gábor @ 2017-02-03  2:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor
In-Reply-To: <20170203025405.8242-1-szeder.dev@gmail.com>

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 word to be completed.  This comes with a
considerable performance penalty when a repository contains a lot of
refs but the current word can be uniquely completed or when only a
handful of refs match the current word.

Specify appropriate globbing patterns to 'git for-each-ref' and 'git
ls-remote' to list only those refs that match the current word to be
completed.  This reduces the number of iterations in __gitcomp_nl()
from the number of refs to the number of matching refs.

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

Strictly speaking this is a fundamental behavior change in
__git_refs(), a helper function that is likely used in users' custom
completion scriptlets.  However, arguably this change should be
harmless, because:

  - __git_refs() was only ever intended to feed refs to Bash's
    completion machinery, for which non-matching refs have to be
    filtered out anyway.

  - There were already code paths that list only matching refs
    (listing unique remote branches for 'git checkout's tracking
    DWIMery and listing full remote refs via 'git ls-remote').

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

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index d55eadfd1..615292f8b 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -394,7 +394,7 @@ __git_refs ()
 		case "$cur_" in
 		refs|refs/*)
 			format="refname"
-			refs="${cur_%/*}"
+			refs=("$cur_*" "$cur_*/**")
 			track=""
 			;;
 		*)
@@ -402,11 +402,13 @@ __git_refs ()
 				if [ -e "$dir/$i" ]; then echo $pfx$i; fi
 			done
 			format="refname:strip=2"
-			refs="refs/tags refs/heads refs/remotes"
+			refs=("refs/tags/$cur_*" "refs/tags/$cur_*/**"
+				"refs/heads/$cur_*" "refs/heads/$cur_*/**"
+				"refs/remotes/$cur_*" "refs/remotes/$cur_*/**")
 			;;
 		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
@@ -438,10 +440,12 @@ __git_refs ()
 		if [ "$list_refs_from" = remote ]; then
 			echo "HEAD"
 			__git for-each-ref --format="%(refname:strip=2)" \
-				"refs/remotes/$remote/" | sed -e "s#^$remote/##"
+				"refs/remotes/$remote/$cur_*" \
+				"refs/remotes/$remote/$cur_*/**" | sed -e "s#^$remote/##"
 		else
 			__git ls-remote "$remote" HEAD \
-				"refs/tags/*" "refs/heads/*" "refs/remotes/*" |
+				"refs/tags/$cur_*" "refs/heads/$cur_*" \
+				"refs/remotes/$cur_*" |
 			while read -r hash i; do
 				case "$i" in
 				*^{})	;;
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 7b42a686c..499be5879 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -837,6 +837,108 @@ 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 - only matching refs' '
+	cat >expected <<-EOF &&
+	HEAD
+	matching-branch
+	matching/branch
+	matching-tag
+	matching/tag
+	EOF
+	(
+		cur=mat &&
+		__git_refs >"$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 >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - only matching refs - remote on local file system' '
+	cat >expected <<-EOF &&
+	HEAD
+	master-in-other
+	matching/branch-in-other
+	EOF
+	(
+		cur=ma &&
+		__git_refs otherrepo >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - only matching refs - configured remote' '
+	cat >expected <<-EOF &&
+	HEAD
+	master-in-other
+	matching/branch-in-other
+	EOF
+	(
+		cur=ma &&
+		__git_refs other >"$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 >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - only matching refs - checkout DWIMery' '
+	cat >expected <<-EOF &&
+	HEAD
+	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 >"$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
+'
+
 test_expect_success '__git_complete_refs - simple' '
 	sed -e "s/Z$//g" >expected <<-EOF &&
 	HEAD Z
-- 
2.11.0.555.g967c1bcb3


^ permalink raw reply related

* [PATCH 03/12] completion: support completing full refs after '--option=refs/<TAB>'
From: SZEDER Gábor @ 2017-02-03  2:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor
In-Reply-To: <20170203025405.8242-1-szeder.dev@gmail.com>

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 7f19e2a4f..67a03cfd4 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 50c534072..8fe748839 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$//g" >expected <<-EOF &&
 	HEAD Z
-- 
2.11.0.555.g967c1bcb3


^ permalink raw reply related

* [PATCH 08/12] completion: let 'for-each-ref' strip the remote name from remote branches
From: SZEDER Gábor @ 2017-02-03  2:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor
In-Reply-To: <20170203025405.8242-1-szeder.dev@gmail.com>

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 615292f8b..8f1203025 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -414,11 +414,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" == "$cur_"* ]]; then
 					echo "$ref"
 				fi
@@ -439,9 +438,9 @@ __git_refs ()
 	*)
 		if [ "$list_refs_from" = remote ]; then
 			echo "HEAD"
-			__git for-each-ref --format="%(refname:strip=2)" \
+			__git for-each-ref --format="%(refname:strip=3)" \
 				"refs/remotes/$remote/$cur_*" \
-				"refs/remotes/$remote/$cur_*/**" | sed -e "s#^$remote/##"
+				"refs/remotes/$remote/$cur_*/**"
 		else
 			__git ls-remote "$remote" HEAD \
 				"refs/tags/$cur_*" "refs/heads/$cur_*" \
-- 
2.11.0.555.g967c1bcb3


^ permalink raw reply related

* [PATCH 12/12] completion: fill COMPREPLY directly when completing refs
From: SZEDER Gábor @ 2017-02-03  2:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor
In-Reply-To: <20170203025405.8242-1-szeder.dev@gmail.com>

__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.
  - 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.  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                  | 16 ++++++++++
 3 files changed, 64 insertions(+), 15 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 0ad02cec6..dbbb62f5f 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,17 +368,19 @@ __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 instead of the current word being
-#    completed (optional).
+#    completed (optional; NOT ignored, if empty, but lists all refs).
+# 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-}"
 
 	__git_find_repo_path
 	dir="$__git_repo_path"
@@ -389,7 +405,7 @@ __git_refs ()
 
 	if [ "$list_refs_from" = path ]; then
 		if [[ "$cur_" == ^* ]]; then
-			pfx="^"
+			pfx="$pfx^"
 			cur_=${cur_#^}
 		fi
 		case "$cur_" in
@@ -402,7 +418,7 @@ __git_refs ()
 			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
 				case "$i" in
 				$cur_*)	if [ -e "$dir/$i" ]; then
-						echo $pfx$i
+						echo "$pfx$i$sfx"
 					fi
 					;;
 				esac
@@ -413,13 +429,13 @@ __git_refs ()
 				"refs/remotes/$cur_*" "refs/remotes/$cur_*/**")
 			;;
 		esac
-		__git_dir="$dir" __git for-each-ref --format="$pfx%($format)" \
+		__git_dir="$dir" __git for-each-ref --format="$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="$pfx%(refname:strip=3)$sfx" \
 				--sort="refname:strip=3" \
 				"refs/remotes/*/$cur_*" "refs/remotes/*/$cur_*/**" | \
 			uniq -u
@@ -432,16 +448,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
-			$cur_*)	echo "HEAD" ;;
+			$cur_*)	echo "${pfx}HEAD$sfx" ;;
 			esac
-			__git for-each-ref --format="%(refname:strip=3)" \
+			__git for-each-ref --format="$pfx%(refname:strip=3)$sfx" \
 				"refs/remotes/$remote/$cur_*" \
 				"refs/remotes/$remote/$cur_*/**"
 		else
@@ -455,8 +471,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
@@ -491,8 +507,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)
@@ -2975,6 +2990,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 5e06acc17..1206a38ed 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
-- 
2.11.0.555.g967c1bcb3


^ permalink raw reply related

* [PATCH 10/12] completion: let 'for-each-ref' sort remote branches for 'checkout' DWIMery
From: SZEDER Gábor @ 2017-02-03  2:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor
In-Reply-To: <20170203025405.8242-1-szeder.dev@gmail.com>

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 e2c4794f3..9f5a6f44e 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -414,8 +414,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/*/$cur_*" "refs/remotes/*/$cur_*/**" | \
-			sort | uniq -u
+			uniq -u
 		fi
 		return
 	fi
-- 
2.11.0.555.g967c1bcb3


^ permalink raw reply related

* [PATCH 11/12] completion: list only matching symbolic and pseudorefs when completing refs
From: SZEDER Gábor @ 2017-02-03  2:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor
In-Reply-To: <20170203025405.8242-1-szeder.dev@gmail.com>

The previous changes in this series ensure that __git_refs() lists
only refs that match the current word to be completed, but
non-matching symbolic and pseudo refs still show up in its output.

Filter out non-matching symbolic and pseudo refs as well, so anything
__git_refs() outputs matches the current word to be completed, as it
will allow us to optimize how refs are placed into the COMPREPLY
array.

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

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9f5a6f44e..0ad02cec6 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 instead of the current word being
+#    completed (optional).
 #
 # Use __git_complete_refs() instead.
 __git_refs ()
@@ -399,7 +400,12 @@ __git_refs ()
 			;;
 		*)
 			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
-				if [ -e "$dir/$i" ]; then echo $pfx$i; fi
+				case "$i" in
+				$cur_*)	if [ -e "$dir/$i" ]; then
+						echo $pfx$i
+					fi
+					;;
+				esac
 			done
 			format="refname:strip=2"
 			refs=("refs/tags/$cur_*" "refs/tags/$cur_*/**"
@@ -432,12 +438,18 @@ __git_refs ()
 		;;
 	*)
 		if [ "$list_refs_from" = remote ]; then
-			echo "HEAD"
+			case "HEAD" in
+			$cur_*)	echo "HEAD" ;;
+			esac
 			__git for-each-ref --format="%(refname:strip=3)" \
 				"refs/remotes/$remote/$cur_*" \
 				"refs/remotes/$remote/$cur_*/**"
 		else
-			__git ls-remote "$remote" HEAD \
+			local query_symref
+			case "HEAD" in
+			$cur_*)	query_symref="HEAD" ;;
+			esac
+			__git ls-remote "$remote" $query_symref \
 				"refs/tags/$cur_*" "refs/heads/$cur_*" \
 				"refs/remotes/$cur_*" |
 			while read -r hash i; do
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 499be5879..5e06acc17 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -847,7 +847,6 @@ test_expect_success 'setup for filtering matching refs' '
 
 test_expect_success '__git_refs - only matching refs' '
 	cat >expected <<-EOF &&
-	HEAD
 	matching-branch
 	matching/branch
 	matching-tag
@@ -874,7 +873,6 @@ test_expect_success '__git_refs - only matching refs - full refs' '
 
 test_expect_success '__git_refs - only matching refs - remote on local file system' '
 	cat >expected <<-EOF &&
-	HEAD
 	master-in-other
 	matching/branch-in-other
 	EOF
@@ -887,7 +885,6 @@ test_expect_success '__git_refs - only matching refs - remote on local file syst
 
 test_expect_success '__git_refs - only matching refs - configured remote' '
 	cat >expected <<-EOF &&
-	HEAD
 	master-in-other
 	matching/branch-in-other
 	EOF
@@ -912,7 +909,6 @@ test_expect_success '__git_refs - only matching refs - remote - full refs' '
 
 test_expect_success '__git_refs - only matching refs - checkout DWIMery' '
 	cat >expected <<-EOF &&
-	HEAD
 	matching-branch
 	matching/branch
 	matching-tag
-- 
2.11.0.555.g967c1bcb3


^ permalink raw reply related

* [PATCH 06/12] completion: don't disambiguate short refs
From: SZEDER Gábor @ 2017-02-03  2:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor
In-Reply-To: <20170203025405.8242-1-szeder.dev@gmail.com>

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 19799e3ba..d55eadfd1 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.11.0.555.g967c1bcb3


^ permalink raw reply related

* [PATCH 04/12] completion: support excluding full refs
From: SZEDER Gábor @ 2017-02-03  2:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor
In-Reply-To: <20170203025405.8242-1-szeder.dev@gmail.com>

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 67a03cfd4..63e803154 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 8fe748839..7b42a686c 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$//g" >expected <<-EOF &&
 	HEAD Z
-- 
2.11.0.555.g967c1bcb3


^ permalink raw reply related

* [PATCH 05/12] completion: don't disambiguate tags and branches
From: SZEDER Gábor @ 2017-02-03  2:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor
In-Reply-To: <20170203025405.8242-1-szeder.dev@gmail.com>

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 63e803154..19799e3ba 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.11.0.555.g967c1bcb3


^ permalink raw reply related

* [PATCH 09/12] completion: let 'for-each-ref' filter remote branches for 'checkout' DWIMery
From: SZEDER Gábor @ 2017-02-03  2:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor
In-Reply-To: <20170203025405.8242-1-szeder.dev@gmail.com>

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 8f1203025..e2c4794f3 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -413,15 +413,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" == "$cur_"* ]]; then
-					echo "$ref"
-				fi
-			done | sort | uniq -u
+			__git for-each-ref --format="%(refname:strip=3)" \
+				"refs/remotes/*/$cur_*" "refs/remotes/*/$cur_*/**" | \
+			sort | uniq -u
 		fi
 		return
 	fi
-- 
2.11.0.555.g967c1bcb3


^ permalink raw reply related

* possible bug:  inconsistent CLI behaviour for empty user.name
From: bs.x.ttp @ 2017-02-03  4:13 UTC (permalink / raw)
  To: git

The problem is that GIT accepts a user.name of " " for some operations (for example when doing a simple "git commit"), but does require a "non-empty" user.name for others (like git commit --amend and git rebase). In case of the latter commands GIT fails with the message "fatal: empty ident name (for <email@address>) not allowed".

As people tend to do simple commits first, before amending or rebasing something, they may have to change their name after some dozen of commits which doesn't look nice.

This is certainly not a big issue, but it turns out to be quite annoying and I've already rewritten the history of a GIT repository once because of it, so that all my commits had the same author.

Proposed solution: GIT's requirements for user.name should not depend on the operation. Either user.name should be enforced to be non-empty everywhere or an empty user.name should be accepted everywhere. Perhaps filling out one of user.name and user.email could be sufficient.






^ permalink raw reply

* Re: [PATCH 00/12] completion: speed up refs completion
From: Jacob Keller @ 2017-02-03  4:15 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Junio C Hamano, Git mailing list
In-Reply-To: <20170203025405.8242-1-szeder.dev@gmail.com>

On Thu, Feb 2, 2017 at 6:53 PM, SZEDER Gábor <szeder.dev@gmail.com> wrote:
> This series speeds up refs completion for large number of refs, partly
> by giving up disambiguating ambiguous refs (patch 6) 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.
>
> The last patch touches the ZSH wrapper, too.  By a lucky educated
> guess I managed to get it work on the first try, but I don't really
> know what I've actually done, so...  ZSH users, please have a closer
> look.
>
> At the end of this series refs completion from a local repository is
> as fast as it can possibly get, at least as far as the completion
> script is concerned, because it basically does nothing anymore :)  All
> it does is run 'git for-each-ref' with assorted options to do all the
> work, and feed its output directly, without any processing into Bash's
> COMPREPLY array.  There is still room for improvements in the code
> paths using 'git ls-remote', but for that we would need enhancements
> to 'ls-remote'.
>
> It goes on top of the __gitdir() improvements series I just posted at:
>
>   http://public-inbox.org/git/20170203024829.8071-1-szeder.dev@gmail.com/T/
>
> This series is also available at:
>
>   https://github.com/szeder/git completion-refs-speedup
>

Nice! This is something i've been bothered by in the past since
completion would take a rather long time!

Regards,
Jake

^ permalink raw reply

* Re: How to use git show's "%<(<N>[,trunc|ltrunc|mtrunc])"?
From: G. Sylvie Davies @ 2017-02-03  4:19 UTC (permalink / raw)
  To: Hilco Wijbenga; +Cc: Git Users
In-Reply-To: <CAE1pOi0-8JnnZbdm9vu1RwTU1mXr7dboLC3ito3LcvK9gkNi_A@mail.gmail.com>

On Thu, Feb 2, 2017 at 9:51 AM, Hilco Wijbenga <hilco.wijbenga@gmail.com> wrote:
> Hi all,
>
> I'm trying to get the committer date printed in a custom fashion.
> Using "%cI" gets me close:
>
> $ git show --format="%cI | %an" master | head -n 1
> 2017-01-31T17:02:13-08:00 | Hilco Wijbenga
>
> I would like to get rid of the "-08:00" bit at the end of the
> timestamp. According to the "git show" manual I should be able to use
> "%<(<N>[,trunc|ltrunc|mtrunc])" to drop that last part.
>
> $ git show --format="%<(19,trunc)cI | %an" master | head -n 1
> cI | Hilco Wijbenga
>
> Mmm, it seems to be recognized as a valid "something" but it's not
> working as I had expected. :-) I tried several other versions of this
> but no luck. Clearly, I'm misunderstanding the format description. How
> do I get "2017-01-31T17:02:13 | Hilco Wijbenga" to be output?
>

Will this work for you?

$ git show -s --pretty='%cd | %an' --date=format:%FT%R:%S
2017-02-02T10:01:36 | G. Sylvie Davies


I have no idea how portable this might be.  As "git help log" says:

         --date=format:...  feeds the format ...  to your system
strftime. Use --date=format:%c to show the date in your system
locale’s preferred format. See the strftime manual for a complete list
of format placeholders.



- Sylvie

^ permalink raw reply

* Re: How to use git show's "%<(<N>[,trunc|ltrunc|mtrunc])"?
From: Hilco Wijbenga @ 2017-02-03  4:32 UTC (permalink / raw)
  To: G. Sylvie Davies; +Cc: Git Users
In-Reply-To: <CAAj3zPz8wFc_5dRmM=-jMnqp6a7fGtc4XSyU5meF6mO5me47Dw@mail.gmail.com>

On 2 February 2017 at 20:19, G. Sylvie Davies <sylvie@bit-booster.com> wrote:
> On Thu, Feb 2, 2017 at 9:51 AM, Hilco Wijbenga <hilco.wijbenga@gmail.com> wrote:
>> Hi all,
>>
>> I'm trying to get the committer date printed in a custom fashion.
>> Using "%cI" gets me close:
>>
>> $ git show --format="%cI | %an" master | head -n 1
>> 2017-01-31T17:02:13-08:00 | Hilco Wijbenga
>>
>> I would like to get rid of the "-08:00" bit at the end of the
>> timestamp. According to the "git show" manual I should be able to use
>> "%<(<N>[,trunc|ltrunc|mtrunc])" to drop that last part.
>>
>> $ git show --format="%<(19,trunc)cI | %an" master | head -n 1
>> cI | Hilco Wijbenga
>>
>> Mmm, it seems to be recognized as a valid "something" but it's not
>> working as I had expected. :-) I tried several other versions of this
>> but no luck. Clearly, I'm misunderstanding the format description. How
>> do I get "2017-01-31T17:02:13 | Hilco Wijbenga" to be output?
>>
>
> Will this work for you?
>
> $ git show -s --pretty='%cd | %an' --date=format:%FT%R:%S
> 2017-02-02T10:01:36 | G. Sylvie Davies

Ah, that does indeed do exactly what I want. Thank you.

> I have no idea how portable this might be.  As "git help log" says:
>
>          --date=format:...  feeds the format ...  to your system
> strftime. Use --date=format:%c to show the date in your system
> locale’s preferred format. See the strftime manual for a complete list
> of format placeholders.

It should be fine for my purposes.

Any idea why "%<(19,trunc)cl" doesn't work? (Your solution solves my
original problem perfectly but I'd like to understand how I'm
misreading the spec.)

^ permalink raw reply

* Re: difflame
From: Edmundo Carmona Antoranz @ 2017-02-03  4:46 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Git List
In-Reply-To: <CAOc6etYzAeD32oSjrvmv-PBJTdAGobsQ30iH8Q+Z9ShWOqiHfQ@mail.gmail.com>

I have been "scripting around git blame --reverse" for some days now.
Mind taking a look? I've been working on blame-deletions for this.

22:41 $ ../difflame/difflame.py HEAD~5 HEAD
diff --git a/file b/file
index b414108..051c298 100644
--- a/file
+++ b/file
@@ -1,6 +1,9 @@
^1513353 (Edmundo 2017-02-02 22:41:45 -0600 1) 1
f20952eb (Edmundo 2017-02-02 22:41:45 -0600 2) 2
bb04dc7c (Edmundo 2017-02-02 22:41:45 -0600 3) 3
-de3c07b (Edmundo 2017-02-02 22:41:47 -060 4) 4
058ea125 (Edmundo 2017-02-02 22:41:45 -0600 4) 5
85fc6b81 (Edmundo 2017-02-02 22:41:45 -0600 5) 6
+2cd990a6 (Edmundo 2017-02-02 22:41:45 -0600 6) 7
+ab0be970 (Edmundo 2017-02-02 22:41:45 -0600 7) 8
+944452c0 (Edmundo 2017-02-02 22:41:45 -0600 8) 9
+6641edb0 (Edmundo 2017-02-02 22:41:45 -0600 9) 10


$ git show de3c07b
commit de3c07bc21a83472d5c5ddf172dcb742665924dd (HEAD -> master)
Author: Edmundo <eantoranz@gmail.com>
Date:   Thu Feb 2 22:41:47 2017 -0600

   drop 4

diff --git a/file b/file
index f00c965..051c298 100644
--- a/file
+++ b/file
@@ -1,7 +1,6 @@
1
2
3
-4
5
6
7


Next step: solve the
find-real-deletion-revision-when-there-are-multiple-child-nodes
problem.... and let me read the discussion around git blame --reverse.

Thanks in advance.

On Mon, Jan 30, 2017 at 8:37 PM, Edmundo Carmona Antoranz
<eantoranz@gmail.com> wrote:
> Maybe a little work on blame to get the actual revision where some
> lines were "deleted"?
>
> Something like git blame --blame-deletion that could be based on --reverse?
>
> On Mon, Jan 30, 2017 at 7:35 PM, Edmundo Carmona Antoranz
> <eantoranz@gmail.com> wrote:
>> I'm thinking of something like this:
>>
>> Say I just discovered a problem in a file.... I want to see who worked
>> on it since some revision that I know is working fine (or even
>> something as generic as HEAD~100..). It could be a number of people
>> with different revisions. I would diff to see what changed.... and
>> blame the added lines (blame reverse to try to get as close as
>> possible with a single command in case I want to see what happened
>> with something that was deleted). If I could get blame information of
>> added/deleted lines in a single run, that would help a lot.
>>
>> Lo and behold: difflame.
>>
>>
>>
>> On Mon, Jan 30, 2017 at 5:26 PM, Jeff King <peff@peff.net> wrote:
>>> On Mon, Jan 30, 2017 at 01:08:41PM -0800, Junio C Hamano wrote:
>>>
>>>> Jeff King <peff@peff.net> writes:
>>>>
>>>> > On Tue, Jan 17, 2017 at 11:24:02PM -0600, Edmundo Carmona Antoranz wrote:
>>>> >
>>>> >> For a very long time I had wanted to get the output of diff to include
>>>> >> blame information as well (to see when something was added/removed).
>>>> >
>>>> > This is something I've wanted, too. The trickiest part, though, is
>>>> > blaming deletions, because git-blame only tracks the origin of content,
>>>> > not the origin of a change.
>>>>
>>>> Hmph, this is a comment without looking at what difflame does
>>>> internally, so you can ignore me if I am misunderstood what problem
>>>> you are pointing out, but I am not sure how "tracks the origin of
>>>> content" could be a problem.
>>>>
>>>> If output from "git show" says this:
>>>>
>>>>       --- a/file
>>>>       +++ b/file
>>>>       @@ -1,5 +1,6 @@
>>>>        a
>>>>        b
>>>>       -c
>>>>       +C
>>>>       +D
>>>>        d
>>>>        e
>>>>
>>>> in order to annotate lines 'a', 'b', 'd', and 'e' for their origin,
>>>> you would run 'blame' on the commit the above output was taken from
>>>> (or its parent---they are in the context so either would be OK).
>>>>
>>>> You know where 'C' and 'D' came from already.  It's the commit you
>>>> are feeding "git show".
>>>
>>> I think the point (or at least as I understand it) is that the diff is
>>> not "git show" for a particular commit. It could be part of a much
>>> larger diff that covers many commits.
>>>
>>> As a non-hypothetical instance, I have a fork of git.git that has
>>> various enhancements. I want to feed those enhancements upstream. I need
>>> to know which commits should be cherry-picked to get those various
>>> enhancements.
>>>
>>> Looking at "log origin..fork" tells me too many commits, because it
>>> includes ones which aren't useful anymore. Either because they already
>>> went upstream, or because they were cherry-picked to the fork and their
>>> upstream counterparts merged (or even equivalent commits made upstream
>>> that obsoleted the features).
>>>
>>> Looking at "git diff origin fork" tells me what the actual differences
>>> are, but it doesn't show me which commits are responsible for them.
>>>
>>> I can "git blame" each individual line of the diff (starting with "fork"
>>> as the tip), but that doesn't work for lines that no longer exist (i.e.,
>>> when the interesting change is a deletion).
>>>
>>>> In order to run blame to find where 'c' came from, you need to start
>>>> at the _parent_ of the commit the above output came from, and the
>>>> hunk header shows which line range to find the final 'c'.
>>>
>>> So perhaps that explains my comment more. "blame" is not good for
>>> finding which commit took away a line. I've tried using "blame
>>> --reverse", but it shows you the parent of the commit you are looking
>>> for, which is slightly annoying. :)
>>>
>>> "git log -S" is probably a better tool for finding that.
>>>
>>> -Peff

^ permalink raw reply related

* Re: difflame
From: Edmundo Carmona Antoranz @ 2017-02-03  4:47 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Git List
In-Reply-To: <CAOc6etabV=h6fEVee=N2-3ERUU7_w6eCM006mSMPqiwkRycQBw@mail.gmail.com>

On Thu, Feb 2, 2017 at 10:46 PM, Edmundo Carmona Antoranz
<eantoranz@gmail.com> wrote:
> I have been "scripting around git blame --reverse" for some days now.
> Mind taking a look? I've been working on blame-deletions for this.

blame-deletions branch, that is

Sorry for the previous top-posting.

^ permalink raw reply

* Re: How to use git show's "%<(<N>[,trunc|ltrunc|mtrunc])"?
From: Duy Nguyen @ 2017-02-03  8:06 UTC (permalink / raw)
  To: Hilco Wijbenga; +Cc: Git Users
In-Reply-To: <CAE1pOi0-8JnnZbdm9vu1RwTU1mXr7dboLC3ito3LcvK9gkNi_A@mail.gmail.com>

On Fri, Feb 3, 2017 at 12:51 AM, Hilco Wijbenga
<hilco.wijbenga@gmail.com> wrote:
> Hi all,
>
> I'm trying to get the committer date printed in a custom fashion.
> Using "%cI" gets me close:
>
> $ git show --format="%cI | %an" master | head -n 1
> 2017-01-31T17:02:13-08:00 | Hilco Wijbenga
>
> I would like to get rid of the "-08:00" bit at the end of the
> timestamp. According to the "git show" manual I should be able to use
> "%<(<N>[,trunc|ltrunc|mtrunc])" to drop that last part.
>
> $ git show --format="%<(19,trunc)cI | %an" master | head -n 1

You're almost there. Just insert another '%' between "trunc)" and "cI".

> How do I get "2017-01-31T17:02:13 | Hilco Wijbenga" to be output?

You'll get an ellipsis at the end though.. (i.e. 02:13... | Hilco)
-- 
Duy

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox