From 08a12175d8b725d80efac41328780b57c4cfa6b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 13 Aug 2019 14:26:42 +0200 Subject: [PATCH 01/11] completion: fix a typo in a comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- 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 e087c4bf0085ad..cd9d8e1940a88b 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -524,7 +524,7 @@ __git_index_files () # Even when a directory name itself does not contain # any special characters, it will still be quoted if # any of its (stripped) trailing path components do. - # Because of this we may have seen the same direcory + # Because of this we may have seen the same directory # both quoted and unquoted. if (p in paths) # We have seen the same directory unquoted, From 840d7e5b3ca97f831b5af804a135a9cc79d9b919 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 13 Aug 2019 14:26:43 +0200 Subject: [PATCH 02/11] completion: complete more values of more 'color.*' configuration variables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Most 'color.*' configuration variables, with the sole exception of 'color.pager', accept the same set of values, but our completion script recognizes only about half of them. We could explicitly add all those missing variables, but let's try to reduce future maintenance burden, and use the catch-all 'color.*' pattern instead, so this list won't get out of sync when a similar new configuration variable accepting the same values is introduced [1]. Furthermore, their documentation explicitly mentions that they all accept the standard boolean values 'false' and 'true' as well, so list these, too, among the possible values. [1] OTOH, there will be a maintenance burden if ever a new 'color.something' is introduced which doesn't accept the same set of values. We'll see which one happens first... Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- contrib/completion/git-completion.bash | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index cd9d8e1940a88b..c59347daeeefdf 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2277,11 +2277,6 @@ _git_config () __gitcomp "$__git_merge_strategies" return ;; - color.branch|color.diff|color.interactive|\ - color.showbranch|color.status|color.ui) - __gitcomp "always never auto" - return - ;; color.pager) __gitcomp "false true" return @@ -2293,6 +2288,10 @@ _git_config () " return ;; + color.*) + __gitcomp "false true always never auto" + return + ;; diff.submodule) __gitcomp "$__git_diff_submodule_formats" return From 7a09a8f093eef940eef7d012907c051974ada254 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 13 Aug 2019 14:26:44 +0200 Subject: [PATCH 03/11] completion: add tests for 'git config' completion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The next patches will change/refactor the way we complete configuration variable names and values, so add a few tests to cover the basics, namely the completion of matching configuration sections, full variable names, and their values. Note that the test checking the completion of configuration sections is currently failing, though it's not a sign of an actual bug. If a section contains multiple variables, then that section is currently repeated as many times as the number of variables in there. This is not a correctness issue in practice, because Bash's completion facilities remove all repetitions anyway. Consequently, we could list all those repeated sections in the expected output of this test as well, but then it would have to be updated whenever a new configuration variable is added to those sections. Instead, list each matching configuration section only once, mark the test as failing for now, and the next patch will update the completion script to avoid those repetitions. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- t/t9902-completion.sh | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 75512c340366f3..e15be1164d1945 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -1698,6 +1698,27 @@ do ' done +test_expect_failure 'git config - section' ' + test_completion "git config br" <<-\EOF + branch.Z + browser.Z + EOF +' + +test_expect_success 'git config - variable name' ' + test_completion "git config log.d" <<-\EOF + log.date Z + log.decorate Z + EOF +' + +test_expect_success 'git config - value' ' + test_completion "git config color.pager " <<-\EOF + false Z + true Z + EOF +' + test_expect_success 'sourcing the completion script clears cached commands' ' __git_compute_all_commands && verbose test -n "$__git_all_commands" && From d9438873c4dd80057a86f4b9a082db5ec75275cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 13 Aug 2019 14:26:45 +0200 Subject: [PATCH 04/11] completion: deduplicate configuration sections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The number of configuration variables listed by the completion script grew quite when we started to auto-generate it from the documentation [1], so we now complete them in two steps: first we list only the section names, then the rest [2]. To get the section names we simply strip everything following the first dot in each variable name, resulting in a lot of repeated section names, because most sections contain more than one configuration variable. This is not a correctness issue in practice, because Bash's completion facilities remove all repetitions anyway, but these repetitions make testing a bit harder. Replace the small 'sed' script removing subsections and variable names with an 'awk' script that does the same, and in addition removes any repeated configuration sections as well (by first creating and filling an associative array indexed by all encountered configuration sections, and then iterating over this array and printing the indices, i.e. the unique section names). This change makes the failing 'git config - section' test in 't9902-completion.sh' pass. Note that this changes the order of section names in the output, and makes it downright undeterministic, but this is not an issue, because Bash sorts them before presenting them to the user, and our completion tests sort them as well before comparing with the expected output. Yeah, it would be simpler and shorter to just append '| sort -u' to that command, but that would incur the overhead of one more external process and pipeline stage every time a user completes configuration sections. [1] e17ca92637 (completion: drop the hard coded list of config vars, 2018-05-26) [2] f22f682695 (completion: complete general config vars in two steps, 2018-05-27) Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- contrib/completion/git-completion.bash | 10 +++++++++- t/t9902-completion.sh | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index c59347daeeefdf..f89324d84faf09 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2398,7 +2398,15 @@ _git_config () ;; *) __git_compute_config_vars - __gitcomp "$(echo "$__git_config_vars" | sed 's/\.[^ ]*/./g')" + __gitcomp "$(echo "$__git_config_vars" | + awk -F . '{ + sections[$1] = 1 + } + END { + for (s in sections) + print s "." + } + ')" esac } diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index e15be1164d1945..008fba7c896388 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -1698,7 +1698,7 @@ do ' done -test_expect_failure 'git config - section' ' +test_expect_success 'git config - section' ' test_completion "git config br" <<-\EOF branch.Z browser.Z From 2675ea1cc0f5d542d9bde4e8a458ac726bf30f63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 13 Aug 2019 14:26:46 +0200 Subject: [PATCH 05/11] completion: use 'sort -u' to deduplicate config variable names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The completion script runs the classic '| sort | uniq' pipeline to deduplicate the output of 'git help --config-for-completion'. 'sort -u' does the same, but uses one less external process and pipeline stage. Not a bit win, as it's only run once as the list of supported configuration variables is initialized, but at least it sets a better example for others to follow. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- 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 f89324d84faf09..b51cb31ea1112d 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2225,7 +2225,7 @@ __git_config_vars= __git_compute_config_vars () { test -n "$__git_config_vars" || - __git_config_vars="$(git help --config-for-completion | sort | uniq)" + __git_config_vars="$(git help --config-for-completion | sort -u)" } _git_config () From d9ee1e061783be43d10dee996b85bac0f9223ec4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 13 Aug 2019 14:26:47 +0200 Subject: [PATCH 06/11] completion: simplify inner 'case' pattern in __gitcomp() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The second '*' in the '--*=*' pattern of the inner 'case' statement of the __gitcomp() helper function never matches anything, so let's use '--*=' instead. The purpose of that inner case statement is to decide when to append a trailing space to the listed options and when not. When an option requires a stuck argument, i.e. '--option=', then the trailing space should not be added, so the user can continue typing the required argument right away. That '--*=*' pattern is supposed to match these options, but for this purpose that second '*' is unnecessary, a '--*=' pattern works just as well. That second '*' would only make a difference in case of a possible completion word like '--option=value', but our completion script never passes such a word to __gitcomp(), because the '--option=' and its 'value' must be completed separately. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- 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 b51cb31ea1112d..fc437bf3eb0e3f 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -340,7 +340,7 @@ __gitcomp () c="$c${4-}" if [[ $c == "$cur_"* ]]; then case $c in - --*=*|*.) ;; + --*=|*.) ;; *) c="$c " ;; esac COMPREPLY[i++]="${2-}$c" @@ -360,7 +360,7 @@ __gitcomp () c="$c${4-}" if [[ $c == "$cur_"* ]]; then case $c in - --*=*|*.) ;; + --*=|*.) ;; *) c="$c " ;; esac COMPREPLY[i++]="${2-}$c" From 42d0efec592ecd6bc8858ffb5d64fb0bbca827d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 13 Aug 2019 14:26:48 +0200 Subject: [PATCH 07/11] completion: split _git_config() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _git_config() contains two enormous case statements, one to complete configuration sections and variable names, and the other to complete their values. Split these out into two separate helper functions, so in the next patches we can use them to implement completion for 'git -c '. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- contrib/completion/git-completion.bash | 39 ++++++++++++++++++++------ 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index fc437bf3eb0e3f..3e9c5b6b71a052 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2228,7 +2228,8 @@ __git_compute_config_vars () __git_config_vars="$(git help --config-for-completion | sort -u)" } -_git_config () +# Completes possible values of various configuration variables. +__git_complete_config_variable_value () { local varname @@ -2320,19 +2321,16 @@ _git_config () __gitcomp "7bit 8bit quoted-printable base64" return ;; - --get|--get-all|--unset|--unset-all) - __gitcomp_nl "$(__git_config_get_set_variables)" - return - ;; *.*) return ;; esac +} + +# Completes configuration sections, subsections, variable names. +__git_complete_config_variable_name () +{ case "$cur" in - --*) - __gitcomp_builtin config - return - ;; branch.*.*) local pfx="${cur%.*}." cur_="${cur##*.}" __gitcomp "remote pushRemote merge mergeOptions rebase" "$pfx" "$cur_" @@ -2407,6 +2405,29 @@ _git_config () print s "." } ')" + ;; + esac +} + +_git_config () +{ + case "$prev" in + --get|--get-all|--unset|--unset-all) + __gitcomp_nl "$(__git_config_get_set_variables)" + return + ;; + *.*) + __git_complete_config_variable_value + return + ;; + esac + case "$cur" in + --*) + __gitcomp_builtin config + ;; + *) + __git_complete_config_variable_name + ;; esac } From e1e00089da9f616d23f0ca3bb183258e9013c469 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 13 Aug 2019 14:26:49 +0200 Subject: [PATCH 08/11] completion: complete configuration sections and variable names for 'git -c' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 'git config' expects a configuration variable's name and value in separate arguments, so we let the __gitcomp() helper append a space character to each variable name by default, like we do for most other things (--options, refs, paths, etc.). 'git -c', however, expects them in a single option joined by a '=' character, i.e. 'section.name=value', so we should append a '=' character to each fully completed variable name, but no space, so the user can continue typing the value right away. Add an option to the __git_complete_config_variable_name() function to allow callers to specify an alternate suffix to add, and use it to append that '=' character to configuration variables. Update the __gitcomp() helper function to not append a trailing space to any completion words ending with a '=', not just to those option with a stuck argument. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- contrib/completion/git-completion.bash | 59 ++++++++++++++++++++------ t/t9902-completion.sh | 14 ++++++ 2 files changed, 60 insertions(+), 13 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 3e9c5b6b71a052..367b1c50f450e0 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -360,7 +360,7 @@ __gitcomp () c="$c${4-}" if [[ $c == "$cur_"* ]]; then case $c in - --*=|*.) ;; + *=|*.) ;; *) c="$c " ;; esac COMPREPLY[i++]="${2-}$c" @@ -2328,18 +2328,33 @@ __git_complete_config_variable_value () } # Completes configuration sections, subsections, variable names. +# +# Usage: __git_complete_config_variable_name [