From 30bd55f901c97c310e674c051f00920f38a66ee0 Mon Sep 17 00:00:00 2001 From: Philippe Blain Date: Sat, 10 Feb 2024 18:32:20 +0000 Subject: [PATCH 1/4] completion: add space after config variable names also in Bash 3 In be6444d1ca (completion: bash: add correct suffix in variables, 2021-08-16), __git_complete_config_variable_name was changed to use "${sfx- }" instead of "$sfx" as the fourth argument of _gitcomp_nl and _gitcomp_nl_append, such that this argument evaluates to a space if sfx is unset. This was to ensure that e.g. git config branch.autoSetupMe[TAB] correctly completes to 'branch.autoSetupMerge ' with the trailing space. This commits notes that the fix only works in Bash 4 because in Bash 3 the 'local sfx' construct at the beginning of __git_complete_config_variable_name creates an empty string. Make the fix also work for Bash 3 by using the "unset or null' parameter expansion syntax ("${sfx:- }"), such that the parameter is also expanded to a space if it is set but null, as is the behaviour of 'local sfx' in Bash 3. Signed-off-by: Philippe Blain Signed-off-by: Junio C Hamano --- 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 6662db221d..159a4fd8ad 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2750,7 +2750,7 @@ __git_complete_config_variable_name () local pfx="${cur_%.*}." cur_="${cur_#*.}" __gitcomp_direct "$(__git_heads "$pfx" "$cur_" ".")" - __gitcomp_nl_append $'autoSetupMerge\nautoSetupRebase\n' "$pfx" "$cur_" "${sfx- }" + __gitcomp_nl_append $'autoSetupMerge\nautoSetupRebase\n' "$pfx" "$cur_" "${sfx:- }" return ;; guitool.*.*) @@ -2784,7 +2784,7 @@ __git_complete_config_variable_name () local pfx="${cur_%.*}." cur_="${cur_#*.}" __git_compute_all_commands - __gitcomp_nl "$__git_all_commands" "$pfx" "$cur_" "${sfx- }" + __gitcomp_nl "$__git_all_commands" "$pfx" "$cur_" "${sfx:- }" return ;; remote.*.*) @@ -2800,7 +2800,7 @@ __git_complete_config_variable_name () local pfx="${cur_%.*}." cur_="${cur_#*.}" __gitcomp_nl "$(__git_remotes)" "$pfx" "$cur_" "." - __gitcomp_nl_append "pushDefault" "$pfx" "$cur_" "${sfx- }" + __gitcomp_nl_append "pushDefault" "$pfx" "$cur_" "${sfx:- }" return ;; url.*.*) From b1d0cc68d1921f8d26a947fe662697e42dffb730 Mon Sep 17 00:00:00 2001 From: Philippe Blain Date: Sat, 10 Feb 2024 18:32:21 +0000 Subject: [PATCH 2/4] completion: complete 'submodule.*' config variables In the Bash completion script, function __git_complete_config_variable_name completes config variables and has special logic to deal with config variables involving user-defined names, like branch..* and remote..*. This special logic is missing for submodule-related config variables. Add the appropriate branches to the case statement, making use of the in-tree '.gitmodules' to list relevant submodules. Add corresponding tests in t9902-completion.sh, making sure we complete both first level submodule config variables as well as second level variables involving submodule names. Signed-off-by: Philippe Blain Signed-off-by: Junio C Hamano --- contrib/completion/git-completion.bash | 13 ++++++++++++ t/t9902-completion.sh | 29 ++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 159a4fd8ad..8af9bc3f4e 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2803,6 +2803,19 @@ __git_complete_config_variable_name () __gitcomp_nl_append "pushDefault" "$pfx" "$cur_" "${sfx:- }" return ;; + submodule.*.*) + local pfx="${cur_%.*}." + cur_="${cur_##*.}" + __gitcomp "url update branch fetchRecurseSubmodules ignore active" "$pfx" "$cur_" "$sfx" + return + ;; + submodule.*) + local pfx="${cur_%.*}." + cur_="${cur_#*.}" + __gitcomp_nl "$(__git config -f "$(__git rev-parse --show-toplevel)/.gitmodules" --get-regexp 'submodule.*.path' | awk -F. '{print $2}')" "$pfx" "$cur_" "." + __gitcomp_nl_append $'alternateErrorStrategy\nfetchJobs\nactive\nalternateLocation\nrecurse\npropagateBranches' "$pfx" "$cur_" "${sfx:- }" + return + ;; url.*.*) local pfx="${cur_%.*}." cur_="${cur_##*.}" diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 35eb534fdd..23d0e71324 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -2583,6 +2583,35 @@ test_expect_success 'git config - variable name include' ' EOF ' +test_expect_success 'setup for git config submodule tests' ' + test_create_repo sub && + test_commit -C sub initial && + git submodule add ./sub +' + +test_expect_success 'git config - variable name - submodule' ' + test_completion "git config submodule." <<-\EOF + submodule.active Z + submodule.alternateErrorStrategy Z + submodule.alternateLocation Z + submodule.fetchJobs Z + submodule.propagateBranches Z + submodule.recurse Z + submodule.sub.Z + EOF +' + +test_expect_success 'git config - variable name - submodule names' ' + test_completion "git config submodule.sub." <<-\EOF + submodule.sub.url Z + submodule.sub.update Z + submodule.sub.branch Z + submodule.sub.fetchRecurseSubmodules Z + submodule.sub.ignore Z + submodule.sub.active Z + EOF +' + test_expect_success 'git config - value' ' test_completion "git config color.pager " <<-\EOF false Z From 1e0ee4087e5e5f66882ce60dad522b4f49f79eb0 Mon Sep 17 00:00:00 2001 From: Philippe Blain Date: Sat, 10 Feb 2024 18:32:22 +0000 Subject: [PATCH 3/4] completion: add and use __git_compute_first_level_config_vars_for_section The function __git_complete_config_variable_name in the Bash completion script hardcodes several config variable names. These variables are those in config sections where user-defined names can appear, such as "branch.". These sections are treated first by the case statement, and the two last "catch all" cases are used for other sections, making use of the __git_compute_config_vars and __git_compute_config_sections function, which omit listing any variables containing wildcards or placeholders. Having hardcoded config variables introduces the risk of the completion code becoming out of sync with the actual config variables accepted by Git. To avoid these hardcoded config variables, introduce a new function, __git_compute_first_level_config_vars_for_section, making use of the existing __git_config_vars variable. This function takes as argument a config section name and computes the matching "first level" config variables for that section, i.e. those _not_ containing any placeholder, like 'branch.autoSetupMerge, 'remote.pushDefault', etc. Use this function and the variables it defines in the 'branch.*', 'remote.*' and 'submodule.*' switches of the case statement instead of hardcoding the corresponding config variables. Note that we use indirect expansion to create a variable for each section, instead of using a single associative array indexed by section names, because associative arrays are not supported in Bash 3, on which macOS is stuck for licensing reasons. Use the existing pattern in the completion script of using global variables to cache the list of config variables for each section. The rationale for such caching is explained in eaa4e6ee2a (Speed up bash completion loading, 2009-11-17), and the current approach to using and defining them via 'test -n' is explained in cf0ff02a38 (completion: work around zsh option propagation bug, 2012-02-02). Adjust the name of one of the tests added in the previous commit, reflecting that it now also tests the new function. Signed-off-by: Philippe Blain Signed-off-by: Junio C Hamano --- contrib/completion/git-completion.bash | 24 +++++++++++++++++++++--- t/t9902-completion.sh | 2 +- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 8af9bc3f4e..57a8da7ca1 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2596,6 +2596,15 @@ __git_compute_config_vars () __git_config_vars="$(git help --config-for-completion)" } +__git_compute_first_level_config_vars_for_section () +{ + local section="$1" + __git_compute_config_vars + local this_section="__git_first_level_config_vars_for_section_${section}" + test -n "${!this_section}" || + printf -v "__git_first_level_config_vars_for_section_${section}" %s "$(echo "$__git_config_vars" | grep -E "^${section}\.[a-z]" | awk -F. '{print $2}')" +} + __git_config_sections= __git_compute_config_sections () { @@ -2749,8 +2758,11 @@ __git_complete_config_variable_name () branch.*) local pfx="${cur_%.*}." cur_="${cur_#*.}" + local section="${pfx%.}" __gitcomp_direct "$(__git_heads "$pfx" "$cur_" ".")" - __gitcomp_nl_append $'autoSetupMerge\nautoSetupRebase\n' "$pfx" "$cur_" "${sfx:- }" + __git_compute_first_level_config_vars_for_section "${section}" + local this_section="__git_first_level_config_vars_for_section_${section}" + __gitcomp_nl_append "${!this_section}" "$pfx" "$cur_" "${sfx:- }" return ;; guitool.*.*) @@ -2799,8 +2811,11 @@ __git_complete_config_variable_name () remote.*) local pfx="${cur_%.*}." cur_="${cur_#*.}" + local section="${pfx%.}" __gitcomp_nl "$(__git_remotes)" "$pfx" "$cur_" "." - __gitcomp_nl_append "pushDefault" "$pfx" "$cur_" "${sfx:- }" + __git_compute_first_level_config_vars_for_section "${section}" + local this_section="__git_first_level_config_vars_for_section_${section}" + __gitcomp_nl_append "${!this_section}" "$pfx" "$cur_" "${sfx:- }" return ;; submodule.*.*) @@ -2812,8 +2827,11 @@ __git_complete_config_variable_name () submodule.*) local pfx="${cur_%.*}." cur_="${cur_#*.}" + local section="${pfx%.}" __gitcomp_nl "$(__git config -f "$(__git rev-parse --show-toplevel)/.gitmodules" --get-regexp 'submodule.*.path' | awk -F. '{print $2}')" "$pfx" "$cur_" "." - __gitcomp_nl_append $'alternateErrorStrategy\nfetchJobs\nactive\nalternateLocation\nrecurse\npropagateBranches' "$pfx" "$cur_" "${sfx:- }" + __git_compute_first_level_config_vars_for_section "${section}" + local this_section="__git_first_level_config_vars_for_section_${section}" + __gitcomp_nl_append "${!this_section}" "$pfx" "$cur_" "${sfx:- }" return ;; url.*.*) diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 23d0e71324..8600b9e0dd 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -2589,7 +2589,7 @@ test_expect_success 'setup for git config submodule tests' ' git submodule add ./sub ' -test_expect_success 'git config - variable name - submodule' ' +test_expect_success 'git config - variable name - submodule and __git_compute_first_level_config_vars_for_section' ' test_completion "git config submodule." <<-\EOF submodule.active Z submodule.alternateErrorStrategy Z From 6e32f718ffa2e1358f3c34dafb9ed0dc8e479781 Mon Sep 17 00:00:00 2001 From: Philippe Blain Date: Sat, 10 Feb 2024 18:32:23 +0000 Subject: [PATCH 4/4] completion: add and use __git_compute_second_level_config_vars_for_section In a previous commit we removed some hardcoded config variable names from function __git_complete_config_variable_name in the completion script by introducing a new function, __git_compute_first_level_config_vars_for_section. The remaining hardcoded config variables are "second level" configuration variables, meaning 'branch..upstream', 'remote..url', etc. where is a user-defined name. Making use of the new existing --config flag to 'git help', add a new function, __git_compute_second_level_config_vars_for_section. This function takes as argument a config section name and computes the corresponding second-level config variables, i.e. those that contain a '<' which indicates the start of a placeholder. Note that as in __git_compute_first_level_config_vars_for_section added previsouly, we use indirect expansion instead of associative arrays to stay compatible with Bash 3 on which macOS is stuck for licensing reasons. As explained in the previous commit, we use the existing pattern in the completion script of using global variables to cache the list of variables for each section. Use this new function and the variables it defines in __git_complete_config_variable_name to remove hardcoded config variables, and add a test to verify the new function. Use a single 'case' for all sections with second-level variables names, since the code for each of them is now exactly the same. Adjust the name of a test added in a previous commit to reflect that it now tests the added function. Signed-off-by: Philippe Blain Signed-off-by: Junio C Hamano --- contrib/completion/git-completion.bash | 71 ++++++++------------------ t/t9902-completion.sh | 2 +- 2 files changed, 22 insertions(+), 51 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 57a8da7ca1..87678a5bb3 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2596,6 +2596,13 @@ __git_compute_config_vars () __git_config_vars="$(git help --config-for-completion)" } +__git_config_vars_all= +__git_compute_config_vars_all () +{ + test -n "$__git_config_vars_all" || + __git_config_vars_all="$(git --no-pager help --config)" +} + __git_compute_first_level_config_vars_for_section () { local section="$1" @@ -2605,6 +2612,15 @@ __git_compute_first_level_config_vars_for_section () printf -v "__git_first_level_config_vars_for_section_${section}" %s "$(echo "$__git_config_vars" | grep -E "^${section}\.[a-z]" | awk -F. '{print $2}')" } +__git_compute_second_level_config_vars_for_section () +{ + local section="$1" + __git_compute_config_vars_all + local this_section="__git_second_level_config_vars_for_section_${section}" + test -n "${!this_section}" || + printf -v "__git_second_level_config_vars_for_section_${section}" %s "$(echo "$__git_config_vars_all" | grep -E "^${section}\.<" | awk -F. '{print $3}')" +} + __git_config_sections= __git_compute_config_sections () { @@ -2749,10 +2765,13 @@ __git_complete_config_variable_name () done case "$cur_" in - branch.*.*) + branch.*.*|guitool.*.*|difftool.*.*|man.*.*|mergetool.*.*|remote.*.*|submodule.*.*|url.*.*) local pfx="${cur_%.*}." cur_="${cur_##*.}" - __gitcomp "remote pushRemote merge mergeOptions rebase" "$pfx" "$cur_" "$sfx" + local section="${pfx%.*.}" + __git_compute_second_level_config_vars_for_section "${section}" + local this_section="__git_second_level_config_vars_for_section_${section}" + __gitcomp "${!this_section}" "$pfx" "$cur_" "$sfx" return ;; branch.*) @@ -2765,33 +2784,6 @@ __git_complete_config_variable_name () __gitcomp_nl_append "${!this_section}" "$pfx" "$cur_" "${sfx:- }" return ;; - guitool.*.*) - local pfx="${cur_%.*}." - cur_="${cur_##*.}" - __gitcomp " - argPrompt cmd confirm needsFile noConsole noRescan - prompt revPrompt revUnmerged title - " "$pfx" "$cur_" "$sfx" - return - ;; - difftool.*.*) - local pfx="${cur_%.*}." - cur_="${cur_##*.}" - __gitcomp "cmd path" "$pfx" "$cur_" "$sfx" - return - ;; - man.*.*) - local pfx="${cur_%.*}." - cur_="${cur_##*.}" - __gitcomp "cmd path" "$pfx" "$cur_" "$sfx" - return - ;; - mergetool.*.*) - local pfx="${cur_%.*}." - cur_="${cur_##*.}" - __gitcomp "cmd path trustExitCode" "$pfx" "$cur_" "$sfx" - return - ;; pager.*) local pfx="${cur_%.*}." cur_="${cur_#*.}" @@ -2799,15 +2791,6 @@ __git_complete_config_variable_name () __gitcomp_nl "$__git_all_commands" "$pfx" "$cur_" "${sfx:- }" return ;; - remote.*.*) - local pfx="${cur_%.*}." - cur_="${cur_##*.}" - __gitcomp " - url proxy fetch push mirror skipDefaultUpdate - receivepack uploadpack tagOpt pushurl - " "$pfx" "$cur_" "$sfx" - return - ;; remote.*) local pfx="${cur_%.*}." cur_="${cur_#*.}" @@ -2818,12 +2801,6 @@ __git_complete_config_variable_name () __gitcomp_nl_append "${!this_section}" "$pfx" "$cur_" "${sfx:- }" return ;; - submodule.*.*) - local pfx="${cur_%.*}." - cur_="${cur_##*.}" - __gitcomp "url update branch fetchRecurseSubmodules ignore active" "$pfx" "$cur_" "$sfx" - return - ;; submodule.*) local pfx="${cur_%.*}." cur_="${cur_#*.}" @@ -2834,12 +2811,6 @@ __git_complete_config_variable_name () __gitcomp_nl_append "${!this_section}" "$pfx" "$cur_" "${sfx:- }" return ;; - url.*.*) - local pfx="${cur_%.*}." - cur_="${cur_##*.}" - __gitcomp "insteadOf pushInsteadOf" "$pfx" "$cur_" "$sfx" - return - ;; *.*) __git_compute_config_vars __gitcomp "$__git_config_vars" "" "$cur_" "$sfx" diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 8600b9e0dd..64031a9eff 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -2601,7 +2601,7 @@ test_expect_success 'git config - variable name - submodule and __git_compute_fi EOF ' -test_expect_success 'git config - variable name - submodule names' ' +test_expect_success 'git config - variable name - __git_compute_second_level_config_vars_for_section' ' test_completion "git config submodule.sub." <<-\EOF submodule.sub.url Z submodule.sub.update Z