Skip to content

Commit fad9484

Browse files
szedergitster
authored andcommitted
completion: cache the path to the repository
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent beb6ee7 commit fad9484

File tree

2 files changed

+132
-75
lines changed

2 files changed

+132
-75
lines changed

contrib/completion/git-completion.bash

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,11 @@ esac
3939
# variable.
4040
__git_find_repo_path ()
4141
{
42+
if [ -n "$__git_repo_path" ]; then
43+
# we already know where it is
44+
return
45+
fi
46+
4247
if [ -n "${__git_C_args-}" ]; then
4348
__git_repo_path="$(git "${__git_C_args[@]}" \
4449
${__git_dir:+--git-dir="$__git_dir"} \
@@ -56,6 +61,7 @@ __git_find_repo_path ()
5661
fi
5762
}
5863

64+
# Deprecated: use __git_find_repo_path() and $__git_repo_path instead
5965
# __gitdir accepts 0 or 1 arguments (i.e., location)
6066
# returns location of .git repo
6167
__gitdir ()
@@ -350,10 +356,13 @@ __git_tags ()
350356
# 'git checkout's tracking DWIMery (optional; ignored, if set but empty).
351357
__git_refs ()
352358
{
353-
local i hash dir="$(__gitdir)" track="${2-}"
359+
local i hash dir track="${2-}"
354360
local list_refs_from=path remote="${1-}"
355361
local format refs pfx
356362

363+
__git_find_repo_path
364+
dir="$__git_repo_path"
365+
357366
if [ -z "$remote" ]; then
358367
if [ -z "$dir" ]; then
359368
return
@@ -458,8 +467,8 @@ __git_refs_remotes ()
458467

459468
__git_remotes ()
460469
{
461-
local d="$(__gitdir)"
462-
test -d "$d/remotes" && ls -1 "$d/remotes"
470+
__git_find_repo_path
471+
test -d "$__git_repo_path/remotes" && ls -1 "$__git_repo_path/remotes"
463472
__git remote
464473
}
465474

@@ -957,8 +966,8 @@ __git_whitespacelist="nowarn warn error error-all fix"
957966

958967
_git_am ()
959968
{
960-
local dir="$(__gitdir)"
961-
if [ -d "$dir"/rebase-apply ]; then
969+
__git_find_repo_path
970+
if [ -d "$__git_repo_path"/rebase-apply ]; then
962971
__gitcomp "--skip --continue --resolved --abort"
963972
return
964973
fi
@@ -1041,7 +1050,8 @@ _git_bisect ()
10411050
local subcommands="start bad good skip reset visualize replay log run"
10421051
local subcommand="$(__git_find_on_cmdline "$subcommands")"
10431052
if [ -z "$subcommand" ]; then
1044-
if [ -f "$(__gitdir)"/BISECT_START ]; then
1053+
__git_find_repo_path
1054+
if [ -f "$__git_repo_path"/BISECT_START ]; then
10451055
__gitcomp "$subcommands"
10461056
else
10471057
__gitcomp "replay start"
@@ -1146,8 +1156,8 @@ _git_cherry ()
11461156

11471157
_git_cherry_pick ()
11481158
{
1149-
local dir="$(__gitdir)"
1150-
if [ -f "$dir"/CHERRY_PICK_HEAD ]; then
1159+
__git_find_repo_path
1160+
if [ -f "$__git_repo_path"/CHERRY_PICK_HEAD ]; then
11511161
__gitcomp "--continue --quit --abort"
11521162
return
11531163
fi
@@ -1538,10 +1548,10 @@ __git_log_date_formats="relative iso8601 rfc2822 short local default raw"
15381548
_git_log ()
15391549
{
15401550
__git_has_doubledash && return
1551+
__git_find_repo_path
15411552

1542-
local g="$(__gitdir)"
15431553
local merge=""
1544-
if [ -f "$g/MERGE_HEAD" ]; then
1554+
if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
15451555
merge="--merge"
15461556
fi
15471557
case "$cur" in
@@ -1788,11 +1798,12 @@ _git_push ()
17881798

17891799
_git_rebase ()
17901800
{
1791-
local dir="$(__gitdir)"
1792-
if [ -f "$dir"/rebase-merge/interactive ]; then
1801+
__git_find_repo_path
1802+
if [ -f "$__git_repo_path"/rebase-merge/interactive ]; then
17931803
__gitcomp "--continue --skip --abort --quit --edit-todo"
17941804
return
1795-
elif [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
1805+
elif [ -d "$__git_repo_path"/rebase-apply ] || \
1806+
[ -d "$__git_repo_path"/rebase-merge ]; then
17961807
__gitcomp "--continue --skip --abort --quit"
17971808
return
17981809
fi
@@ -2467,8 +2478,8 @@ _git_reset ()
24672478

24682479
_git_revert ()
24692480
{
2470-
local dir="$(__gitdir)"
2471-
if [ -f "$dir"/REVERT_HEAD ]; then
2481+
__git_find_repo_path
2482+
if [ -f "$__git_repo_path"/REVERT_HEAD ]; then
24722483
__gitcomp "--continue --quit --abort"
24732484
return
24742485
fi
@@ -2865,9 +2876,10 @@ __gitk_main ()
28652876
__git_has_doubledash && return
28662877

28672878
local __git_repo_path
2868-
local g="$(__gitdir)"
2879+
__git_find_repo_path
2880+
28692881
local merge=""
2870-
if [ -f "$g/MERGE_HEAD" ]; then
2882+
if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
28712883
merge="--merge"
28722884
fi
28732885
case "$cur" in

0 commit comments

Comments
 (0)