Subject: [PATCH v3 0/4] completion: remove hardcoded config variable names

From: "Philippe Blain via GitGitGadget" <[email-masked]>
Date: Sat, 10 Feb 2024 18:32:19 +0000

Changes since v2:

 * Moved the addition of the tests to 2/4, and tweaked 3/4 and 4/4 so they
   simply adjust the test names
 * Added a test for user-defined submodule names, as suggested by Patrick
 * Added more details in the commit message of 3/4 around the use of global
   variables as caches
 * Slightly improved commit message wording and fixed typos
 * Added 'local' where suggested
 * Dropped 4/5 which modified 'git help', since it's not needed (thanks
   Patrick!)

Changes since v1:

 * Corrected my email in PATCH 2/5 (sorry for the noise)

v1: This series removes hardcoded config variable names in the
__git_complete_config_variable_name function, partly by adding a new mode to
'git help'. It also adds completion for 'submodule.*' config variables,
which were previously missing.

I think it makes sense to do that in the same series since it's closely
related, and splitting it would result in textual conflicts between both
series if one does not build on top of the other, but I'm open to other
suggestions.

Thanks,

Philippe.

Philippe Blain (4):
  completion: add space after config variable names also in Bash 3
  completion: complete 'submodule.*' config variables
  completion: add and use
    __git_compute_first_level_config_vars_for_section
  completion: add and use
    __git_compute_second_level_config_vars_for_section

 contrib/completion/git-completion.bash | 90 +++++++++++++-------------
 t/t9902-completion.sh                  | 29 +++++++++
 2 files changed, 75 insertions(+), 44 deletions(-)

base-commit: b50a608ba20348cb3dfc16a696816d51780e3f0f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1660%2Fphil-blain%2Fcompletion-submodule-config-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1660/phil-blain/completion-submodule-config-v3
Pull-Request: https://github.com/git/git/pull/1660

Range-diff vs v2:

 1:  837d92a6c27 = 1:  837d92a6c27 completion: add space after config variable names also in Bash 3
 2:  426374ff9b3 ! 2:  6b75582ee35 completion: complete 'submodule.*' config variables
     @@ Commit message
          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 <[email-masked]>
      
       ## contrib/completion/git-completion.bash ##
     @@ contrib/completion/git-completion.bash: __git_complete_config_variable_name ()
       	url.*.*)
       		local pfx="${cur_%.*}."
       		cur_="${cur_##*.}"
     +
     + ## t/t9902-completion.sh ##
     +@@ t/t9902-completion.sh: 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
 3:  838aabf2858 ! 3:  fb210325394 completion: add and use __git_compute_first_level_config_vars_for_section
     @@ Commit message
      
          The function __git_complete_config_variable_name in the Bash completion
          script hardcodes several config variable names. These variables are
     -    those in config section where user-defined names can appear, such as
     +    those in config sections where user-defined names can appear, such as
          "branch.<name>". 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
     @@ Commit message
          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
     -    instead of associative arrays because those are not supported in Bash 3,
     -    on which macOS is stuck for licensing reasons.
     +    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.
      
     -    Add a test to make sure the new function works correctly by verfying it
     -    lists all 'submodule' config variables. This has the downside that this
     -    test must be updated when new 'submodule' configuration are added, but
     -    this should be a small burden since it happens infrequently.
     +    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 <[email-masked]>
      
     @@ contrib/completion/git-completion.bash: __git_compute_config_vars ()
       
      +__git_compute_first_level_config_vars_for_section ()
      +{
     -+	section="$1"
     ++	local section="$1"
      +	__git_compute_config_vars
      +	local this_section="__git_first_level_config_vars_for_section_${section}"
      +	test -n "${!this_section}" ||
     @@ contrib/completion/git-completion.bash: __git_complete_config_variable_name ()
       	url.*.*)
      
       ## t/t9902-completion.sh ##
     -@@ t/t9902-completion.sh: test_expect_success 'git config - variable name include' '
     - 	EOF
     +@@ t/t9902-completion.sh: test_expect_success 'setup for git config submodule tests' '
     + 	git submodule add ./sub
       '
       
     -+test_expect_success 'git config - variable name - __git_compute_first_level_config_vars_for_section' '
     -+	test_completion "git config submodule." <<-\EOF
     -+	submodule.active Z
     -+	submodule.alternateErrorStrategy Z
     -+	submodule.alternateLocation Z
     -+	submodule.fetchJobs Z
     -+	submodule.propagateBranches Z
     -+	submodule.recurse Z
     -+	EOF
     -+'
     -+
     - test_expect_success 'git config - value' '
     - 	test_completion "git config color.pager " <<-\EOF
     - 	false Z
     +-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
 4:  d442a039b27 < -:  ----------- builtin/help: add --config-all-for-completion
 5:  a2e792c911e ! 4:  69fc02bb6b4 completion: add an use __git_compute_second_level_config_vars_for_section
     @@ Metadata
      Author: Philippe Blain <[email-masked]>
      
       ## Commit message ##
     -    completion: add an use __git_compute_second_level_config_vars_for_section
     +    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
     @@ Commit message
          configuration variables, meaning 'branch.<name>.upstream',
          'remote.<name>.url', etc. where <name> is a user-defined name.
      
     -    Making use of the new --config-all-for-completion flag to 'git help'
     -    introduced in the previous commit, 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
     +    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 <[email-masked]>
      
       ## contrib/completion/git-completion.bash ##
     @@ contrib/completion/git-completion.bash: __git_compute_config_vars ()
      +__git_compute_config_vars_all ()
      +{
      +	test -n "$__git_config_vars_all" ||
     -+	__git_config_vars_all="$(git help --config-all-for-completion)"
     ++	__git_config_vars_all="$(git --no-pager help --config)"
      +}
      +
       __git_compute_first_level_config_vars_for_section ()
       {
     - 	section="$1"
     + 	local section="$1"
      @@ contrib/completion/git-completion.bash: __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 ()
      +{
     -+	section="$1"
     ++	local section="$1"
      +	__git_compute_config_vars_all
      +	local this_section="__git_second_level_config_vars_for_section_${section}"
      +	test -n "${!this_section}" ||
     @@ contrib/completion/git-completion.bash: __git_complete_config_variable_name ()
       		__gitcomp "$__git_config_vars" "" "$cur_" "$sfx"
      
       ## t/t9902-completion.sh ##
     -@@ t/t9902-completion.sh: test_expect_success 'git config - variable name - __git_compute_first_level_conf
     - 	submodule.recurse Z
     +@@ t/t9902-completion.sh: test_expect_success 'git config - variable name - submodule and __git_compute_fi
       	EOF
       '
     -+test_expect_success 'git config - variable name - __git_compute_second_level_config_vars_for_section' '
     -+	test_completion "git config branch.main." <<-\EOF
     -+	branch.main.description Z
     -+	branch.main.remote Z
     -+	branch.main.pushRemote Z
     -+	branch.main.merge Z
     -+	branch.main.mergeOptions Z
     -+	branch.main.rebase Z
     -+	EOF
     -+'
       
     - test_expect_success 'git config - value' '
     - 	test_completion "git config color.pager " <<-\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

-- 
gitgitgadget