{"thread":{"id":"32132","subject":"[RFC/PATCH 3/5] completion: trivial test improvement","startedAt":"2012-11-17T01:38:13Z","lastAt":"2012-11-18T00:53:50Z","messageCount":15,"participants":["Felipe Contreras","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"203390","messageId":"1353116298-11798-1-git-send-email-felipe.contreras@gmail.com","threadId":"32132","inReplyTo":null,"subject":"[RFC/PATCH 0/5] completion: compgen/compadd cleanups","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-17T01:38:13Z","receivedAt":"2012-11-17T01:38:13Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hi,\n\nThese were hinted by SZEDER, so I went ahead and implemented the changes trying\nto keep in mind the new zsh completion werapper. The resulting code should be\nmore efficient, and a known breakage is fixed.\n\nThe first two patches come from another patch series for convenience.\n\nTake them with a pinch of salt.\n\nFelipe Contreras (5):\n completion: get rid of empty COMPREPLY assignments\n completion: add new __gitcompadd helper\n completion: trivial test improvement\n completion: get rid of compgen\n completion: refactor __gitcomp_1\n\n contrib/completion/git-completion.bash | 68 +++++++++++++---------------------\n t/t9902-completion.sh | 3 +-\n 2 files changed, 27 insertions(+), 44 deletions(-)\n\n-- \n1.8.0\n"},{"id":"203388","messageId":"1353116298-11798-2-git-send-email-felipe.contreras@gmail.com","threadId":"32132","inReplyTo":"1353116298-11798-1-git-send-email-felipe.contreras@gmail.com","subject":"[RFC/PATCH 1/5] completion: get rid of empty COMPREPLY assignments","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-17T01:38:14Z","receivedAt":"2012-11-17T01:38:14Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"There's no functional reason for those, the only purpose they are\nsupposed to serve is to say \"we don't provide any words here\", but even\nfor that it's not used consitently.\n\nSigned-off-by: Felipe Contreras \n---\n contrib/completion/git-completion.bash | 28 ----------------------------\n 1 file changed, 28 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex be800e0..7bdd6a8 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -238,7 +238,6 @@ __gitcomp ()\n \n \tcase \"$cur_\" in\n \t--*=)\n-\t\tCOMPREPLY=()\n \t\t;;\n \t*)\n \t\tlocal IFS=$'\\n'\n@@ -486,7 +485,6 @@ __git_complete_remote_or_refspec ()\n \t\t\tcase \"$cmd\" in\n \t\t\tpush) no_complete_refspec=1 ;;\n \t\t\tfetch)\n-\t\t\t\tCOMPREPLY=()\n \t\t\t\treturn\n \t\t\t\t;;\n \t\t\t*) ;;\n@@ -502,7 +500,6 @@ __git_complete_remote_or_refspec ()\n \t\treturn\n \tfi\n \tif [ $no_complete_refspec = 1 ]; then\n-\t\tCOMPREPLY=()\n \t\treturn\n \tfi\n \t[ \"$remote\" = \".\" ] && remote=\n@@ -776,7 +773,6 @@ _git_am ()\n \t\t\t\"\n \t\treturn\n \tesac\n-\tCOMPREPLY=()\n }\n \n _git_apply ()\n@@ -796,7 +792,6 @@ _git_apply ()\n \t\t\t\"\n \t\treturn\n \tesac\n-\tCOMPREPLY=()\n }\n \n _git_add ()\n@@ -811,7 +806,6 @@ _git_add ()\n \t\t\t\"\n \t\treturn\n \tesac\n-\tCOMPREPLY=()\n }\n \n _git_archive ()\n@@ -856,7 +850,6 @@ _git_bisect ()\n \t\t__gitcomp_nl \"$(__git_refs)\"\n \t\t;;\n \t*)\n-\t\tCOMPREPLY=()\n \t\t;;\n \tesac\n }\n@@ -969,7 +962,6 @@ _git_clean ()\n \t\treturn\n \t\t;;\n \tesac\n-\tCOMPREPLY=()\n }\n \n _git_clone ()\n@@ -993,7 +985,6 @@ _git_clone ()\n \t\treturn\n \t\t;;\n \tesac\n-\tCOMPREPLY=()\n }\n \n _git_commit ()\n@@ -1027,7 +1018,6 @@ _git_commit ()\n \t\t\t\"\n \t\treturn\n \tesac\n-\tCOMPREPLY=()\n }\n \n _git_describe ()\n@@ -1158,7 +1148,6 @@ _git_fsck ()\n \t\treturn\n \t\t;;\n \tesac\n-\tCOMPREPLY=()\n }\n \n _git_gc ()\n@@ -1169,7 +1158,6 @@ _git_gc ()\n \t\treturn\n \t\t;;\n \tesac\n-\tCOMPREPLY=()\n }\n \n _git_gitk ()\n@@ -1246,7 +1234,6 @@ _git_init ()\n \t\treturn\n \t\t;;\n \tesac\n-\tCOMPREPLY=()\n }\n \n _git_ls_files ()\n@@ -1265,7 +1252,6 @@ _git_ls_files ()\n \t\treturn\n \t\t;;\n \tesac\n-\tCOMPREPLY=()\n }\n \n _git_ls_remote ()\n@@ -1381,7 +1367,6 @@ _git_mergetool ()\n \t\treturn\n \t\t;;\n \tesac\n-\tCOMPREPLY=()\n }\n \n _git_merge_base ()\n@@ -1397,7 +1382,6 @@ _git_mv ()\n \t\treturn\n \t\t;;\n \tesac\n-\tCOMPREPLY=()\n }\n \n _git_name_rev ()\n@@ -1567,7 +1551,6 @@ _git_send_email ()\n \t\treturn\n \t\t;;\n \tesac\n-\tCOMPREPLY=()\n }\n \n _git_stage ()\n@@ -1680,7 +1663,6 @@ _git_config ()\n \t\treturn\n \t\t;;\n \t*.*)\n-\t\tCOMPREPLY=()\n \t\treturn\n \t\t;;\n \tesac\n@@ -2060,7 +2042,6 @@ _git_remote ()\n \t\t__gitcomp \"$c\"\n \t\t;;\n \t*)\n-\t\tCOMPREPLY=()\n \t\t;;\n \tesac\n }\n@@ -2104,7 +2085,6 @@ _git_rm ()\n \t\treturn\n \t\t;;\n \tesac\n-\tCOMPREPLY=()\n }\n \n _git_shortlog ()\n@@ -2173,8 +2153,6 @@ _git_stash ()\n \t\t*)\n \t\t\tif [ -z \"$(__git_find_on_cmdline \"$save_opts\")\" ]; then\n \t\t\t\t__gitcomp \"$subcommands\"\n-\t\t\telse\n-\t\t\t\tCOMPREPLY=()\n \t\t\tfi\n \t\t\t;;\n \t\tesac\n@@ -2187,14 +2165,12 @@ _git_stash ()\n \t\t\t__gitcomp \"--index --quiet\"\n \t\t\t;;\n \t\tshow,--*|drop,--*|branch,--*)\n-\t\t\tCOMPREPLY=()\n \t\t\t;;\n \t\tshow,*|apply,*|drop,*|pop,*|branch,*)\n \t\t\t__gitcomp_nl \"$(git --git-dir=\"$(__gitdir)\" stash list \\\n \t\t\t\t\t| sed -n -e 's/:.*//p')\"\n \t\t\t;;\n \t\t*)\n-\t\t\tCOMPREPLY=()\n \t\t\t;;\n \t\tesac\n \tfi\n@@ -2311,7 +2287,6 @@ _git_svn ()\n \t\t\t__gitcomp \"--revision= --parent\"\n \t\t\t;;\n \t\t*)\n-\t\t\tCOMPREPLY=()\n \t\t\t;;\n \t\tesac\n \tfi\n@@ -2336,13 +2311,10 @@ _git_tag ()\n \n \tcase \"$prev\" in\n \t-m|-F)\n-\t\tCOMPREPLY=()\n \t\t;;\n \t-*|tag)\n \t\tif [ $f = 1 ]; then\n \t\t\t__gitcomp_nl \"$(__git_tags)\"\n-\t\telse\n-\t\t\tCOMPREPLY=()\n \t\tfi\n \t\t;;\n \t*)\n-- \n1.8.0\n"},{"id":"203389","messageId":"1353116298-11798-3-git-send-email-felipe.contreras@gmail.com","threadId":"32132","inReplyTo":"1353116298-11798-1-git-send-email-felipe.contreras@gmail.com","subject":"[RFC/PATCH 2/5] completion: add new __gitcompadd helper","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-17T01:38:15Z","receivedAt":"2012-11-17T01:38:15Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The idea is to never touch the COMPREPLY variable directly.\n\nThis allows other completion systems override __gitcompadd, and do\nsomething different instead.\n\nAlso, this allows the simplification of the completion tests (separate\npatch).\n\nThere should be no functional changes.\n\nSigned-off-by: Felipe Contreras \n---\n contrib/completion/git-completion.bash | 13 ++++++++-----\n 1 file changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 7bdd6a8..975ae13 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -225,6 +225,11 @@ _get_comp_words_by_ref ()\n fi\n fi\n \n+__gitcompadd ()\n+{\n+\tCOMPREPLY=($(compgen -W \"$1\" -P \"$2\" -S \"$4\" -- \"$3\"))\n+}\n+\n # Generates completion reply with compgen, appending a space to possible\n # completion words, if necessary.\n # It accepts 1 to 4 arguments:\n@@ -241,9 +246,7 @@ __gitcomp ()\n \t\t;;\n \t*)\n \t\tlocal IFS=$'\\n'\n-\t\tCOMPREPLY=($(compgen -P \"${2-}\" \\\n-\t\t\t-W \"$(__gitcomp_1 \"${1-}\" \"${4-}\")\" \\\n-\t\t\t-- \"$cur_\"))\n+\t\t__gitcompadd \"$(__gitcomp_1 \"${1-}\" \"${4-}\")\" \"${2-}\" \"$cur_\" \"\"\n \t\t;;\n \tesac\n }\n@@ -260,7 +263,7 @@ __gitcomp ()\n __gitcomp_nl ()\n {\n \tlocal IFS=$'\\n'\n-\tCOMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$1\" -- \"${3-$cur}\"))\n+\t__gitcompadd \"$1\" \"${2-}\" \"${3-$cur}\" \"${4- }\"\n }\n \n __git_heads ()\n@@ -1603,7 +1606,7 @@ _git_config ()\n \t\tlocal remote=\"${prev#remote.}\"\n \t\tremote=\"${remote%.fetch}\"\n \t\tif [ -z \"$cur\" ]; then\n-\t\t\tCOMPREPLY=(\"refs/heads/\")\n+\t\t\t__gitcompadd \"refs/heads/\"\n \t\t\treturn\n \t\tfi\n \t\t__gitcomp_nl \"$(__git_refs_remotes \"$remote\")\"\n-- \n1.8.0\n"},{"id":"203387","messageId":"1353116298-11798-4-git-send-email-felipe.contreras@gmail.com","threadId":"32132","inReplyTo":"1353116298-11798-1-git-send-email-felipe.contreras@gmail.com","subject":"[RFC/PATCH 3/5] completion: trivial test improvement","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-17T01:38:16Z","receivedAt":"2012-11-17T01:38:16Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Instead of passing a dummy \"\", let's check if the last character is a\nspace, and then move the _cword accordingly.\n\nApparently we were passing \"\" all the way to compgen, which fortunately\nexpanded it to nothing.\n\nLets do the right thing though.\n\nSigned-off-by: Felipe Contreras \n---\n t/t9902-completion.sh | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex cbd0fb6..0b5e1f5 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -51,6 +51,7 @@ run_completion ()\n \tlocal _cword\n \t_words=( $1 )\n \t(( _cword = ${#_words[@]} - 1 ))\n+\ttest \"${1: -1}\" == ' ' && (( _cword += 1 ))\n \t__git_wrap__git_main && print_comp\n }\n \n@@ -156,7 +157,7 @@ test_expect_success '__gitcomp - suffix' '\n '\n \n test_expect_success 'basic' '\n-\trun_completion \"git \\\"\\\"\" &&\n+\trun_completion \"git \" &&\n \t# built-in\n \tgrep -q \"^add \\$\" out &&\n \t# script\n-- \n1.8.0\n"},{"id":"203392","messageId":"1353116298-11798-5-git-send-email-felipe.contreras@gmail.com","threadId":"32132","inReplyTo":"1353116298-11798-1-git-send-email-felipe.contreras@gmail.com","subject":"[RFC/PATCH 4/5] completion: get rid of compgen","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-17T01:38:17Z","receivedAt":"2012-11-17T01:38:17Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The functionality we use is very simple, plus, this fixes a known\nbreakage 'complete tree filename with metacharacters'.\n\nSigned-off-by: Felipe Contreras \n---\n contrib/completion/git-completion.bash | 6 +++++-\n 1 file changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 975ae13..ad3e1fe 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -227,7 +227,11 @@ fi\n \n __gitcompadd ()\n {\n-\tCOMPREPLY=($(compgen -W \"$1\" -P \"$2\" -S \"$4\" -- \"$3\"))\n+\tfor x in $1; do\n+\t\tif [[ \"$x\" = \"$3\"* ]]; then\n+\t\t\tCOMPREPLY+=(\"$2$x$4\")\n+\t\tfi\n+\tdone\n }\n \n # Generates completion reply with compgen, appending a space to possible\n-- \n1.8.0\n"},{"id":"203391","messageId":"1353116298-11798-6-git-send-email-felipe.contreras@gmail.com","threadId":"32132","inReplyTo":"1353116298-11798-1-git-send-email-felipe.contreras@gmail.com","subject":"[RFC/PATCH 5/5] completion: refactor __gitcomp_1","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-17T01:38:18Z","receivedAt":"2012-11-17T01:38:18Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"It's only used by __gitcomp, so move some code there and avoid going\nthrough the loop again.\n\nWe could get rid of it altogether, but it's useful for zsh's completion\nwrapper.\n\nSigned-off-by: Felipe Contreras \n---\n contrib/completion/git-completion.bash | 25 ++++++++++++++-----------\n 1 file changed, 14 insertions(+), 11 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex ad3e1fe..d92d11e 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -58,15 +58,12 @@ __gitdir ()\n \n __gitcomp_1 ()\n {\n-\tlocal c IFS=$' \\t\\n'\n-\tfor c in $1; do\n-\t\tc=\"$c$2\"\n-\t\tcase $c in\n-\t\t--*=*|*.) ;;\n-\t\t*) c=\"$c \" ;;\n-\t\tesac\n-\t\tprintf '%s\\n' \"$c\"\n-\tdone\n+\tlocal c=$1\n+\tcase $c in\n+\t--*=*|*.) ;;\n+\t*) c=\"$c \" ;;\n+\tesac\n+\tprintf '%s\\n' \"$c\"\n }\n \n # The following function is based on code from:\n@@ -249,10 +246,16 @@ __gitcomp ()\n \t--*=)\n \t\t;;\n \t*)\n-\t\tlocal IFS=$'\\n'\n-\t\t__gitcompadd \"$(__gitcomp_1 \"${1-}\" \"${4-}\")\" \"${2-}\" \"$cur_\" \"\"\n+\t\tlocal c IFS=$' \\t\\n'\n+\t\tfor c in ${1-}; do\n+\t\t\tc=`__gitcomp_1 \"$c${4-}\"`\n+\t\t\tif [[ \"$c\" = \"$cur_\"* ]]; then\n+\t\t\t\tCOMPREPLY+=(\"${2-}$c\")\n+\t\t\tfi\n+\t\tdone\n \t\t;;\n \tesac\n+\n }\n \n # Generates completion reply with compgen from newline-separated possible\n-- \n1.8.0\n"},{"id":"203400","messageId":"20121117105837.GC12052@goldbirke","threadId":"32132","inReplyTo":"1353116298-11798-6-git-send-email-felipe.contreras@gmail.com","subject":"Re: [RFC/PATCH 5/5] completion: refactor __gitcomp_1","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-11-17T10:58:37Z","receivedAt":"2012-11-17T10:58:37Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"[Wow, that's quite the Cc-list above. I wonder why e.g. Robert ended\nup there, when all his \"sins\" were to add a couple of 'git svn'\noptions back in 2009.]\n\nOn Sat, Nov 17, 2012 at 02:38:18AM +0100, Felipe Contreras wrote:\n> It's only used by __gitcomp, so move some code there and avoid going\n> through the loop again.\n> \n> We could get rid of it altogether, but it's useful for zsh's completion\n> wrapper.\n> \n> Signed-off-by: Felipe Contreras \n> ---\n> contrib/completion/git-completion.bash | 25 ++++++++++++++-----------\n> 1 file changed, 14 insertions(+), 11 deletions(-)\n> \n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index ad3e1fe..d92d11e 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -58,15 +58,12 @@ __gitdir ()\n> \n> __gitcomp_1 ()\n> {\n> -\tlocal c IFS=$' \\t\\n'\n> -\tfor c in $1; do\n> -\t\tc=\"$c$2\"\n> -\t\tcase $c in\n> -\t\t--*=*|*.) ;;\n> -\t\t*) c=\"$c \" ;;\n> -\t\tesac\n> -\t\tprintf '%s\\n' \"$c\"\n> -\tdone\n> +\tlocal c=$1\n> +\tcase $c in\n> +\t--*=*|*.) ;;\n> +\t*) c=\"$c \" ;;\n> +\tesac\n> +\tprintf '%s\\n' \"$c\"\n> }\n> \n> # The following function is based on code from:\n> @@ -249,10 +246,16 @@ __gitcomp ()\n> \t--*=)\n> \t\t;;\n> \t*)\n> -\t\tlocal IFS=$'\\n'\n> -\t\t__gitcompadd \"$(__gitcomp_1 \"${1-}\" \"${4-}\")\" \"${2-}\" \"$cur_\" \"\"\n> +\t\tlocal c IFS=$' \\t\\n'\n> +\t\tfor c in ${1-}; do\n> +\t\t\tc=`__gitcomp_1 \"$c${4-}\"`\n\n1. Backticks.\n2. A subshell for every word in the wordlist?\n\n> +\t\t\tif [[ \"$c\" = \"$cur_\"* ]]; then\n> +\t\t\t\tCOMPREPLY+=(\"${2-}$c\")\n\nThis is the first time we use the append operator in the completion\nscript. When it came up last time the question was whether the\nbenefit of using it is large enough for worrying about supported Bash\nversions.\n\n http://article.gmane.org/gmane.comp.version-control.git/206525\n\n> +\t\t\tfi\n> +\t\tdone\n> \t\t;;\n> \tesac\n> +\n> }\n> \n> # Generates completion reply with compgen from newline-separated possible\n> -- \n> 1.8.0\n> \n"},{"id":"203401","messageId":"20121117105924.GD12052@goldbirke","threadId":"32132","inReplyTo":"1353116298-11798-4-git-send-email-felipe.contreras@gmail.com","subject":"Re: [RFC/PATCH 3/5] completion: trivial test improvement","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-11-17T10:59:24Z","receivedAt":"2012-11-17T10:59:24Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Sat, Nov 17, 2012 at 02:38:16AM +0100, Felipe Contreras wrote:\n> Instead of passing a dummy \"\", let's check if the last character is a\n> space, and then move the _cword accordingly.\n> \n> Apparently we were passing \"\" all the way to compgen, which fortunately\n> expanded it to nothing.\n\nGlad you noticed it, too.\nI posted an alternative fix (without any new conditions in the code\npath) a while ago:\n\n http://article.gmane.org/gmane.comp.version-control.git/206525\n\nWill repost it as part of my series shortly.\n\n> Lets do the right thing though.\n> \n> Signed-off-by: Felipe Contreras \n> ---\n> t/t9902-completion.sh | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n> \n> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n> index cbd0fb6..0b5e1f5 100755\n> --- a/t/t9902-completion.sh\n> +++ b/t/t9902-completion.sh\n> @@ -51,6 +51,7 @@ run_completion ()\n> \tlocal _cword\n> \t_words=( $1 )\n> \t(( _cword = ${#_words[@]} - 1 ))\n> +\ttest \"${1: -1}\" == ' ' && (( _cword += 1 ))\n> \t__git_wrap__git_main && print_comp\n> }\n> \n> @@ -156,7 +157,7 @@ test_expect_success '__gitcomp - suffix' '\n> '\n> \n> test_expect_success 'basic' '\n> -\trun_completion \"git \\\"\\\"\" &&\n> +\trun_completion \"git \" &&\n> \t# built-in\n> \tgrep -q \"^add \\$\" out &&\n> \t# script\n> -- \n> 1.8.0\n> \n"},{"id":"203402","messageId":"20121117110031.GE12052@goldbirke","threadId":"32132","inReplyTo":"1353116298-11798-5-git-send-email-felipe.contreras@gmail.com","subject":"Re: [RFC/PATCH 4/5] completion: get rid of compgen","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-11-17T11:00:31Z","receivedAt":"2012-11-17T11:00:31Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Sat, Nov 17, 2012 at 02:38:17AM +0100, Felipe Contreras wrote:\n> The functionality we use is very simple, plus, this fixes a known\n> breakage 'complete tree filename with metacharacters'.\n> \n> Signed-off-by: Felipe Contreras \n> ---\n> contrib/completion/git-completion.bash | 6 +++++-\n> 1 file changed, 5 insertions(+), 1 deletion(-)\n> \n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 975ae13..ad3e1fe 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -227,7 +227,11 @@ fi\n> \n> __gitcompadd ()\n> {\n> -\tCOMPREPLY=($(compgen -W \"$1\" -P \"$2\" -S \"$4\" -- \"$3\"))\n> +\tfor x in $1; do\n> +\t\tif [[ \"$x\" = \"$3\"* ]]; then\n> +\t\t\tCOMPREPLY+=(\"$2$x$4\")\n> +\t\tfi\n> +\tdone\n\nThe whole point of creating __gitcomp_nl() back then was to fill\nCOMPREPLY without iterating through all words in the wordlist, making\ncompletion faster for large number of words, e.g. a lot of refs, or\nlater a lot of symbols for 'git grep' in a larger project.\n\nThe loop here kills that optimization.\n\n> }\n> \n> # Generates completion reply with compgen, appending a space to possible\n> -- \n> 1.8.0\n"},{"id":"203411","messageId":"CAMP44s3OG+dzxZNpR+qELvcS37KDWh+Bnf0K1zGze4f3P4OWNg@mail.gmail.com","threadId":"32132","inReplyTo":"20121117105837.GC12052@goldbirke","subject":"Re: [RFC/PATCH 5/5] completion: refactor __gitcomp_1","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-17T11:27:40Z","receivedAt":"2012-11-17T11:27:40Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Nov 17, 2012 at 11:58 AM, SZEDER Gábor wrote:\n\n>> # The following function is based on code from:\n>> @@ -249,10 +246,16 @@ __gitcomp ()\n>> --*=)\n>> ;;\n>> *)\n>> - local IFS=$'\\n'\n>> - __gitcompadd \"$(__gitcomp_1 \"${1-}\" \"${4-}\")\" \"${2-}\" \"$cur_\" \"\"\n>> + local c IFS=$' \\t\\n'\n>> + for c in ${1-}; do\n>> + c=`__gitcomp_1 \"$c${4-}\"`\n>\n> 1. Backticks.\n> 2. A subshell for every word in the wordlist?\n\nFine, lets make it hard for zsh then:\n\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -56,19 +56,6 @@ __gitdir ()\n fi\n }\n\n-__gitcomp_1 ()\n-{\n- local c IFS=$' \\t\\n'\n- for c in $1; do\n- c=\"$c$2\"\n- case $c in\n- --*=*|*.) ;;\n- *) c=\"$c \" ;;\n- esac\n- printf '%s\\n' \"$c\"\n- done\n-}\n-\n # The following function is based on code from:\n #\n # bash_completion - programmable completion functions for bash 3.2+\n@@ -241,12 +228,22 @@ __gitcomp ()\n COMPREPLY=()\n ;;\n *)\n- local IFS=$'\\n'\n- COMPREPLY=($(compgen -P \"${2-}\" \\\n- -W \"$(__gitcomp_1 \"${1-}\" \"${4-}\")\" \\\n- -- \"$cur_\"))\n+ local c i IFS=$' \\t\\n'\n+ i=0\n+ for c in ${1-}; do\n+ c=\"$c${4-}\"\n+ case $c in\n+ --*=*|*.) ;;\n+ *) c=\"$c \" ;;\n+ esac\n+ if [[ \"$c\" = \"$cur_\"* ]]; then\n+ (( i++ ))\n+ COMPREPLY[$i]=\"${2-}$c\"\n+ fi\n+ done\n ;;\n esac\n+\n }\n\n # Generates completion reply with compgen from newline-separated possible\n\n-- \nFelipe Contreras\n"},{"id":"203413","messageId":"CAMP44s21CUb3_KhHBfJXW+Eqd45kz1hcbx3GCbs+f0HNRDEAzw@mail.gmail.com","threadId":"32132","inReplyTo":"20121117110031.GE12052@goldbirke","subject":"Re: [RFC/PATCH 4/5] completion: get rid of compgen","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-17T11:42:38Z","receivedAt":"2012-11-17T11:42:38Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Nov 17, 2012 at 12:00 PM, SZEDER Gábor wrote:\n> On Sat, Nov 17, 2012 at 02:38:17AM +0100, Felipe Contreras wrote:\n>> The functionality we use is very simple, plus, this fixes a known\n>> breakage 'complete tree filename with metacharacters'.\n>>\n>> Signed-off-by: Felipe Contreras \n>> ---\n>> contrib/completion/git-completion.bash | 6 +++++-\n>> 1 file changed, 5 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n>> index 975ae13..ad3e1fe 100644\n>> --- a/contrib/completion/git-completion.bash\n>> +++ b/contrib/completion/git-completion.bash\n>> @@ -227,7 +227,11 @@ fi\n>>\n>> __gitcompadd ()\n>> {\n>> - COMPREPLY=($(compgen -W \"$1\" -P \"$2\" -S \"$4\" -- \"$3\"))\n>> + for x in $1; do\n>> + if [[ \"$x\" = \"$3\"* ]]; then\n>> + COMPREPLY+=(\"$2$x$4\")\n>> + fi\n>> + done\n>\n> The whole point of creating __gitcomp_nl() back then was to fill\n> COMPREPLY without iterating through all words in the wordlist, making\n> completion faster for large number of words, e.g. a lot of refs, or\n> later a lot of symbols for 'git grep' in a larger project.\n>\n> The loop here kills that optimization.\n\nSo your solution is to move the loop to awk? I fail to see how that\ncould bring more optimization, specially since it includes an extra\nfork now.\n\n-- \nFelipe Contreras\n"},{"id":"203418","messageId":"20121117141215.GG12052@goldbirke","threadId":"32132","inReplyTo":"CAMP44s21CUb3_KhHBfJXW+Eqd45kz1hcbx3GCbs+f0HNRDEAzw@mail.gmail.com","subject":"Re: [RFC/PATCH 4/5] completion: get rid of compgen","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-11-17T14:12:15Z","receivedAt":"2012-11-17T14:12:15Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Sat, Nov 17, 2012 at 12:42:38PM +0100, Felipe Contreras wrote:\n> On Sat, Nov 17, 2012 at 12:00 PM, SZEDER Gábor wrote:\n> > On Sat, Nov 17, 2012 at 02:38:17AM +0100, Felipe Contreras wrote:\n> >> The functionality we use is very simple, plus, this fixes a known\n> >> breakage 'complete tree filename with metacharacters'.\n> >>\n> >> Signed-off-by: Felipe Contreras \n> >> ---\n> >> contrib/completion/git-completion.bash | 6 +++++-\n> >> 1 file changed, 5 insertions(+), 1 deletion(-)\n> >>\n> >> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> >> index 975ae13..ad3e1fe 100644\n> >> --- a/contrib/completion/git-completion.bash\n> >> +++ b/contrib/completion/git-completion.bash\n> >> @@ -227,7 +227,11 @@ fi\n> >>\n> >> __gitcompadd ()\n> >> {\n> >> - COMPREPLY=($(compgen -W \"$1\" -P \"$2\" -S \"$4\" -- \"$3\"))\n> >> + for x in $1; do\n> >> + if [[ \"$x\" = \"$3\"* ]]; then\n> >> + COMPREPLY+=(\"$2$x$4\")\n> >> + fi\n> >> + done\n> >\n> > The whole point of creating __gitcomp_nl() back then was to fill\n> > COMPREPLY without iterating through all words in the wordlist, making\n> > completion faster for large number of words, e.g. a lot of refs, or\n> > later a lot of symbols for 'git grep' in a larger project.\n> >\n> > The loop here kills that optimization.\n> \n> So your solution is to move the loop to awk? I fail to see how that\n> could bring more optimization, specially since it includes an extra\n> fork now.\n\nThis patch didn't aim for more optimization, but it was definitely a\ngoal not to waste what we gained by creating __gitcomp_nl() in\na31e6262 (completion: optimize refs completion, 2011-10-15). However,\nas it turns out the new version with awk is actually faster than\ncurrent master with compgen:\n\n Before:\n\n $ refs=\"$(for i in {0..9999} ; do echo branch$i ; done)\"\n $ time __gitcomp_nl \"$refs\"\n\n real 0m0.242s\n user 0m0.220s\n sys 0m0.028s\n\n After:\n\n $ time __gitcomp_nl \"$refs\"\n\n real 0m0.109s\n user 0m0.096s\n sys 0m0.012s\n"},{"id":"203419","messageId":"20121117141330.GH12052@goldbirke","threadId":"32132","inReplyTo":"CAMP44s3OG+dzxZNpR+qELvcS37KDWh+Bnf0K1zGze4f3P4OWNg@mail.gmail.com","subject":"Re: [RFC/PATCH 5/5] completion: refactor __gitcomp_1","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-11-17T14:13:30Z","receivedAt":"2012-11-17T14:13:30Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Sat, Nov 17, 2012 at 12:27:40PM +0100, Felipe Contreras wrote:\n> On Sat, Nov 17, 2012 at 11:58 AM, SZEDER Gábor wrote:\n> \n> >> # The following function is based on code from:\n> >> @@ -249,10 +246,16 @@ __gitcomp ()\n> >> --*=)\n> >> ;;\n> >> *)\n> >> - local IFS=$'\\n'\n> >> - __gitcompadd \"$(__gitcomp_1 \"${1-}\" \"${4-}\")\" \"${2-}\" \"$cur_\" \"\"\n> >> + local c IFS=$' \\t\\n'\n> >> + for c in ${1-}; do\n> >> + c=`__gitcomp_1 \"$c${4-}\"`\n> >\n> > 1. Backticks.\n> > 2. A subshell for every word in the wordlist?\n> \n> Fine, lets make it hard for zsh then:\n\nNo, it's about keeping it usable. With this change offering the\napproximately 170 commands for 'git help ' would take more than 4\nseconds on Windows.\n\n\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -56,19 +56,6 @@ __gitdir ()\n> fi\n> }\n> \n> -__gitcomp_1 ()\n> -{\n> - local c IFS=$' \\t\\n'\n> - for c in $1; do\n> - c=\"$c$2\"\n> - case $c in\n> - --*=*|*.) ;;\n> - *) c=\"$c \" ;;\n> - esac\n> - printf '%s\\n' \"$c\"\n> - done\n> -}\n> -\n> # The following function is based on code from:\n> #\n> # bash_completion - programmable completion functions for bash 3.2+\n> @@ -241,12 +228,22 @@ __gitcomp ()\n> COMPREPLY=()\n> ;;\n> *)\n> - local IFS=$'\\n'\n> - COMPREPLY=($(compgen -P \"${2-}\" \\\n> - -W \"$(__gitcomp_1 \"${1-}\" \"${4-}\")\" \\\n> - -- \"$cur_\"))\n> + local c i IFS=$' \\t\\n'\n> + i=0\n> + for c in ${1-}; do\n> + c=\"$c${4-}\"\n> + case $c in\n> + --*=*|*.) ;;\n> + *) c=\"$c \" ;;\n> + esac\n> + if [[ \"$c\" = \"$cur_\"* ]]; then\n> + (( i++ ))\n> + COMPREPLY[$i]=\"${2-}$c\"\n> + fi\n> + done\n> ;;\n> esac\n> +\n> }\n> \n> # Generates completion reply with compgen from newline-separated possible\n> \n> -- \n> Felipe Contreras\n"},{"id":"203433","messageId":"CAMP44s0YnoGwJEgUbXZ831_OrgO=dDf5_QHxT5JYnGUHYPuiTw@mail.gmail.com","threadId":"32132","inReplyTo":"20121117141215.GG12052@goldbirke","subject":"Re: [RFC/PATCH 4/5] completion: get rid of compgen","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-17T19:33:22Z","receivedAt":"2012-11-17T19:33:22Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Nov 17, 2012 at 3:12 PM, SZEDER Gábor wrote:\n> On Sat, Nov 17, 2012 at 12:42:38PM +0100, Felipe Contreras wrote:\n>> On Sat, Nov 17, 2012 at 12:00 PM, SZEDER Gábor wrote:\n>> > On Sat, Nov 17, 2012 at 02:38:17AM +0100, Felipe Contreras wrote:\n>> >> The functionality we use is very simple, plus, this fixes a known\n>> >> breakage 'complete tree filename with metacharacters'.\n>> >>\n>> >> Signed-off-by: Felipe Contreras \n>> >> ---\n>> >> contrib/completion/git-completion.bash | 6 +++++-\n>> >> 1 file changed, 5 insertions(+), 1 deletion(-)\n>> >>\n>> >> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n>> >> index 975ae13..ad3e1fe 100644\n>> >> --- a/contrib/completion/git-completion.bash\n>> >> +++ b/contrib/completion/git-completion.bash\n>> >> @@ -227,7 +227,11 @@ fi\n>> >>\n>> >> __gitcompadd ()\n>> >> {\n>> >> - COMPREPLY=($(compgen -W \"$1\" -P \"$2\" -S \"$4\" -- \"$3\"))\n>> >> + for x in $1; do\n>> >> + if [[ \"$x\" = \"$3\"* ]]; then\n>> >> + COMPREPLY+=(\"$2$x$4\")\n>> >> + fi\n>> >> + done\n>> >\n>> > The whole point of creating __gitcomp_nl() back then was to fill\n>> > COMPREPLY without iterating through all words in the wordlist, making\n>> > completion faster for large number of words, e.g. a lot of refs, or\n>> > later a lot of symbols for 'git grep' in a larger project.\n>> >\n>> > The loop here kills that optimization.\n>>\n>> So your solution is to move the loop to awk? I fail to see how that\n>> could bring more optimization, specially since it includes an extra\n>> fork now.\n>\n> This patch didn't aim for more optimization, but it was definitely a\n> goal not to waste what we gained by creating __gitcomp_nl() in\n> a31e6262 (completion: optimize refs completion, 2011-10-15). However,\n> as it turns out the new version with awk is actually faster than\n> current master with compgen:\n>\n> Before:\n>\n> $ refs=\"$(for i in {0..9999} ; do echo branch$i ; done)\"\n> $ time __gitcomp_nl \"$refs\"\n>\n> real 0m0.242s\n> user 0m0.220s\n> sys 0m0.028s\n>\n> After:\n>\n> $ time __gitcomp_nl \"$refs\"\n>\n> real 0m0.109s\n> user 0m0.096s\n> sys 0m0.012s\n\nThis one is even faster:\n\nwhile read -r x; do\n\tif [[ \"$x\" == \"$3\"* ]]; then\n\t\tCOMPREPLY+=(\"$2$x$4\")\n\tfi\ndone <<< $1\n\n== 10000 ==\none:\nreal\t0m0.148s\nuser\t0m0.134s\nsys\t0m0.025s\ntwo:\nreal\t0m0.055s\nuser\t0m0.054s\nsys\t0m0.000s\n\n-- \nFelipe Contreras\n"},{"id":"203449","messageId":"CAMP44s3nyrYNf11O6Kehu9uZd5Ji8O-aD8GgQh0QW-Xw_8Ah+g@mail.gmail.com","threadId":"32132","inReplyTo":"CAMP44s0YnoGwJEgUbXZ831_OrgO=dDf5_QHxT5JYnGUHYPuiTw@mail.gmail.com","subject":"Re: [RFC/PATCH 4/5] completion: get rid of compgen","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-18T00:53:50Z","receivedAt":"2012-11-18T00:53:50Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Nov 17, 2012 at 8:33 PM, Felipe Contreras\n wrote:\n> On Sat, Nov 17, 2012 at 3:12 PM, SZEDER Gábor wrote:\n>> On Sat, Nov 17, 2012 at 12:42:38PM +0100, Felipe Contreras wrote:\n>>> On Sat, Nov 17, 2012 at 12:00 PM, SZEDER Gábor wrote:\n>>> > On Sat, Nov 17, 2012 at 02:38:17AM +0100, Felipe Contreras wrote:\n>>> >> The functionality we use is very simple, plus, this fixes a known\n>>> >> breakage 'complete tree filename with metacharacters'.\n>>> >>\n>>> >> Signed-off-by: Felipe Contreras \n>>> >> ---\n>>> >> contrib/completion/git-completion.bash | 6 +++++-\n>>> >> 1 file changed, 5 insertions(+), 1 deletion(-)\n>>> >>\n>>> >> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n>>> >> index 975ae13..ad3e1fe 100644\n>>> >> --- a/contrib/completion/git-completion.bash\n>>> >> +++ b/contrib/completion/git-completion.bash\n>>> >> @@ -227,7 +227,11 @@ fi\n>>> >>\n>>> >> __gitcompadd ()\n>>> >> {\n>>> >> - COMPREPLY=($(compgen -W \"$1\" -P \"$2\" -S \"$4\" -- \"$3\"))\n>>> >> + for x in $1; do\n>>> >> + if [[ \"$x\" = \"$3\"* ]]; then\n>>> >> + COMPREPLY+=(\"$2$x$4\")\n>>> >> + fi\n>>> >> + done\n>>> >\n>>> > The whole point of creating __gitcomp_nl() back then was to fill\n>>> > COMPREPLY without iterating through all words in the wordlist, making\n>>> > completion faster for large number of words, e.g. a lot of refs, or\n>>> > later a lot of symbols for 'git grep' in a larger project.\n>>> >\n>>> > The loop here kills that optimization.\n>>>\n>>> So your solution is to move the loop to awk? I fail to see how that\n>>> could bring more optimization, specially since it includes an extra\n>>> fork now.\n>>\n>> This patch didn't aim for more optimization, but it was definitely a\n>> goal not to waste what we gained by creating __gitcomp_nl() in\n>> a31e6262 (completion: optimize refs completion, 2011-10-15). However,\n>> as it turns out the new version with awk is actually faster than\n>> current master with compgen:\n>>\n>> Before:\n>>\n>> $ refs=\"$(for i in {0..9999} ; do echo branch$i ; done)\"\n>> $ time __gitcomp_nl \"$refs\"\n>>\n>> real 0m0.242s\n>> user 0m0.220s\n>> sys 0m0.028s\n>>\n>> After:\n>>\n>> $ time __gitcomp_nl \"$refs\"\n>>\n>> real 0m0.109s\n>> user 0m0.096s\n>> sys 0m0.012s\n>\n> This one is even faster:\n>\n> while read -r x; do\n> if [[ \"$x\" == \"$3\"* ]]; then\n> COMPREPLY+=(\"$2$x$4\")\n> fi\n> done <<< $1\n>\n> == 10000 ==\n> one:\n> real 0m0.148s\n> user 0m0.134s\n> sys 0m0.025s\n> two:\n> real 0m0.055s\n> user 0m0.054s\n> sys 0m0.000s\n\nAh, nevermind, I didn't quote the $1.\n\nHowever, this one is quite close and much simpler:\n\n\tlocal IFS=$'\\n'\n\tCOMPREPLY=($(printf -- \"$2%s$4\\n\" $1 | grep \"^$2$3\"))\n\n== 10000 ==\nawk 1:\nreal\t0m0.064s\nuser\t0m0.062s\nsys\t0m0.003s\nprintf:\nreal\t0m0.069s\nuser\t0m0.064s\nsys\t0m0.020s\n\n\n-- \nFelipe Contreras\n"}]}