{"thread":{"id":"39289","subject":"[PATCH 2/2] completion: simplify query for config variables","startedAt":"2015-05-10T12:50:17Z","lastAt":"2015-05-12T22:14:59Z","messageCount":4,"participants":["SZEDER Gábor","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"260926","messageId":"1431262218-7304-1-git-send-email-szeder@ira.uka.de","threadId":"39289","inReplyTo":null,"subject":"[PATCH 1/2] completion: add a helper function to get config variables","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-05-10T12:50:17Z","receivedAt":"2015-05-10T12:50:17Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Currently there are a few completion functions that perform similar 'git\nconfig' queries and filtering to get config variable names: the completion\nof pretty aliases, aliases, and remote groups for 'git remote update'.\n\nUnify those 'git config' queries in a helper function to eliminate code\nduplication.\n\nThough the helper functions to get pretty aliases and alieses are reduced\nto mere one-liner wrappers around the newly added function, keep these\nhelpers still, because users' completion functions out there might depend\non them.  And they keep their callers a tad easier to read, too.\n\nAdd tests for the pretty alias and alias helper to show that they work\nas before; not for the remote groups query, though, because that's not\nextracted into a helper function and it's not worth the effort to do so\nfor a sole callsite.\n\nSigned-off-by: SZEDER Gábor <szeder@ira.uka.de>\n---\n contrib/completion/git-completion.bash | 34 ++++++++++++++--------------------\n t/t9902-completion.sh                  | 22 ++++++++++++++++++++++\n 2 files changed, 36 insertions(+), 20 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 5944c824ab..6973620857 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -740,30 +740,29 @@ __git_compute_porcelain_commands ()\n \t__git_porcelain_commands=$(__git_list_porcelain_commands)\n }\n \n-__git_pretty_aliases ()\n+# Lists all set config variables starting with the given section prefix,\n+# with the prefix removed.\n+__git_get_config_variables ()\n {\n-\tlocal i IFS=$'\\n'\n-\tfor i in $(git --git-dir=\"$(__gitdir)\" config --get-regexp \"pretty\\..*\" 2>/dev/null); do\n+\tlocal section=\"$1\" i IFS=$'\\n'\n+\tfor i in $(git --git-dir=\"$(__gitdir)\" config --get-regexp \"$section\\..*\" 2>/dev/null); do\n \t\tcase \"$i\" in\n-\t\tpretty.*)\n-\t\t\ti=\"${i#pretty.}\"\n+\t\t$section.*)\n+\t\t\ti=\"${i#$section.}\"\n \t\t\techo \"${i/ */}\"\n \t\t\t;;\n \t\tesac\n \tdone\n }\n \n+__git_pretty_aliases ()\n+{\n+\t__git_get_config_variables \"pretty\"\n+}\n+\n __git_aliases ()\n {\n-\tlocal i IFS=$'\\n'\n-\tfor i in $(git --git-dir=\"$(__gitdir)\" config --get-regexp \"alias\\..*\" 2>/dev/null); do\n-\t\tcase \"$i\" in\n-\t\talias.*)\n-\t\t\ti=\"${i#alias.}\"\n-\t\t\techo \"${i/ */}\"\n-\t\t\t;;\n-\t\tesac\n-\tdone\n+\t__git_get_config_variables \"alias\"\n }\n \n # __git_aliased_command requires 1 argument\n@@ -2260,12 +2259,7 @@ _git_remote ()\n \t\t__git_complete_remote_or_refspec\n \t\t;;\n \tupdate)\n-\t\tlocal i c='' IFS=$'\\n'\n-\t\tfor i in $(git --git-dir=\"$(__gitdir)\" config --get-regexp \"remotes\\..*\" 2>/dev/null); do\n-\t\t\ti=\"${i#remotes.}\"\n-\t\t\tc=\"$c ${i/ */}\"\n-\t\tdone\n-\t\t__gitcomp \"$c\"\n+\t\t__gitcomp \"$(__git_get_config_variables \"remotes\")\"\n \t\t;;\n \t*)\n \t\t;;\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 4a14a5892e..07f2478c9b 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -370,6 +370,28 @@ test_expect_success '__git_remotes - list remotes from $GIT_DIR/remotes and from\n \ttest_cmp expect actual\n '\n \n+test_expect_success '__git_pretty_aliases' '\n+\tcat >expect <<-EOF &&\n+\tauthor\n+\thash\n+\tEOF\n+\ttest_config pretty.author \"%an %ae\" &&\n+\ttest_config pretty.hash %H &&\n+\t__git_pretty_aliases >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '__git_aliases' '\n+\tcat >expect <<-EOF &&\n+\tci\n+\tco\n+\tEOF\n+\ttest_config alias.ci commit &&\n+\ttest_config alias.co checkout &&\n+\t__git_aliases >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'basic' '\n \trun_completion \"git \" &&\n \t# built-in\n-- \n1.9.5.msysgit.0\n"},{"id":"260925","messageId":"1431262218-7304-2-git-send-email-szeder@ira.uka.de","threadId":"39289","inReplyTo":"1431262218-7304-1-git-send-email-szeder@ira.uka.de","subject":"[PATCH 2/2] completion: simplify query for config variables","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-05-10T12:50:18Z","receivedAt":"2015-05-10T12:50:18Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"To get the name of all config variables in a given section we perform a\n'git config --get-regex' query for all config variables containing the\nname of that section, and then filter its output through a case statement\nto throw away those that though contain but don't start with the given\nsection.\n\nModify the regex to match only at the beginning, so the case statement\nbecomes unnecessary and we can get rid of it.  Add a test to check that a\nmatch in the middle doesn't fool us.\n\nSigned-off-by: SZEDER Gábor <szeder@ira.uka.de>\n---\n contrib/completion/git-completion.bash | 10 +++-------\n t/t9902-completion.sh                  | 12 ++++++++++++\n 2 files changed, 15 insertions(+), 7 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 6973620857..63f960b33d 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -745,13 +745,9 @@ __git_compute_porcelain_commands ()\n __git_get_config_variables ()\n {\n \tlocal section=\"$1\" i IFS=$'\\n'\n-\tfor i in $(git --git-dir=\"$(__gitdir)\" config --get-regexp \"$section\\..*\" 2>/dev/null); do\n-\t\tcase \"$i\" in\n-\t\t$section.*)\n-\t\t\ti=\"${i#$section.}\"\n-\t\t\techo \"${i/ */}\"\n-\t\t\t;;\n-\t\tesac\n+\tfor i in $(git --git-dir=\"$(__gitdir)\" config --get-regexp \"^$section\\..*\" 2>/dev/null); do\n+\t\ti=\"${i#$section.}\"\n+\t\techo \"${i/ */}\"\n \tdone\n }\n \ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 07f2478c9b..2ba62fbc17 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -370,6 +370,18 @@ test_expect_success '__git_remotes - list remotes from $GIT_DIR/remotes and from\n \ttest_cmp expect actual\n '\n \n+test_expect_success '__git_get_config_variables' '\n+\tcat >expect <<-EOF &&\n+\tname-1\n+\tname-2\n+\tEOF\n+\ttest_config interesting.name-1 good &&\n+\ttest_config interesting.name-2 good &&\n+\ttest_config subsection.interesting.name-3 bad &&\n+\t__git_get_config_variables interesting >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '__git_pretty_aliases' '\n \tcat >expect <<-EOF &&\n \tauthor\n-- \n1.9.5.msysgit.0\n"},{"id":"260927","messageId":"20150510151652.Horde.FOfYcnwZZkdBJUYkl0pDQg1@webmail.informatik.kit.edu","threadId":"39289","inReplyTo":"1431262218-7304-1-git-send-email-szeder@ira.uka.de","subject":"Re: [PATCH 1/2] completion: add a helper function to get config variables","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-05-10T13:16:52Z","receivedAt":"2015-05-10T13:16:52Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nQuoting SZEDER Gábor <szeder@ira.uka.de>:\n\n> Currently there are a few completion functions that perform similar 'git\n> config' queries and filtering to get config variable names: the completion\n> of pretty aliases, aliases, and remote groups for 'git remote update'.\n>\n> Unify those 'git config' queries in a helper function to eliminate code\n> duplication.\n>\n> Though the helper functions to get pretty aliases and alieses are reduced\n> to mere one-liner wrappers around the newly added function, keep these\n> helpers still, because users' completion functions out there might depend\n> on them.  And they keep their callers a tad easier to read, too.\n>\n> Add tests for the pretty alias and alias helper to show that they work\n> as before; not for the remote groups query, though, because that's not\n> extracted into a helper function and it's not worth the effort to do so\n> for a sole callsite.\n>\n> Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>\n> ---\n> @@ -2260,12 +2259,7 @@ _git_remote ()\n>  \t\t__git_complete_remote_or_refspec\n>  \t\t;;\n>  \tupdate)\n> -\t\tlocal i c='' IFS=$'\\n'\n> -\t\tfor i in $(git --git-dir=\"$(__gitdir)\" config --get-regexp  \n> \"remotes\\..*\" 2>/dev/null); do\n> -\t\t\ti=\"${i#remotes.}\"\n> -\t\t\tc=\"$c ${i/ */}\"\n> -\t\tdone\n> -\t\t__gitcomp \"$c\"\n> +\t\t__gitcomp \"$(__git_get_config_variables \"remotes\")\"\n\nI just noticed that this query for remote groups does lack that  \nfiltering case statement eliminated in the next patch and the regex is  \nnot limited to the beginning of the config variable name.\nSo, let's suppose we have a branch named 'remotes', for which we add a  \nbranch description (and no remote groups, for symplicity's sake):\n\n   $ git config branch.remotes.description uh-oh\n   $ git remote update <TAB>\n\nThis will complete to:\n\n   $ git remote update branch.remotes.description\n\nand that's obviously bad.\n\nThis series fixes this.\n\nBest,\nGábor\n"},{"id":"261095","messageId":"xmqqa8x9e93w.fsf@gitster.dls.corp.google.com","threadId":"39289","inReplyTo":"1431262218-7304-1-git-send-email-szeder@ira.uka.de","subject":"Re: [PATCH 1/2] completion: add a helper function to get config variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-12T22:14:59Z","receivedAt":"2015-05-12T22:14:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor <szeder@ira.uka.de> writes:\n\n> +\tfor i in $(git --git-dir=\"$(__gitdir)\" config --get-regexp \"$section\\..*\" 2>/dev/null); do\n>  \t\tcase \"$i\" in\n> -\t\tpretty.*)\n> -\t\t\ti=\"${i#pretty.}\"\n> +\t\t$section.*)\n> +\t\t\ti=\"${i#$section.}\"\n\nThe case arm treats $section as a shell glob, --get-regexp treats it\nas a regex fragment and then the shell expansion uses it as a\nliteral.\n\nThe current set of callers give simple single-token like pretty,\nremote, etc. to it so this is safe, but you may want to give a\ncomment to the function to help future generation, perhaps?\n\nOther than that, looks quite straight-forward.\n\nThanks.\n"}]}