* [PATCH 00/12] completion: speed up refs completion
@ 2017-02-03 2:53 SZEDER Gábor
2017-02-03 2:53 ` [PATCH 01/12] completion: remove redundant __gitcomp_nl() options from _git_commit() SZEDER Gábor
` (12 more replies)
0 siblings, 13 replies; 23+ messages in thread
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 [flat|nested] 23+ messages in thread
* [PATCH 01/12] completion: remove redundant __gitcomp_nl() options from _git_commit()
2017-02-03 2:53 [PATCH 00/12] completion: speed up refs completion SZEDER Gábor
@ 2017-02-03 2:53 ` SZEDER Gábor
2017-02-03 2:53 ` [PATCH 02/12] completion: wrap __git_refs() for better option parsing SZEDER Gábor
` (11 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: SZEDER Gábor @ 2017-02-03 2:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, SZEDER Gábor
Those two options are specifying the default values that
__gitcomp_nl() would use anyway when invoked with no options at all.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
contrib/completion/git-completion.bash | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 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 [flat|nested] 23+ messages in thread
* [PATCH 02/12] completion: wrap __git_refs() for better option parsing
2017-02-03 2:53 [PATCH 00/12] completion: speed up refs completion SZEDER Gábor
2017-02-03 2:53 ` [PATCH 01/12] completion: remove redundant __gitcomp_nl() options from _git_commit() SZEDER Gábor
@ 2017-02-03 2:53 ` SZEDER Gábor
2017-02-03 2:53 ` [PATCH 03/12] completion: support completing full refs after '--option=refs/<TAB>' SZEDER Gábor
` (10 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: SZEDER Gábor @ 2017-02-03 2:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, SZEDER Gábor
__git_refs() currently accepts two optional positional parameters: a
remote and a flag for 'git checkout's tracking DWIMery. To fix a
minor bug, and, more importantly, for faster refs completion, this
series will add three more parameters: a prefix, the current word to
be completed and a suffix, i.e. the options accepted by __gitcomp() &
friends, and will change __git_refs() to list only refs matching that
given current word and to add that given prefix and suffix to the
listed refs.
However, __git_refs() is the helper function that is most likely used
in users' custom completion scriptlets for their own git commands, and
we don't want to break those, so
- we can't change __git_refs()'s default output format, i.e. we
can't by default append a trailing space to every listed ref,
meaning that the suffix parameter containing the default trailing
space would have to be specified on every invocation, and
- we can't change the position of existing positional parameters
either, so there would have to be plenty of set-but-empty
placeholder positional parameters all over the completion script.
Furthermore, with five positional parameters it would be really hard
to remember which position means what.
To keep callsites simple, add the new wrapper function
__git_complete_refs() around __git_refs(), which:
- instead of positional parameters accepts real '--opt=val'-style
options and with minimalistic option parsing translates them to
__git_refs()'s and __gitcomp_nl()'s positional parameters, and
- includes the '__gitcomp_nl "$(__git_refs ...)" ...' command
substitution to make its behavior match its name and the behavior
of other __git_complete_* functions, and to limit future changes
in this series to __git_refs() and this new wrapper function.
Call this wrapper function instead of __git_refs() wherever possible
throughout the completion script, i.e. when __git_refs()'s output is
fed to __gitcomp_nl() right away without further processing, which
means all callsites except a single one in the __git_refs2() helper.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
contrib/completion/git-completion.bash | 102 ++++++++++++++++++++-----------
t/t9902-completion.sh | 106 +++++++++++++++++++++++++++++++++
2 files changed, 173 insertions(+), 35 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 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 [flat|nested] 23+ messages in thread
* [PATCH 03/12] completion: support completing full refs after '--option=refs/<TAB>'
2017-02-03 2:53 [PATCH 00/12] completion: speed up refs completion SZEDER Gábor
2017-02-03 2:53 ` [PATCH 01/12] completion: remove redundant __gitcomp_nl() options from _git_commit() SZEDER Gábor
2017-02-03 2:53 ` [PATCH 02/12] completion: wrap __git_refs() for better option parsing SZEDER Gábor
@ 2017-02-03 2:53 ` SZEDER Gábor
2017-02-03 2:53 ` [PATCH 04/12] completion: support excluding full refs SZEDER Gábor
` (9 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: SZEDER Gábor @ 2017-02-03 2:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, SZEDER Gábor
Completing full refs currently only works when the full ref stands on
in its own on the command line, but doesn't work when the current word
to be completed contains a prefix before the full ref, e.g.
'--option=refs/<TAB>' or 'master..refs/bis<TAB>'.
The reason is that __git_refs() looks at the current word to be
completed ($cur) as a whole to decide whether it has to list full (if
it starts with 'refs/') or short refs (otherwise). However, $cur also
holds said '--option=' or 'master..' prefixes, which of course throw
off this decision. Luckily, the default action is to list short refs,
that's why completing short refs happens to work even after a
'master..<TAB>' prefix and similar cases.
Pass only the ref part of the current word to be completed to
__git_refs() as a new positional parameter, so it can make the right
decision even if the whole current word contains some kind of a
prefix.
Make this new parameter the 4. positional parameter and leave the 3.
as an ignored placeholder for now (it will be used later in this patch
series).
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
contrib/completion/git-completion.bash | 21 ++++++++++++++-------
t/t9902-completion.sh | 31 +++++++++++++++++++++++++++++++
2 files changed, 45 insertions(+), 7 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 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 [flat|nested] 23+ messages in thread
* [PATCH 04/12] completion: support excluding full refs
2017-02-03 2:53 [PATCH 00/12] completion: speed up refs completion SZEDER Gábor
` (2 preceding siblings ...)
2017-02-03 2:53 ` [PATCH 03/12] completion: support completing full refs after '--option=refs/<TAB>' SZEDER Gábor
@ 2017-02-03 2:53 ` SZEDER Gábor
2017-02-03 2:53 ` [PATCH 05/12] completion: don't disambiguate tags and branches SZEDER Gábor
` (8 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: SZEDER Gábor @ 2017-02-03 2:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, SZEDER Gábor
Commit 49416ad22 (completion: support excluding refs, 2016-08-24) made
possible to complete short refs with a '^' prefix.
Extend the support to full refs to make completing '^refs/...' work.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
contrib/completion/git-completion.bash | 8 ++++----
t/t9902-completion.sh | 31 +++++++++++++++++++++++++++++++
2 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 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 [flat|nested] 23+ messages in thread
* [PATCH 05/12] completion: don't disambiguate tags and branches
2017-02-03 2:53 [PATCH 00/12] completion: speed up refs completion SZEDER Gábor
` (3 preceding siblings ...)
2017-02-03 2:53 ` [PATCH 04/12] completion: support excluding full refs SZEDER Gábor
@ 2017-02-03 2:53 ` SZEDER Gábor
2017-02-03 2:53 ` [PATCH 06/12] completion: don't disambiguate short refs SZEDER Gábor
` (7 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: SZEDER Gábor @ 2017-02-03 2:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, SZEDER Gábor
When the completion script has to list only tags or only branches, it
uses the 'git for-each-ref' format 'refname:short', which makes sure
that all listed tags and branches are unambiguous. However,
disambiguating tags and branches in these cases is wrong, because:
- __git_tags(), the helper function listing possible tagname
arguments for 'git tag', lists an ambiguous tag
'refs/tags/ambiguous' as 'tags/ambiguous'. Its only consumer,
'git tag' expects its tagname argument to be under 'refs/tags/',
thus it interprets that abgiguous tag as
'refs/tags/tags/ambiguous'. Clearly wrong.
- __git_heads() lists possible branchname arguments for 'git branch'
and possible 'branch.<branchname>' configuration subsections.
Both of these expect branchnames to be under 'refs/heads/' and
misinterpret a disambiguated branchname like 'heads/ambiguous'.
Furthermore, disambiguation involves several stat() syscalls for each
tag or branch, thus comes at a steep cost especially on Windows and/or
when there are a lot of tags or branches to be listed.
Use the 'git for-each-ref' format 'refname:strip=2' instead of
'refname:short' to avoid harmful disambiguation of tags and branches
in __git_tags() and __git_heads().
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
contrib/completion/git-completion.bash | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 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 [flat|nested] 23+ messages in thread
* [PATCH 06/12] completion: don't disambiguate short refs
2017-02-03 2:53 [PATCH 00/12] completion: speed up refs completion SZEDER Gábor
` (4 preceding siblings ...)
2017-02-03 2:53 ` [PATCH 05/12] completion: don't disambiguate tags and branches SZEDER Gábor
@ 2017-02-03 2:53 ` SZEDER Gábor
2017-02-03 2:54 ` [PATCH 07/12] completion: let 'for-each-ref' and 'ls-remote' filter matching refs SZEDER Gábor
` (6 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: SZEDER Gábor @ 2017-02-03 2:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, SZEDER Gábor
When the completion script lists short refs it does so using the 'git
for-each-ref' format 'refname:short', which makes sure that all listed
refs are unambiguous. While disambiguating refs is technically
correct in this case, as opposed to the cases discussed in the
previous patch, this disambiguation involves several stat() syscalls
for each ref, thus, unfortunately, comes at a steep cost especially on
Windows and/or when there are a lot of refs to be listed. A user of
Git for Windows reported[1] 'git checkout <TAB>' taking ~11 seconds in
a repository with just about 4000 refs.
However, it's questionable whether ambiguous refs are really that bad
to justify that much extra cost:
- Ambiguous refs are not that common,
- even if a repository contains ambiguous refs, they only hurt when
the user actually happens to want to do something with one of the
ambiguous refs, and
- the issue can be easily circumvented by renaming those ambiguous
refs.
- On the other hand, apparently not that many refs are needed to
make refs completion unacceptably slow on Windows,
- and this slowness bites each and every time the user attempts refs
completion, even when the repository doesn't contain any ambiguous
refs.
- Furthermore, circumventing the issue might not be possible or
might be considerably more difficult and requires various
trade-offs (e.g. working in a repository with only a few selected
important refs while keeping a separate repository with all refs
for reference).
Arguably, in this case the benefits of technical correctness are
rather minor compared to the price we pay for it, and we are better
off opting for performance over correctness.
Use the 'git for-each-ref' format 'refname:strip=2' to list short refs
to spare the substantial cost of disambiguating.
This speeds up refs completion considerably. Uniquely completing a
branch in a repository with 100k local branches, all packed, best of
five:
On Linux, before:
$ time __git_complete_refs --cur=maste
real 0m1.662s
user 0m1.368s
sys 0m0.296s
After:
real 0m0.831s
user 0m0.808s
sys 0m0.028s
On Windows, before:
real 0m12.457s
user 0m1.016s
sys 0m0.092s
After:
real 0m1.480s
user 0m1.031s
sys 0m0.060s
[1] - https://github.com/git-for-windows/git/issues/524
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
contrib/completion/git-completion.bash | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 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 [flat|nested] 23+ messages in thread
* [PATCH 07/12] completion: let 'for-each-ref' and 'ls-remote' filter matching refs
2017-02-03 2:53 [PATCH 00/12] completion: speed up refs completion SZEDER Gábor
` (5 preceding siblings ...)
2017-02-03 2:53 ` [PATCH 06/12] completion: don't disambiguate short refs SZEDER Gábor
@ 2017-02-03 2:54 ` SZEDER Gábor
2017-02-03 2:54 ` [PATCH 08/12] completion: let 'for-each-ref' strip the remote name from remote branches SZEDER Gábor
` (5 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: SZEDER Gábor @ 2017-02-03 2:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, SZEDER Gábor
When completing refs, several __git_refs() code paths list all the
refs from the refs/{heads,tags,remotes}/ hierarchy and then
__gitcomp_nl() iterates over those refs in a shell loop to filter out
refs not matching the current 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 [flat|nested] 23+ messages in thread
* [PATCH 08/12] completion: let 'for-each-ref' strip the remote name from remote branches
2017-02-03 2:53 [PATCH 00/12] completion: speed up refs completion SZEDER Gábor
` (6 preceding siblings ...)
2017-02-03 2:54 ` [PATCH 07/12] completion: let 'for-each-ref' and 'ls-remote' filter matching refs SZEDER Gábor
@ 2017-02-03 2:54 ` SZEDER Gábor
2017-02-03 2:54 ` [PATCH 09/12] completion: let 'for-each-ref' filter remote branches for 'checkout' DWIMery SZEDER Gábor
` (4 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: SZEDER Gábor @ 2017-02-03 2:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, SZEDER Gábor
The code listing unique remote branches for 'git checkout's tracking
DWIMery uses a shell parameter expansion in a loop iterating over each
listed ref to remove the remote's name from the remote branches, i.e.
the leading path component from the short ref. When listing refs from
a configured remote repository, '| sed s///' is used for the same
purpose.
Let 'git for-each-ref' strip one more leading path component from the
refs, i.e. use the format 'refname:strip=3' instead of '=2', making
that parameter expansion and 'sed' execution unnecessary.
This speeds up refs completion for 'git checkout'. Uniquely
completing a branch for 'git checkout maste<TAB>' in a repo with 100k
remote branches, all packed, best of five:
On Linux, near the beginning of this series, for reference:
$ time __git_complete_refs --cur=maste --track
real 0m8.185s
user 0m6.896s
sys 0m1.616s
Before this patch:
real 0m2.714s
user 0m2.344s
sys 0m0.436s
After:
real 0m1.993s
user 0m1.740s
sys 0m0.304s
On Windows, near the beginning:
real 1m8.421s
user 0m7.591s
sys 0m3.557s
Before this patch:
real 0m8.191s
user 0m4.638s
sys 0m2.918s
After:
real 0m6.187s
user 0m3.358s
sys 0m2.121s
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
contrib/completion/git-completion.bash | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 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 [flat|nested] 23+ messages in thread
* [PATCH 09/12] completion: let 'for-each-ref' filter remote branches for 'checkout' DWIMery
2017-02-03 2:53 [PATCH 00/12] completion: speed up refs completion SZEDER Gábor
` (7 preceding siblings ...)
2017-02-03 2:54 ` [PATCH 08/12] completion: let 'for-each-ref' strip the remote name from remote branches SZEDER Gábor
@ 2017-02-03 2:54 ` SZEDER Gábor
2017-02-03 2:54 ` [PATCH 10/12] completion: let 'for-each-ref' sort " SZEDER Gábor
` (3 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: SZEDER Gábor @ 2017-02-03 2:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, SZEDER Gábor
The code listing unique remote branches for 'git checkout's tracking
DWIMery outputs only remote branches that match the current word to be
completed, but the filtering is done in a shell loop iterating over
all remote refs.
Let 'git for-each-ref' do the filtering, as it can do so much more
efficiently and we can remove that shell loop entirely.
This speeds up refs completion for 'git checkout' considerably when
there are a lot of non-matching remote refs to be filtered out.
Uniquely completing a branch in a repository with 100k remote
branches, all packed, best of five:
On Linux, before:
$ time __git_complete_refs --cur=maste --track
real 0m1.993s
user 0m1.740s
sys 0m0.304s
After:
real 0m0.266s
user 0m0.248s
sys 0m0.012s
On Windows, before:
real 0m6.187s
user 0m3.358s
sys 0m2.121s
After:
real 0m0.750s
user 0m0.015s
sys 0m0.090s
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
contrib/completion/git-completion.bash | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 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 [flat|nested] 23+ messages in thread
* [PATCH 10/12] completion: let 'for-each-ref' sort remote branches for 'checkout' DWIMery
2017-02-03 2:53 [PATCH 00/12] completion: speed up refs completion SZEDER Gábor
` (8 preceding siblings ...)
2017-02-03 2:54 ` [PATCH 09/12] completion: let 'for-each-ref' filter remote branches for 'checkout' DWIMery SZEDER Gábor
@ 2017-02-03 2:54 ` SZEDER Gábor
2017-02-03 2:54 ` [PATCH 11/12] completion: list only matching symbolic and pseudorefs when completing refs SZEDER Gábor
` (2 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: SZEDER Gábor @ 2017-02-03 2:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, SZEDER Gábor
When listing unique remote branches for 'git checkout's tracking
DWIMery, __git_refs() runs the classic '... |sort |uniq -u' pattern to
filter out duplicate remote branches.
Let 'git for-each-ref' do the sorting, sparing the overhead of
fork()+exec()ing 'sort' and a stage in the pipeline where potentially
relatively large amount of data can be passed between two subsequent
pipeline stages.
This speeds up refs completion for 'git checkout' a bit when a lot of
remote branches match the current word to be completed. Listing a
single local and 100k remote branches, all packed, best of five:
On Linux, before:
$ time __git_complete_refs --track
real 0m1.856s
user 0m1.816s
sys 0m0.060s
After:
real 0m1.550s
user 0m1.512s
sys 0m0.060s
On Windows, before:
real 0m3.128s
user 0m2.155s
sys 0m0.183s
After:
real 0m2.781s
user 0m1.826s
sys 0m0.136s
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
contrib/completion/git-completion.bash | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 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 [flat|nested] 23+ messages in thread
* [PATCH 11/12] completion: list only matching symbolic and pseudorefs when completing refs
2017-02-03 2:53 [PATCH 00/12] completion: speed up refs completion SZEDER Gábor
` (9 preceding siblings ...)
2017-02-03 2:54 ` [PATCH 10/12] completion: let 'for-each-ref' sort " SZEDER Gábor
@ 2017-02-03 2:54 ` SZEDER Gábor
2017-02-03 2:54 ` [PATCH 12/12] completion: fill COMPREPLY directly " SZEDER Gábor
2017-02-03 4:15 ` [PATCH 00/12] completion: speed up refs completion Jacob Keller
12 siblings, 0 replies; 23+ messages in thread
From: SZEDER Gábor @ 2017-02-03 2:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, SZEDER Gábor
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 [flat|nested] 23+ messages in thread
* [PATCH 12/12] completion: fill COMPREPLY directly when completing refs
2017-02-03 2:53 [PATCH 00/12] completion: speed up refs completion SZEDER Gábor
` (10 preceding siblings ...)
2017-02-03 2:54 ` [PATCH 11/12] completion: list only matching symbolic and pseudorefs when completing refs SZEDER Gábor
@ 2017-02-03 2:54 ` SZEDER Gábor
2017-02-06 18:15 ` [PATCH] squash! " SZEDER Gábor
2017-02-03 4:15 ` [PATCH 00/12] completion: speed up refs completion Jacob Keller
12 siblings, 1 reply; 23+ messages in thread
From: SZEDER Gábor @ 2017-02-03 2:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, SZEDER Gábor
__gitcomp_nl() iterates over all the possible completion words it gets
as argument
- filtering matching words,
- appending a trailing space to each matching word (in all but two
cases),
- prepending a prefix to each matching word (when completing words
after e.g. '--option=<TAB>' or 'master..<TAB>'), and
- adding each matching word to the COMPREPLY array.
This takes a while when a lot of refs are passed to __gitcomp_nl().
The previous changes in this series ensure that __git_refs() lists
only refs matching the current word to be completed, making a second
filtering in __gitcomp_nl() redundant.
Adding the necessary prefix and suffix could be done in __git_refs()
as well:
- When refs come from 'git for-each-ref', then that prefix and
suffix could be added much more efficiently using a 'git
for-each-ref' format containing said prefix and suffix.
- 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 [flat|nested] 23+ messages in thread
* Re: [PATCH 00/12] completion: speed up refs completion
2017-02-03 2:53 [PATCH 00/12] completion: speed up refs completion SZEDER Gábor
` (11 preceding siblings ...)
2017-02-03 2:54 ` [PATCH 12/12] completion: fill COMPREPLY directly " SZEDER Gábor
@ 2017-02-03 4:15 ` Jacob Keller
2017-02-04 3:15 ` Jacob Keller
12 siblings, 1 reply; 23+ messages in thread
From: Jacob Keller @ 2017-02-03 4:15 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Junio C Hamano, Git mailing list
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 [flat|nested] 23+ messages in thread
* Re: [PATCH 00/12] completion: speed up refs completion
2017-02-03 4:15 ` [PATCH 00/12] completion: speed up refs completion Jacob Keller
@ 2017-02-04 3:15 ` Jacob Keller
2017-02-04 6:21 ` Junio C Hamano
2017-02-06 18:31 ` Jacob Keller
0 siblings, 2 replies; 23+ messages in thread
From: Jacob Keller @ 2017-02-04 3:15 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Junio C Hamano, Git mailing list
On Thu, Feb 2, 2017 at 8:15 PM, Jacob Keller <jacob.keller@gmail.com> wrote:
> 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
I haven't had a chance to further investigate, but I tried this series
out (from your github) and it appears that this series (or the
previous series for __gitdir work) breaks "git log" ref completion.
I'll have further details when I am able to investigate a it more.
Thanks,
Jake
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 00/12] completion: speed up refs completion
2017-02-04 3:15 ` Jacob Keller
@ 2017-02-04 6:21 ` Junio C Hamano
2017-02-06 18:31 ` Jacob Keller
1 sibling, 0 replies; 23+ messages in thread
From: Junio C Hamano @ 2017-02-04 6:21 UTC (permalink / raw)
To: Jacob Keller; +Cc: SZEDER Gábor, Git mailing list
Jacob Keller <jacob.keller@gmail.com> writes:
> On Thu, Feb 2, 2017 at 8:15 PM, Jacob Keller <jacob.keller@gmail.com> wrote:
>> 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
>>> ...
>>> 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/
>>>
>> Nice! This is something i've been bothered by in the past since
>> completion would take a rather long time!
>
> I haven't had a chance to further investigate, but I tried this series
> out (from your github) and it appears that this series (or the
> previous series for __gitdir work) breaks "git log" ref completion.
> I'll have further details when I am able to investigate a it more.
Thanks, both. I'll look forward to how the story unfolds from
sidelines ;-)
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] squash! completion: fill COMPREPLY directly when completing refs
2017-02-03 2:54 ` [PATCH 12/12] completion: fill COMPREPLY directly " SZEDER Gábor
@ 2017-02-06 18:15 ` SZEDER Gábor
2017-02-10 21:44 ` Junio C Hamano
0 siblings, 1 reply; 23+ messages in thread
From: SZEDER Gábor @ 2017-02-06 18:15 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, SZEDER Gábor
Care should be taken, though, because that prefix might contain
'for-each-ref' format specifiers as part of the left hand side of a
'..' range or '...' symmetric difference notation or fetch/push/etc.
refspec, e.g. 'git log "evil-%(refname)..br<TAB>'. Doubling every '%'
in the prefix will prevent 'git for-each-ref' from interpolating any
of those contained format specifiers.
---
This is really pathological, and I'm sure this has nothing to do with
whatever breakage Jacob experienced.
The shell metacharacters '(' and ')' still cause us trouble in various
ways, but that's nothing new and has been the case for quite a while
(always?).
It's already incorporated into (the rewritten)
https://github.com/szeder/git completion-refs-speedup
contrib/completion/git-completion.bash | 8 +++++---
t/t9902-completion.sh | 11 +++++++++++
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index dbbb62f5f..058f1d0ee 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -381,6 +381,7 @@ __git_refs ()
local list_refs_from=path remote="${1-}"
local format refs
local pfx="${3-}" cur_="${4-$cur}" sfx="${5-}"
+ local fer_pfx="${pfx//\%/%%}"
__git_find_repo_path
dir="$__git_repo_path"
@@ -406,6 +407,7 @@ __git_refs ()
if [ "$list_refs_from" = path ]; then
if [[ "$cur_" == ^* ]]; then
pfx="$pfx^"
+ fer_pfx="$fer_pfx^"
cur_=${cur_#^}
fi
case "$cur_" in
@@ -429,13 +431,13 @@ __git_refs ()
"refs/remotes/$cur_*" "refs/remotes/$cur_*/**")
;;
esac
- __git_dir="$dir" __git for-each-ref --format="$pfx%($format)$sfx" \
+ __git_dir="$dir" __git for-each-ref --format="$fer_pfx%($format)$sfx" \
"${refs[@]}"
if [ -n "$track" ]; then
# employ the heuristic used by git checkout
# Try to find a remote branch that matches the completion word
# but only output if the branch name is unique
- __git for-each-ref --format="$pfx%(refname:strip=3)$sfx" \
+ __git for-each-ref --format="$fer_pfx%(refname:strip=3)$sfx" \
--sort="refname:strip=3" \
"refs/remotes/*/$cur_*" "refs/remotes/*/$cur_*/**" | \
uniq -u
@@ -457,7 +459,7 @@ __git_refs ()
case "HEAD" in
$cur_*) echo "${pfx}HEAD$sfx" ;;
esac
- __git for-each-ref --format="$pfx%(refname:strip=3)$sfx" \
+ __git for-each-ref --format="$fer_pfx%(refname:strip=3)$sfx" \
"refs/remotes/$remote/$cur_*" \
"refs/remotes/$remote/$cur_*/**"
else
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 1206a38ed..be584c069 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -951,6 +951,17 @@ test_expect_success 'teardown after filtering matching refs' '
git update-ref -d refs/remotes/other/matching/branch-in-other
'
+test_expect_success '__git_refs - for-each-ref format specifiers in prefix' '
+ cat >expected <<-EOF &&
+ evil-%%-%42-%(refname)..master
+ EOF
+ (
+ cur="evil-%%-%42-%(refname)..mas" &&
+ __git_refs "" "" "evil-%%-%42-%(refname).." mas >"$actual"
+ ) &&
+ test_cmp expected "$actual"
+'
+
test_expect_success '__git_complete_refs - simple' '
sed -e "s/Z$//g" >expected <<-EOF &&
HEAD Z
--
2.11.1.499.gb6fcc8b3a
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 00/12] completion: speed up refs completion
2017-02-04 3:15 ` Jacob Keller
2017-02-04 6:21 ` Junio C Hamano
@ 2017-02-06 18:31 ` Jacob Keller
2017-02-06 19:36 ` SZEDER Gábor
1 sibling, 1 reply; 23+ messages in thread
From: Jacob Keller @ 2017-02-06 18:31 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Junio C Hamano, Git mailing list
On Fri, Feb 3, 2017 at 7:15 PM, Jacob Keller <jacob.keller@gmail.com> wrote:
> I haven't had a chance to further investigate, but I tried this series
> out (from your github) and it appears that this series (or the
> previous series for __gitdir work) breaks "git log" ref completion.
> I'll have further details when I am able to investigate a it more.
>
> Thanks,
> Jake
At first I had the same problem, but I verified by re-installing the
completion script and the problem appears to have gone away. I suspect
what happened is that the original time, I forgot to actually install
the new version of git, and only installed the completion script, so
when some of the commands were run with new options they (silently)
failed and the result was missing completion values.
Once I properly re-installed everything it appears to work as
expected. I haven't found any other issues yet.
Regards,
Jake
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 00/12] completion: speed up refs completion
2017-02-06 18:31 ` Jacob Keller
@ 2017-02-06 19:36 ` SZEDER Gábor
2017-02-06 23:55 ` Jacob Keller
0 siblings, 1 reply; 23+ messages in thread
From: SZEDER Gábor @ 2017-02-06 19:36 UTC (permalink / raw)
To: Jacob Keller; +Cc: Junio C Hamano, Git mailing list
On Mon, Feb 6, 2017 at 7:31 PM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Fri, Feb 3, 2017 at 7:15 PM, Jacob Keller <jacob.keller@gmail.com> wrote:
>> I haven't had a chance to further investigate, but I tried this series
>> out (from your github) and it appears that this series (or the
>> previous series for __gitdir work) breaks "git log" ref completion.
>> I'll have further details when I am able to investigate a it more.
>>
>> Thanks,
>> Jake
>
> At first I had the same problem, but I verified by re-installing the
> completion script and the problem appears to have gone away. I suspect
> what happened is that the original time, I forgot to actually install
> the new version of git, and only installed the completion script, so
> when some of the commands were run with new options they (silently)
> failed and the result was missing completion values.
>
> Once I properly re-installed everything it appears to work as
> expected. I haven't found any other issues yet.
Thanks, that's good to hear.
Still, I'm a bit puzzled as to what exactly might have caused your
problem. Considering new options:
- the __gitdir()-related series added the 'git rev-parse
--absolute-git-dir' option, but only ever used it if you invoked
completion after 'git -C some/where'.
- The refs completion speedup didn't add any new options but started
to use two that it previously didn't:
- 'git for-each-ref --sort=<key>' option, but that's with us since
the earliest ever 'for-each-ref' version from more than a decade
ago...
- 'git for-each-ref' format modifier 'strip=2', which was
introduced in v2.7.1~15^2 (tag: do not show ambiguous tag names
as "tags/foo", 2016-01-25), only about a year ago. Were you
using a pre-2.7.1 version when seeing the problems?
Gábor
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 00/12] completion: speed up refs completion
2017-02-06 19:36 ` SZEDER Gábor
@ 2017-02-06 23:55 ` Jacob Keller
0 siblings, 0 replies; 23+ messages in thread
From: Jacob Keller @ 2017-02-06 23:55 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Junio C Hamano, Git mailing list
On Mon, Feb 6, 2017 at 11:36 AM, SZEDER Gábor <szeder.dev@gmail.com> wrote:
> On Mon, Feb 6, 2017 at 7:31 PM, Jacob Keller <jacob.keller@gmail.com> wrote:
>> On Fri, Feb 3, 2017 at 7:15 PM, Jacob Keller <jacob.keller@gmail.com> wrote:
>>> I haven't had a chance to further investigate, but I tried this series
>>> out (from your github) and it appears that this series (or the
>>> previous series for __gitdir work) breaks "git log" ref completion.
>>> I'll have further details when I am able to investigate a it more.
>>>
>>> Thanks,
>>> Jake
>>
>> At first I had the same problem, but I verified by re-installing the
>> completion script and the problem appears to have gone away. I suspect
>> what happened is that the original time, I forgot to actually install
>> the new version of git, and only installed the completion script, so
>> when some of the commands were run with new options they (silently)
>> failed and the result was missing completion values.
>>
>> Once I properly re-installed everything it appears to work as
>> expected. I haven't found any other issues yet.
>
> Thanks, that's good to hear.
>
> Still, I'm a bit puzzled as to what exactly might have caused your
> problem. Considering new options:
>
> - the __gitdir()-related series added the 'git rev-parse
> --absolute-git-dir' option, but only ever used it if you invoked
> completion after 'git -C some/where'.
>
> - The refs completion speedup didn't add any new options but started
> to use two that it previously didn't:
>
> - 'git for-each-ref --sort=<key>' option, but that's with us since
> the earliest ever 'for-each-ref' version from more than a decade
> ago...
>
> - 'git for-each-ref' format modifier 'strip=2', which was
> introduced in v2.7.1~15^2 (tag: do not show ambiguous tag names
> as "tags/foo", 2016-01-25), only about a year ago. Were you
> using a pre-2.7.1 version when seeing the problems?
>
> Gábor
Nope. I was using some version of next at some point recently (less
than a couple months old). I do not know exactly what caused it, and
I'm not really able to find out because I can't reproduce it any more.
(after a fresh make install).
Thanks,
Jake
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] squash! completion: fill COMPREPLY directly when completing refs
2017-02-06 18:15 ` [PATCH] squash! " SZEDER Gábor
@ 2017-02-10 21:44 ` Junio C Hamano
2017-02-13 19:32 ` SZEDER Gábor
0 siblings, 1 reply; 23+ messages in thread
From: Junio C Hamano @ 2017-02-10 21:44 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git
SZEDER Gábor <szeder.dev@gmail.com> writes:
> Care should be taken, though, because that prefix might contain
> 'for-each-ref' format specifiers as part of the left hand side of a
> '..' range or '...' symmetric difference notation or fetch/push/etc.
> refspec, e.g. 'git log "evil-%(refname)..br<TAB>'. Doubling every '%'
> in the prefix will prevent 'git for-each-ref' from interpolating any
> of those contained format specifiers.
> ---
>
> This is really pathological, and I'm sure this has nothing to do
> with whatever breakage Jacob experienced. The shell
> metacharacters '(' and ')' still cause us trouble in various ways,
> but that's nothing new and has been the case for quite a while
> (always?).
>
> It's already incorporated into (the rewritten)
>
> https://github.com/szeder/git completion-refs-speedup
Should I expect a reroll to come, or is this the only fix-up to the
series that begins at <20170203025405.8242-1-szeder.dev@gmail.com>?
No hurries.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] squash! completion: fill COMPREPLY directly when completing refs
2017-02-10 21:44 ` Junio C Hamano
@ 2017-02-13 19:32 ` SZEDER Gábor
2017-02-13 20:24 ` Junio C Hamano
0 siblings, 1 reply; 23+ messages in thread
From: SZEDER Gábor @ 2017-02-13 19:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git mailing list
On Fri, Feb 10, 2017 at 10:44 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Should I expect a reroll to come, or is this the only fix-up to the
> series that begins at <20170203025405.8242-1-szeder.dev@gmail.com>?
>
> No hurries.
Yes, definitely.
I found a minor bug in the middle of the series, and haven't quite
made up my mind about it and its fix yet. Perhaps in a day or three.
Regarding the 'strip' -> 'lstrip' 'for-each-ref' format modifier
rename that broke pu: I should keep using 'strip', right?
Gábor
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] squash! completion: fill COMPREPLY directly when completing refs
2017-02-13 19:32 ` SZEDER Gábor
@ 2017-02-13 20:24 ` Junio C Hamano
0 siblings, 0 replies; 23+ messages in thread
From: Junio C Hamano @ 2017-02-13 20:24 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Git mailing list
SZEDER Gábor <szeder.dev@gmail.com> writes:
> On Fri, Feb 10, 2017 at 10:44 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
>> Should I expect a reroll to come, or is this the only fix-up to the
>> series that begins at <20170203025405.8242-1-szeder.dev@gmail.com>?
>>
>> No hurries.
>
> Yes, definitely.
>
> I found a minor bug in the middle of the series, and haven't quite
> made up my mind about it and its fix yet. Perhaps in a day or three.
>
> Regarding the 'strip' -> 'lstrip' 'for-each-ref' format modifier
> rename that broke pu: I should keep using 'strip', right?
Right. I view the removal of 'strip' as an accident when 'lstrip'
was introduced, not an intended change.
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2017-02-13 20:24 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-03 2:53 [PATCH 00/12] completion: speed up refs completion SZEDER Gábor
2017-02-03 2:53 ` [PATCH 01/12] completion: remove redundant __gitcomp_nl() options from _git_commit() SZEDER Gábor
2017-02-03 2:53 ` [PATCH 02/12] completion: wrap __git_refs() for better option parsing SZEDER Gábor
2017-02-03 2:53 ` [PATCH 03/12] completion: support completing full refs after '--option=refs/<TAB>' SZEDER Gábor
2017-02-03 2:53 ` [PATCH 04/12] completion: support excluding full refs SZEDER Gábor
2017-02-03 2:53 ` [PATCH 05/12] completion: don't disambiguate tags and branches SZEDER Gábor
2017-02-03 2:53 ` [PATCH 06/12] completion: don't disambiguate short refs SZEDER Gábor
2017-02-03 2:54 ` [PATCH 07/12] completion: let 'for-each-ref' and 'ls-remote' filter matching refs SZEDER Gábor
2017-02-03 2:54 ` [PATCH 08/12] completion: let 'for-each-ref' strip the remote name from remote branches SZEDER Gábor
2017-02-03 2:54 ` [PATCH 09/12] completion: let 'for-each-ref' filter remote branches for 'checkout' DWIMery SZEDER Gábor
2017-02-03 2:54 ` [PATCH 10/12] completion: let 'for-each-ref' sort " SZEDER Gábor
2017-02-03 2:54 ` [PATCH 11/12] completion: list only matching symbolic and pseudorefs when completing refs SZEDER Gábor
2017-02-03 2:54 ` [PATCH 12/12] completion: fill COMPREPLY directly " SZEDER Gábor
2017-02-06 18:15 ` [PATCH] squash! " SZEDER Gábor
2017-02-10 21:44 ` Junio C Hamano
2017-02-13 19:32 ` SZEDER Gábor
2017-02-13 20:24 ` Junio C Hamano
2017-02-03 4:15 ` [PATCH 00/12] completion: speed up refs completion Jacob Keller
2017-02-04 3:15 ` Jacob Keller
2017-02-04 6:21 ` Junio C Hamano
2017-02-06 18:31 ` Jacob Keller
2017-02-06 19:36 ` SZEDER Gábor
2017-02-06 23:55 ` Jacob Keller
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).