{"thread":{"id":"31814","subject":"[PATCH v5 0/3] completion: refactor and zsh wrapper","startedAt":"2012-10-14T15:52:48Z","lastAt":"2012-10-30T23:18:32Z","messageCount":14,"participants":["Felipe Contreras","Matthieu Moy","SZEDER Gábor"],"isPatch":true,"patchVersion":5,"patchTotal":3},"messages":[{"id":"201177","messageId":"1350229971-9343-1-git-send-email-felipe.contreras@gmail.com","threadId":"31814","inReplyTo":null,"subject":"[PATCH v5 0/3] completion: refactor and zsh wrapper","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-10-14T15:52:48Z","receivedAt":"2012-10-14T15:52:48Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hi,\n\nHere's a bit of reorganition. I'm introducing a new __gitcompadd helper that is\nuseful to wrapp all changes to COMPREPLY. 2nd and 3rd patches show how it's\nuseful.\n\nThe zsh wrapper is now very very simple, but I haven't received much feedback\nyet. I hope it will get in at some point in time.\n\nFelipe Contreras (3):\n  completion: add new __gitcompadd helper\n  tests: use __gitcompadd to simplify completion tests\n  completion: add new zsh completion\n\n contrib/completion/git-completion.bash | 65 ++++++++++++++++++----------------\n contrib/completion/git-completion.zsh  | 48 +++++++++++++++++++++++++\n t/t9902-completion.sh                  | 29 +++++----------\n 3 files changed, 91 insertions(+), 51 deletions(-)\n create mode 100644 contrib/completion/git-completion.zsh\n\n-- \n1.7.12.1\n"},{"id":"201179","messageId":"1350229971-9343-2-git-send-email-felipe.contreras@gmail.com","threadId":"31814","inReplyTo":"1350229971-9343-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 1/3] completion: add new __gitcompadd helper","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-10-14T15:52:49Z","receivedAt":"2012-10-14T15:52:49Z","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 simplifcation of the completino tests (separate\npatch).\n\nThere should be no functional changes.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n contrib/completion/git-completion.bash | 65 ++++++++++++++++++----------------\n 1 file changed, 34 insertions(+), 31 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex d743e56..01325de 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@@ -238,13 +243,11 @@ __gitcomp ()\n \n \tcase \"$cur_\" in\n \t--*=)\n-\t\tCOMPREPLY=()\n+\t\t__gitcompadd\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@@ -261,7 +264,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@@ -486,7 +489,7 @@ __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\t__gitcompadd\n \t\t\t\treturn\n \t\t\t\t;;\n \t\t\t*) ;;\n@@ -502,7 +505,7 @@ __git_complete_remote_or_refspec ()\n \t\treturn\n \tfi\n \tif [ $no_complete_refspec = 1 ]; then\n-\t\tCOMPREPLY=()\n+\t\t__gitcompadd\n \t\treturn\n \tfi\n \t[ \"$remote\" = \".\" ] && remote=\n@@ -776,7 +779,7 @@ _git_am ()\n \t\t\t\"\n \t\treturn\n \tesac\n-\tCOMPREPLY=()\n+\t__gitcompadd\n }\n \n _git_apply ()\n@@ -796,7 +799,7 @@ _git_apply ()\n \t\t\t\"\n \t\treturn\n \tesac\n-\tCOMPREPLY=()\n+\t__gitcompadd\n }\n \n _git_add ()\n@@ -811,7 +814,7 @@ _git_add ()\n \t\t\t\"\n \t\treturn\n \tesac\n-\tCOMPREPLY=()\n+\t__gitcompadd\n }\n \n _git_archive ()\n@@ -856,7 +859,7 @@ _git_bisect ()\n \t\t__gitcomp_nl \"$(__git_refs)\"\n \t\t;;\n \t*)\n-\t\tCOMPREPLY=()\n+\t\t__gitcompadd\n \t\t;;\n \tesac\n }\n@@ -965,7 +968,7 @@ _git_clean ()\n \t\treturn\n \t\t;;\n \tesac\n-\tCOMPREPLY=()\n+\t__gitcompadd\n }\n \n _git_clone ()\n@@ -989,7 +992,7 @@ _git_clone ()\n \t\treturn\n \t\t;;\n \tesac\n-\tCOMPREPLY=()\n+\t__gitcompadd\n }\n \n _git_commit ()\n@@ -1023,7 +1026,7 @@ _git_commit ()\n \t\t\t\"\n \t\treturn\n \tesac\n-\tCOMPREPLY=()\n+\t__gitcompadd\n }\n \n _git_describe ()\n@@ -1154,7 +1157,7 @@ _git_fsck ()\n \t\treturn\n \t\t;;\n \tesac\n-\tCOMPREPLY=()\n+\t__gitcompadd\n }\n \n _git_gc ()\n@@ -1165,7 +1168,7 @@ _git_gc ()\n \t\treturn\n \t\t;;\n \tesac\n-\tCOMPREPLY=()\n+\t__gitcompadd\n }\n \n _git_gitk ()\n@@ -1242,7 +1245,7 @@ _git_init ()\n \t\treturn\n \t\t;;\n \tesac\n-\tCOMPREPLY=()\n+\t__gitcompadd\n }\n \n _git_ls_files ()\n@@ -1261,7 +1264,7 @@ _git_ls_files ()\n \t\treturn\n \t\t;;\n \tesac\n-\tCOMPREPLY=()\n+\t__gitcompadd\n }\n \n _git_ls_remote ()\n@@ -1377,7 +1380,7 @@ _git_mergetool ()\n \t\treturn\n \t\t;;\n \tesac\n-\tCOMPREPLY=()\n+\t__gitcompadd\n }\n \n _git_merge_base ()\n@@ -1393,7 +1396,7 @@ _git_mv ()\n \t\treturn\n \t\t;;\n \tesac\n-\tCOMPREPLY=()\n+\t__gitcompadd\n }\n \n _git_name_rev ()\n@@ -1563,7 +1566,7 @@ _git_send_email ()\n \t\treturn\n \t\t;;\n \tesac\n-\tCOMPREPLY=()\n+\t__gitcompadd\n }\n \n _git_stage ()\n@@ -1616,7 +1619,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@@ -1676,7 +1679,7 @@ _git_config ()\n \t\treturn\n \t\t;;\n \t*.*)\n-\t\tCOMPREPLY=()\n+\t\t__gitcompadd\n \t\treturn\n \t\t;;\n \tesac\n@@ -2056,7 +2059,7 @@ _git_remote ()\n \t\t__gitcomp \"$c\"\n \t\t;;\n \t*)\n-\t\tCOMPREPLY=()\n+\t\t__gitcompadd\n \t\t;;\n \tesac\n }\n@@ -2100,7 +2103,7 @@ _git_rm ()\n \t\treturn\n \t\t;;\n \tesac\n-\tCOMPREPLY=()\n+\t__gitcompadd\n }\n \n _git_shortlog ()\n@@ -2170,7 +2173,7 @@ _git_stash ()\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\t\t__gitcompadd\n \t\t\tfi\n \t\t\t;;\n \t\tesac\n@@ -2183,14 +2186,14 @@ _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__gitcompadd\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__gitcompadd\n \t\t\t;;\n \t\tesac\n \tfi\n@@ -2307,7 +2310,7 @@ _git_svn ()\n \t\t\t__gitcomp \"--revision= --parent\"\n \t\t\t;;\n \t\t*)\n-\t\t\tCOMPREPLY=()\n+\t\t\t__gitcompadd\n \t\t\t;;\n \t\tesac\n \tfi\n@@ -2332,13 +2335,13 @@ _git_tag ()\n \n \tcase \"$prev\" in\n \t-m|-F)\n-\t\tCOMPREPLY=()\n+\t\t__gitcompadd\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\t\t__gitcompadd\n \t\tfi\n \t\t;;\n \t*)\n-- \n1.7.12.1\n"},{"id":"201180","messageId":"1350229971-9343-3-git-send-email-felipe.contreras@gmail.com","threadId":"31814","inReplyTo":"1350229971-9343-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 2/3] tests: use __gitcompadd to simplify completion tests","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-10-14T15:52:50Z","receivedAt":"2012-10-14T15:52:50Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t9902-completion.sh | 29 +++++++++--------------------\n 1 file changed, 9 insertions(+), 20 deletions(-)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 92d7eb4..49c6eb4 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -39,19 +39,18 @@ _get_comp_words_by_ref ()\n \tdone\n }\n \n-print_comp ()\n+__gitcompadd ()\n {\n-\tlocal IFS=$'\\n'\n-\techo \"${COMPREPLY[*]}\" > out\n+\tcompgen -P \"${2-}\" -S \"${4- }\" -W \"$1\" -- \"${3-$cur}\" > out\n }\n \n run_completion ()\n {\n-\tlocal -a COMPREPLY _words\n+\tlocal -a _words\n \tlocal _cword\n \t_words=( $1 )\n \t(( _cword = ${#_words[@]} - 1 ))\n-\t__git_wrap__git_main && print_comp\n+\t__git_wrap__git_main\n }\n \n test_completion ()\n@@ -70,12 +69,10 @@ test_expect_success '__gitcomp - trailing space - options' '\n \t--reset-author Z\n \tEOF\n \t(\n-\t\tlocal -a COMPREPLY &&\n \t\tcur=\"--re\" &&\n \t\t__gitcomp \"--dry-run --reuse-message= --reedit-message=\n \t\t\t\t--reset-author\" &&\n-\t\tIFS=\"$newline\" &&\n-\t\techo \"${COMPREPLY[*]}\" > out\n+\t\tIFS=\"$newline\"\n \t) &&\n \ttest_cmp expected out\n '\n@@ -88,12 +85,10 @@ test_expect_success '__gitcomp - trailing space - config keys' '\n \tbrowser.Z\n \tEOF\n \t(\n-\t\tlocal -a COMPREPLY &&\n \t\tcur=\"br\" &&\n \t\t__gitcomp \"branch. branch.autosetupmerge\n \t\t\t\tbranch.autosetuprebase browser.\" &&\n-\t\tIFS=\"$newline\" &&\n-\t\techo \"${COMPREPLY[*]}\" > out\n+\t\tIFS=\"$newline\"\n \t) &&\n \ttest_cmp expected out\n '\n@@ -104,12 +99,10 @@ test_expect_success '__gitcomp - option parameter' '\n \tresolve Z\n \tEOF\n \t(\n-\t\tlocal -a COMPREPLY &&\n \t\tcur=\"--strategy=re\" &&\n \t\t__gitcomp \"octopus ours recursive resolve subtree\n \t\t\t\" \"\" \"re\" &&\n-\t\tIFS=\"$newline\" &&\n-\t\techo \"${COMPREPLY[*]}\" > out\n+\t\tIFS=\"$newline\"\n \t) &&\n \ttest_cmp expected out\n '\n@@ -120,12 +113,10 @@ test_expect_success '__gitcomp - prefix' '\n \tbranch.maint.mergeoptions Z\n \tEOF\n \t(\n-\t\tlocal -a COMPREPLY &&\n \t\tcur=\"branch.me\" &&\n \t\t__gitcomp \"remote merge mergeoptions rebase\n \t\t\t\" \"branch.maint.\" \"me\" &&\n-\t\tIFS=\"$newline\" &&\n-\t\techo \"${COMPREPLY[*]}\" > out\n+\t\tIFS=\"$newline\"\n \t) &&\n \ttest_cmp expected out\n '\n@@ -136,12 +127,10 @@ test_expect_success '__gitcomp - suffix' '\n \tbranch.maint.Z\n \tEOF\n \t(\n-\t\tlocal -a COMPREPLY &&\n \t\tcur=\"branch.me\" &&\n \t\t__gitcomp \"master maint next pu\n \t\t\t\" \"branch.\" \"ma\" \".\" &&\n-\t\tIFS=\"$newline\" &&\n-\t\techo \"${COMPREPLY[*]}\" > out\n+\t\tIFS=\"$newline\"\n \t) &&\n \ttest_cmp expected out\n '\n-- \n1.7.12.1\n"},{"id":"201178","messageId":"1350229971-9343-4-git-send-email-felipe.contreras@gmail.com","threadId":"31814","inReplyTo":"1350229971-9343-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 3/3] completion: add new zsh completion","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-10-14T15:52:51Z","receivedAt":"2012-10-14T15:52:51Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"It seems there's always issues with zsh's bash completion emulation.\nI've tried to fix as many as I could and most of the fixes are already\nin the latest version of zsh, but still, there are issues.\n\nThere is no point in going through all that pain; the emulation is easy\nto achieve, and this patch works better than zsh's emulation.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n\nv5:\n\n * Even more simplification by using __gitcompadd\n\nv4:\n\n * Simplification updates for the latest bash completion\n\nv3:\n\n * Simplification\n * Avoid COMPREPLY; call compadd directly\n * Fix _get_comp_words_by_ref\n\n contrib/completion/git-completion.zsh | 48 +++++++++++++++++++++++++++++++++++\n 1 file changed, 48 insertions(+)\n create mode 100644 contrib/completion/git-completion.zsh\n\ndiff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh\nnew file mode 100644\nindex 0000000..dbb5261\n--- /dev/null\n+++ b/contrib/completion/git-completion.zsh\n@@ -0,0 +1,48 @@\n+#compdef git gitk\n+\n+# zsh completion wrapper for git\n+#\n+# You need git's bash completion script installed somewhere, by default on the\n+# same directory as this script.\n+#\n+# If your script is on ~/.git-completion.sh instead, you can configure it on\n+# your ~/.zshrc:\n+#\n+#  zstyle ':completion:*:*:git:*' script ~/.git-completion.sh\n+#\n+# The recommended way to install this script is to copy to\n+# '~/.zsh/completion/_git', and then add the following to your ~/.zshrc file:\n+#\n+#  fpath=(~/.zsh/completion $fpath)\n+\n+complete ()\n+{\n+\t# do nothing\n+\treturn 0\n+}\n+\n+zstyle -s \":completion:*:*:git:*\" script script\n+test -z \"$script\" && script=\"$(dirname ${funcsourcetrace[1]%:*})\"/git-completion.bash\n+ZSH_VERSION='' . \"$script\"\n+\n+__gitcompadd ()\n+{\n+\tcompadd -Q -S \"$4\" -P \"$2\" -p \"${(M)cur#*[=:]}\" -- ${=1} && _ret=0\n+}\n+\n+_git ()\n+{\n+\tlocal _ret=1\n+\t() {\n+\t\temulate -L ksh\n+\t\tlocal cur cword prev\n+\t\tcur=${words[CURRENT-1]}\n+\t\tprev=${words[CURRENT-2]}\n+\t\tlet cword=CURRENT-1\n+\t\t__${service}_main\n+\t}\n+\tlet _ret && _default -S '' && _ret=0\n+\treturn _ret\n+}\n+\n+_git\n-- \n1.7.12.1\n"},{"id":"201233","messageId":"CAMP44s1XG43c0MDCa2mSA=R5YMF=jNSGVdv=UrHKsHmrmRCXMw@mail.gmail.com","threadId":"31814","inReplyTo":"1350229971-9343-4-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 3/3] completion: add new zsh completion","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-10-15T06:38:03Z","receivedAt":"2012-10-15T06:38:03Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Oct 14, 2012 at 5:52 PM, Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n\n> +__gitcompadd ()\n> +{\n> +       compadd -Q -S \"$4\" -P \"$2\" -p \"${(M)cur#*[=:]}\" -- ${=1} && _ret=0\n\nI just found a bug, should be:\n\ncompadd -Q -S \"$4\" -P \"${(M)cur#*[=:]}\" -p \"$2\" -- ${=1} && _ret=0\n\n-- \nFelipe Contreras\n"},{"id":"201269","messageId":"vpqr4oz65wr.fsf@grenoble-inp.fr","threadId":"31814","inReplyTo":"1350229971-9343-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 0/3] completion: refactor and zsh wrapper","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-10-15T16:45:08Z","receivedAt":"2012-10-15T16:45:08Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> Hi,\n>\n> Here's a bit of reorganition. I'm introducing a new __gitcompadd helper that is\n> useful to wrapp all changes to COMPREPLY. 2nd and 3rd patches show how it's\n> useful.\n>\n> The zsh wrapper is now very very simple, but I haven't received much feedback\n> yet. I hope it will get in at some point in time.\n\nI didn't review the code, but I've installed your patch series, and it\nseems to work well.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"201300","messageId":"CAMP44s2ej4oz4E4tYk7hNjBXe-pvK-v2kQDuz9pg42stBtL61w@mail.gmail.com","threadId":"31814","inReplyTo":"1350229971-9343-3-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 2/3] tests: use __gitcompadd to simplify completion tests","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-10-16T00:24:54Z","receivedAt":"2012-10-16T00:24:54Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Oct 14, 2012 at 5:52 PM, Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n\n> -print_comp ()\n> +__gitcompadd ()\n>  {\n> -       local IFS=$'\\n'\n> -       echo \"${COMPREPLY[*]}\" > out\n> +       compgen -P \"${2-}\" -S \"${4- }\" -W \"$1\" -- \"${3-$cur}\" > out\n\nRather:\n\ncompgen -W \"$1\" -P \"$2\" -S \"$4\" -- \"$3\" > out\n\n-- \nFelipe Contreras\n"},{"id":"201432","messageId":"20121017172808.GD2156@goldbirke","threadId":"31814","inReplyTo":"1350229971-9343-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 1/3] completion: add new __gitcompadd helper","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-10-17T17:28:08Z","receivedAt":"2012-10-17T17:28:08Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Sun, Oct 14, 2012 at 05:52:49PM +0200, Felipe Contreras wrote:\n> The idea is to never touch the COMPREPLY variable directly.\n> \n> This allows other completion systems override __gitcompadd, and do\n> something different instead.\n> \n> Also, this allows the simplifcation of the completino tests (separate\n> patch).\n> \n> There should be no functional changes.\n> \n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  contrib/completion/git-completion.bash | 65 ++++++++++++++++++----------------\n>  1 file changed, 34 insertions(+), 31 deletions(-)\n> \n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index d743e56..01325de 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> @@ -238,13 +243,11 @@ __gitcomp ()\n>  \n>  \tcase \"$cur_\" in\n>  \t--*=)\n> -\t\tCOMPREPLY=()\n> +\t\t__gitcompadd\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> @@ -261,7 +264,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\nI feel hesitant about this change.  One of the ways I'm exploring to\nfix the issues with shell metacharacters and expansion in compgen is\nto actually replace compgen.  We already iterate over all possible\ncompletion words in __gitcomp_1(), so it doesn't make much of a\ndifference to do the filtering for the current word while we are at\nit.  However, the way __gitcompadd() encapsulates COMPREPLY=($(compgen\n...)), and tha basic idea of never touching COMPREPLY directly make\nthis basically impossible.\n\n>  __git_heads ()\n> @@ -486,7 +489,7 @@ __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\t__gitcompadd\n>  \t\t\t\treturn\n>  \t\t\t\t;;\n>  \t\t\t*) ;;\n> @@ -502,7 +505,7 @@ __git_complete_remote_or_refspec ()\n>  \t\treturn\n>  \tfi\n>  \tif [ $no_complete_refspec = 1 ]; then\n> -\t\tCOMPREPLY=()\n> +\t\t__gitcompadd\n>  \t\treturn\n>  \tfi\n>  \t[ \"$remote\" = \".\" ] && remote=\n> @@ -776,7 +779,7 @@ _git_am ()\n>  \t\t\t\"\n>  \t\treturn\n>  \tesac\n> -\tCOMPREPLY=()\n> +\t__gitcompadd\n\nThese changes effectively run compgen in a subshell to generate an\nempty completion reply.  While it doesn't really matter on Linux,\nit'll add another half a tenth of a second delay in those cases on my\nWindows machine.  At least it should be conditional, i.e. $(compgen\n...) shouldn't be executed when there are no possible completion\nwords.\n\nHowever, I think those COMPREPLY=() assignments are pointless anyway.\nCOMPREPLY is always empty when completion functions are invoked, so\nthere is no need to explicitly set it to an empty array when we don't\nprovide any words for completion.  Their only use is basically to\nexplicitly tell us humans that in those cases we don't offer any words\nfor completion.  But we don't do that consistently: there are several\nplaces without offering words for completion and without COMPREPLY=(),\ne.g. the '__git_has_doubledash && return' pattern.\n\nPerhaps it would be time to get rid of these COMPREPLY=() assignments?\n\n>  }\n>  \n>  _git_apply ()\n> @@ -796,7 +799,7 @@ _git_apply ()\n>  \t\t\t\"\n>  \t\treturn\n>  \tesac\n> -\tCOMPREPLY=()\n> +\t__gitcompadd\n>  }\n>  \n>  _git_add ()\n> @@ -811,7 +814,7 @@ _git_add ()\n>  \t\t\t\"\n>  \t\treturn\n>  \tesac\n> -\tCOMPREPLY=()\n> +\t__gitcompadd\n>  }\n>  \n>  _git_archive ()\n> @@ -856,7 +859,7 @@ _git_bisect ()\n>  \t\t__gitcomp_nl \"$(__git_refs)\"\n>  \t\t;;\n>  \t*)\n> -\t\tCOMPREPLY=()\n> +\t\t__gitcompadd\n>  \t\t;;\n>  \tesac\n>  }\n> @@ -965,7 +968,7 @@ _git_clean ()\n>  \t\treturn\n>  \t\t;;\n>  \tesac\n> -\tCOMPREPLY=()\n> +\t__gitcompadd\n>  }\n>  \n>  _git_clone ()\n> @@ -989,7 +992,7 @@ _git_clone ()\n>  \t\treturn\n>  \t\t;;\n>  \tesac\n> -\tCOMPREPLY=()\n> +\t__gitcompadd\n>  }\n>  \n>  _git_commit ()\n> @@ -1023,7 +1026,7 @@ _git_commit ()\n>  \t\t\t\"\n>  \t\treturn\n>  \tesac\n> -\tCOMPREPLY=()\n> +\t__gitcompadd\n>  }\n>  \n>  _git_describe ()\n> @@ -1154,7 +1157,7 @@ _git_fsck ()\n>  \t\treturn\n>  \t\t;;\n>  \tesac\n> -\tCOMPREPLY=()\n> +\t__gitcompadd\n>  }\n>  \n>  _git_gc ()\n> @@ -1165,7 +1168,7 @@ _git_gc ()\n>  \t\treturn\n>  \t\t;;\n>  \tesac\n> -\tCOMPREPLY=()\n> +\t__gitcompadd\n>  }\n>  \n>  _git_gitk ()\n> @@ -1242,7 +1245,7 @@ _git_init ()\n>  \t\treturn\n>  \t\t;;\n>  \tesac\n> -\tCOMPREPLY=()\n> +\t__gitcompadd\n>  }\n>  \n>  _git_ls_files ()\n> @@ -1261,7 +1264,7 @@ _git_ls_files ()\n>  \t\treturn\n>  \t\t;;\n>  \tesac\n> -\tCOMPREPLY=()\n> +\t__gitcompadd\n>  }\n>  \n>  _git_ls_remote ()\n> @@ -1377,7 +1380,7 @@ _git_mergetool ()\n>  \t\treturn\n>  \t\t;;\n>  \tesac\n> -\tCOMPREPLY=()\n> +\t__gitcompadd\n>  }\n>  \n>  _git_merge_base ()\n> @@ -1393,7 +1396,7 @@ _git_mv ()\n>  \t\treturn\n>  \t\t;;\n>  \tesac\n> -\tCOMPREPLY=()\n> +\t__gitcompadd\n>  }\n>  \n>  _git_name_rev ()\n> @@ -1563,7 +1566,7 @@ _git_send_email ()\n>  \t\treturn\n>  \t\t;;\n>  \tesac\n> -\tCOMPREPLY=()\n> +\t__gitcompadd\n>  }\n>  \n>  _git_stage ()\n> @@ -1616,7 +1619,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> @@ -1676,7 +1679,7 @@ _git_config ()\n>  \t\treturn\n>  \t\t;;\n>  \t*.*)\n> -\t\tCOMPREPLY=()\n> +\t\t__gitcompadd\n>  \t\treturn\n>  \t\t;;\n>  \tesac\n> @@ -2056,7 +2059,7 @@ _git_remote ()\n>  \t\t__gitcomp \"$c\"\n>  \t\t;;\n>  \t*)\n> -\t\tCOMPREPLY=()\n> +\t\t__gitcompadd\n>  \t\t;;\n>  \tesac\n>  }\n> @@ -2100,7 +2103,7 @@ _git_rm ()\n>  \t\treturn\n>  \t\t;;\n>  \tesac\n> -\tCOMPREPLY=()\n> +\t__gitcompadd\n>  }\n>  \n>  _git_shortlog ()\n> @@ -2170,7 +2173,7 @@ _git_stash ()\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\t\t__gitcompadd\n>  \t\t\tfi\n>  \t\t\t;;\n>  \t\tesac\n> @@ -2183,14 +2186,14 @@ _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__gitcompadd\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__gitcompadd\n>  \t\t\t;;\n>  \t\tesac\n>  \tfi\n> @@ -2307,7 +2310,7 @@ _git_svn ()\n>  \t\t\t__gitcomp \"--revision= --parent\"\n>  \t\t\t;;\n>  \t\t*)\n> -\t\t\tCOMPREPLY=()\n> +\t\t\t__gitcompadd\n>  \t\t\t;;\n>  \t\tesac\n>  \tfi\n> @@ -2332,13 +2335,13 @@ _git_tag ()\n>  \n>  \tcase \"$prev\" in\n>  \t-m|-F)\n> -\t\tCOMPREPLY=()\n> +\t\t__gitcompadd\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\t\t__gitcompadd\n>  \t\tfi\n>  \t\t;;\n>  \t*)\n> -- \n> 1.7.12.1\n> \n"},{"id":"201434","messageId":"20121017175040.GE2156@goldbirke","threadId":"31814","inReplyTo":"1350229971-9343-3-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 2/3] tests: use __gitcompadd to simplify completion tests","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-10-17T17:50:40Z","receivedAt":"2012-10-17T17:50:40Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Sun, Oct 14, 2012 at 05:52:50PM +0200, Felipe Contreras wrote:\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  t/t9902-completion.sh | 29 +++++++++--------------------\n>  1 file changed, 9 insertions(+), 20 deletions(-)\n> \n> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n> index 92d7eb4..49c6eb4 100755\n> --- a/t/t9902-completion.sh\n> +++ b/t/t9902-completion.sh\n> @@ -39,19 +39,18 @@ _get_comp_words_by_ref ()\n>  \tdone\n>  }\n>  \n> -print_comp ()\n> +__gitcompadd ()\n>  {\n> -\tlocal IFS=$'\\n'\n> -\techo \"${COMPREPLY[*]}\" > out\n> +\tcompgen -P \"${2-}\" -S \"${4- }\" -W \"$1\" -- \"${3-$cur}\" > out\n>  }\n\nPlease don't.  Running compgen is a fundamental part of the completion\nscript, therefore tests must run it as it is in the completion script\nand not some copy of it.\n\n>  run_completion ()\n>  {\n> -\tlocal -a COMPREPLY _words\n> +\tlocal -a _words\n>  \tlocal _cword\n>  \t_words=( $1 )\n>  \t(( _cword = ${#_words[@]} - 1 ))\n> -\t__git_wrap__git_main && print_comp\n> +\t__git_wrap__git_main\n>  }\n>  \n>  test_completion ()\n> @@ -70,12 +69,10 @@ test_expect_success '__gitcomp - trailing space - options' '\n>  \t--reset-author Z\n>  \tEOF\n>  \t(\n> -\t\tlocal -a COMPREPLY &&\n\nI'm not sure what I was thinking when I wrote this, but using the\nlocal keyword while not within a function but in a subshell doesn't\nseem to be that clever ;)  Maybe just a copy-paste from the local\nvariable declarations of run-completion().\n\n>  \t\tcur=\"--re\" &&\n>  \t\t__gitcomp \"--dry-run --reuse-message= --reedit-message=\n>  \t\t\t\t--reset-author\" &&\n> -\t\tIFS=\"$newline\" &&\n> -\t\techo \"${COMPREPLY[*]}\" > out\n\nAnd here I should have used print_comp().\n\nAll these can be cleaned up without overriding __gitcompadd() and\npotentialy compromising correctness.  Will send a patch in a minute.\n\n> +\t\tIFS=\"$newline\"\n\nThis was only necessary for echoing the array.\n\n>  \t) &&\n>  \ttest_cmp expected out\n>  '\n> @@ -88,12 +85,10 @@ test_expect_success '__gitcomp - trailing space - config keys' '\n>  \tbrowser.Z\n>  \tEOF\n>  \t(\n> -\t\tlocal -a COMPREPLY &&\n>  \t\tcur=\"br\" &&\n>  \t\t__gitcomp \"branch. branch.autosetupmerge\n>  \t\t\t\tbranch.autosetuprebase browser.\" &&\n> -\t\tIFS=\"$newline\" &&\n> -\t\techo \"${COMPREPLY[*]}\" > out\n> +\t\tIFS=\"$newline\"\n>  \t) &&\n>  \ttest_cmp expected out\n>  '\n> @@ -104,12 +99,10 @@ test_expect_success '__gitcomp - option parameter' '\n>  \tresolve Z\n>  \tEOF\n>  \t(\n> -\t\tlocal -a COMPREPLY &&\n>  \t\tcur=\"--strategy=re\" &&\n>  \t\t__gitcomp \"octopus ours recursive resolve subtree\n>  \t\t\t\" \"\" \"re\" &&\n> -\t\tIFS=\"$newline\" &&\n> -\t\techo \"${COMPREPLY[*]}\" > out\n> +\t\tIFS=\"$newline\"\n>  \t) &&\n>  \ttest_cmp expected out\n>  '\n> @@ -120,12 +113,10 @@ test_expect_success '__gitcomp - prefix' '\n>  \tbranch.maint.mergeoptions Z\n>  \tEOF\n>  \t(\n> -\t\tlocal -a COMPREPLY &&\n>  \t\tcur=\"branch.me\" &&\n>  \t\t__gitcomp \"remote merge mergeoptions rebase\n>  \t\t\t\" \"branch.maint.\" \"me\" &&\n> -\t\tIFS=\"$newline\" &&\n> -\t\techo \"${COMPREPLY[*]}\" > out\n> +\t\tIFS=\"$newline\"\n>  \t) &&\n>  \ttest_cmp expected out\n>  '\n> @@ -136,12 +127,10 @@ test_expect_success '__gitcomp - suffix' '\n>  \tbranch.maint.Z\n>  \tEOF\n>  \t(\n> -\t\tlocal -a COMPREPLY &&\n>  \t\tcur=\"branch.me\" &&\n>  \t\t__gitcomp \"master maint next pu\n>  \t\t\t\" \"branch.\" \"ma\" \".\" &&\n> -\t\tIFS=\"$newline\" &&\n> -\t\techo \"${COMPREPLY[*]}\" > out\n> +\t\tIFS=\"$newline\"\n>  \t) &&\n>  \ttest_cmp expected out\n>  '\n> -- \n> 1.7.12.1\n> \n"},{"id":"201435","messageId":"20121017175428.GF2156@goldbirke","threadId":"31814","inReplyTo":"20121017175040.GE2156@goldbirke","subject":"[PATCH] completion: clean up __gitcomp() tests","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-10-17T17:54:28Z","receivedAt":"2012-10-17T17:54:28Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Clean up two issues in the tests I added in 74a8c849 (tests: add tests\nfor the __gitcomp() completion helper function, 2012-04-17):\n\n - The COMPREPLY array is created using 'local -a' while in a\n   subshell.  However, the local keyword should only be used in a\n   shell function, and a variable created in a subshell is by\n   definition local to that subshell.  Use 'declare -a' instead.\n\n - The contents of the COMPREPLY array is written through an IFS\n   fiddling + echo + redirection combo, although there is the\n   print_comp() helper function for exactly this purpose.\n\nSigned-off-by: SZEDER Gábor <szeder@ira.uka.de>\n---\n t/t9902-completion.sh | 27 ++++++++++-----------------\n 1 file changed, 10 insertions(+), 17 deletions(-)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex cbd0fb66..cc375ed0 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -70,8 +70,6 @@ test_completion_long ()\n \ttest_completion \"$1\"\n }\n \n-newline=$'\\n'\n-\n test_expect_success '__gitcomp - trailing space - options' '\n \tsed -e \"s/Z$//\" >expected <<-\\EOF &&\n \t--reuse-message=Z\n@@ -79,12 +77,11 @@ test_expect_success '__gitcomp - trailing space - options' '\n \t--reset-author Z\n \tEOF\n \t(\n-\t\tlocal -a COMPREPLY &&\n+\t\tdeclare -a COMPREPLY &&\n \t\tcur=\"--re\" &&\n \t\t__gitcomp \"--dry-run --reuse-message= --reedit-message=\n \t\t\t\t--reset-author\" &&\n-\t\tIFS=\"$newline\" &&\n-\t\techo \"${COMPREPLY[*]}\" > out\n+\t\tprint_comp\n \t) &&\n \ttest_cmp expected out\n '\n@@ -97,12 +94,11 @@ test_expect_success '__gitcomp - trailing space - config keys' '\n \tbrowser.Z\n \tEOF\n \t(\n-\t\tlocal -a COMPREPLY &&\n+\t\tdeclare -a COMPREPLY &&\n \t\tcur=\"br\" &&\n \t\t__gitcomp \"branch. branch.autosetupmerge\n \t\t\t\tbranch.autosetuprebase browser.\" &&\n-\t\tIFS=\"$newline\" &&\n-\t\techo \"${COMPREPLY[*]}\" > out\n+\t\tprint_comp\n \t) &&\n \ttest_cmp expected out\n '\n@@ -113,12 +109,11 @@ test_expect_success '__gitcomp - option parameter' '\n \tresolve Z\n \tEOF\n \t(\n-\t\tlocal -a COMPREPLY &&\n+\t\tdeclare -a COMPREPLY &&\n \t\tcur=\"--strategy=re\" &&\n \t\t__gitcomp \"octopus ours recursive resolve subtree\n \t\t\t\" \"\" \"re\" &&\n-\t\tIFS=\"$newline\" &&\n-\t\techo \"${COMPREPLY[*]}\" > out\n+\t\tprint_comp\n \t) &&\n \ttest_cmp expected out\n '\n@@ -129,12 +124,11 @@ test_expect_success '__gitcomp - prefix' '\n \tbranch.maint.mergeoptions Z\n \tEOF\n \t(\n-\t\tlocal -a COMPREPLY &&\n+\t\tdeclare -a COMPREPLY &&\n \t\tcur=\"branch.me\" &&\n \t\t__gitcomp \"remote merge mergeoptions rebase\n \t\t\t\" \"branch.maint.\" \"me\" &&\n-\t\tIFS=\"$newline\" &&\n-\t\techo \"${COMPREPLY[*]}\" > out\n+\t\tprint_comp\n \t) &&\n \ttest_cmp expected out\n '\n@@ -145,12 +139,11 @@ test_expect_success '__gitcomp - suffix' '\n \tbranch.maint.Z\n \tEOF\n \t(\n-\t\tlocal -a COMPREPLY &&\n+\t\tdeclare -a COMPREPLY &&\n \t\tcur=\"branch.me\" &&\n \t\t__gitcomp \"master maint next pu\n \t\t\t\" \"branch.\" \"ma\" \".\" &&\n-\t\tIFS=\"$newline\" &&\n-\t\techo \"${COMPREPLY[*]}\" > out\n+\t\tprint_comp\n \t) &&\n \ttest_cmp expected out\n '\n-- \n1.8.0.rc0.83.gc8e1777\n"},{"id":"201438","messageId":"CAMP44s2Daj-yvTqKc-fMiqvA64z_odZy_dhGvtB7rM--LbuQRQ@mail.gmail.com","threadId":"31814","inReplyTo":"20121017175428.GF2156@goldbirke","subject":"Re: [PATCH] completion: clean up __gitcomp() tests","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-10-17T18:21:21Z","receivedAt":"2012-10-17T18:21:21Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Oct 17, 2012 at 7:54 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:\n> Clean up two issues in the tests I added in 74a8c849 (tests: add tests\n> for the __gitcomp() completion helper function, 2012-04-17):\n>\n>  - The COMPREPLY array is created using 'local -a' while in a\n>    subshell.  However, the local keyword should only be used in a\n>    shell function, and a variable created in a subshell is by\n>    definition local to that subshell.  Use 'declare -a' instead.\n>\n>  - The contents of the COMPREPLY array is written through an IFS\n>    fiddling + echo + redirection combo, although there is the\n>    print_comp() helper function for exactly this purpose.\n\nMakes sense. But this code seems awfully similar, a helper function might help.\n\n-- \nFelipe Contreras\n"},{"id":"201439","messageId":"CAMP44s1L_0FS4jrdb4aFxgPENrO1ySMXhCqJt3QiODhbuTPFQA@mail.gmail.com","threadId":"31814","inReplyTo":"20121017175040.GE2156@goldbirke","subject":"Re: [PATCH v5 2/3] tests: use __gitcompadd to simplify completion tests","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-10-17T18:26:49Z","receivedAt":"2012-10-17T18:26:49Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Oct 17, 2012 at 7:50 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:\n> On Sun, Oct 14, 2012 at 05:52:50PM +0200, Felipe Contreras wrote:\n>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n>> ---\n>>  t/t9902-completion.sh | 29 +++++++++--------------------\n>>  1 file changed, 9 insertions(+), 20 deletions(-)\n>>\n>> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n>> index 92d7eb4..49c6eb4 100755\n>> --- a/t/t9902-completion.sh\n>> +++ b/t/t9902-completion.sh\n>> @@ -39,19 +39,18 @@ _get_comp_words_by_ref ()\n>>       done\n>>  }\n>>\n>> -print_comp ()\n>> +__gitcompadd ()\n>>  {\n>> -     local IFS=$'\\n'\n>> -     echo \"${COMPREPLY[*]}\" > out\n>> +     compgen -P \"${2-}\" -S \"${4- }\" -W \"$1\" -- \"${3-$cur}\" > out\n>>  }\n>\n> Please don't.  Running compgen is a fundamental part of the completion\n> script, therefore tests must run it as it is in the completion script\n> and not some copy of it.\n\nAll right. I added this patch as an after though to help sell the idea\nof __gitcompadd. Either  way I'm not to worried about overriding it,\nwe are not really exercising any code that could catch issues with\ncalling compgen; we probably need specialized tests for that. In fact\nI amended the quote you are quoting above as it's totally different\nfrom the proposed __gitcompadd, but it still works nonetheless.\n\n-- \nFelipe Contreras\n"},{"id":"201672","messageId":"CAMP44s3guAhFe1ACrAzC9T05fCMAejU6Wn+GtYfSpmfJ8EX-xg@mail.gmail.com","threadId":"31814","inReplyTo":"20121017172808.GD2156@goldbirke","subject":"Re: [PATCH v5 1/3] completion: add new __gitcompadd helper","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-10-22T00:41:21Z","receivedAt":"2012-10-22T00:41:21Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Oct 17, 2012 at 7:28 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:\n> On Sun, Oct 14, 2012 at 05:52:49PM +0200, Felipe Contreras wrote:\n\n>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n>> index d743e56..01325de 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>> +     COMPREPLY=($(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>> @@ -238,13 +243,11 @@ __gitcomp ()\n>>\n>>       case \"$cur_\" in\n>>       --*=)\n>> -             COMPREPLY=()\n>> +             __gitcompadd\n>>               ;;\n>>       *)\n>>               local IFS=$'\\n'\n>> -             COMPREPLY=($(compgen -P \"${2-}\" \\\n>> -                     -W \"$(__gitcomp_1 \"${1-}\" \"${4-}\")\" \\\n>> -                     -- \"$cur_\"))\n>> +             __gitcompadd \"$(__gitcomp_1 \"${1-}\" \"${4-}\")\" \"${2-}\" \"$cur_\" \"\"\n>>               ;;\n>>       esac\n>>  }\n>> @@ -261,7 +264,7 @@ __gitcomp ()\n>>  __gitcomp_nl ()\n>>  {\n>>       local IFS=$'\\n'\n>> -     COMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$1\" -- \"${3-$cur}\"))\n>> +     __gitcompadd \"$1\" \"${2-}\" \"${3-$cur}\" \"${4- }\"\n>>  }\n>\n> I feel hesitant about this change.  One of the ways I'm exploring to\n> fix the issues with shell metacharacters and expansion in compgen is\n> to actually replace compgen.  We already iterate over all possible\n> completion words in __gitcomp_1(), so it doesn't make much of a\n> difference to do the filtering for the current word while we are at\n> it.  However, the way __gitcompadd() encapsulates COMPREPLY=($(compgen\n> ...)), and tha basic idea of never touching COMPREPLY directly make\n> this basically impossible.\n\nHow is it impossible? You can still replace compgen, all you have to\ndo is modify __gitcompadd and replace that code with whatever custom\ncode you want. You can change the arguments and everything. The only\nlimitation is that it should be the only place where COMPREPLY is\nmodified, and all is good. Well, it doesn't have to be only _one_\nplace, but the less functions that do this, the better.\n\n>>  __git_heads ()\n>> @@ -486,7 +489,7 @@ __git_complete_remote_or_refspec ()\n>>                       case \"$cmd\" in\n>>                       push) no_complete_refspec=1 ;;\n>>                       fetch)\n>> -                             COMPREPLY=()\n>> +                             __gitcompadd\n>>                               return\n>>                               ;;\n>>                       *) ;;\n>> @@ -502,7 +505,7 @@ __git_complete_remote_or_refspec ()\n>>               return\n>>       fi\n>>       if [ $no_complete_refspec = 1 ]; then\n>> -             COMPREPLY=()\n>> +             __gitcompadd\n>>               return\n>>       fi\n>>       [ \"$remote\" = \".\" ] && remote=\n>> @@ -776,7 +779,7 @@ _git_am ()\n>>                       \"\n>>               return\n>>       esac\n>> -     COMPREPLY=()\n>> +     __gitcompadd\n>\n> These changes effectively run compgen in a subshell to generate an\n> empty completion reply.  While it doesn't really matter on Linux,\n> it'll add another half a tenth of a second delay in those cases on my\n> Windows machine.  At least it should be conditional, i.e. $(compgen\n> ...) shouldn't be executed when there are no possible completion\n> words.\n>\n> However, I think those COMPREPLY=() assignments are pointless anyway.\n> COMPREPLY is always empty when completion functions are invoked, so\n> there is no need to explicitly set it to an empty array when we don't\n> provide any words for completion.  Their only use is basically to\n> explicitly tell us humans that in those cases we don't offer any words\n> for completion.  But we don't do that consistently: there are several\n> places without offering words for completion and without COMPREPLY=(),\n> e.g. the '__git_has_doubledash && return' pattern.\n>\n> Perhaps it would be time to get rid of these COMPREPLY=() assignments?\n\nI'm all for it, I never understood what was the purpose of that. I\nbelieve zsh could benefit from this information to decide whether to\nrun the default completion (e.g. files) or not, but as you said, if\nit's not used consistently for bash, there's no point in trying.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"202268","messageId":"20121030231832.GB15709@goldbirke","threadId":"31814","inReplyTo":"CAMP44s3guAhFe1ACrAzC9T05fCMAejU6Wn+GtYfSpmfJ8EX-xg@mail.gmail.com","subject":"Re: [PATCH v5 1/3] completion: add new __gitcompadd helper","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-10-30T23:18:32Z","receivedAt":"2012-10-30T23:18:32Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Oct 22, 2012 at 02:41:21AM +0200, Felipe Contreras wrote:\n> On Wed, Oct 17, 2012 at 7:28 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:\n> > On Sun, Oct 14, 2012 at 05:52:49PM +0200, Felipe Contreras wrote:\n> \n> >> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> >> index d743e56..01325de 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> >> +     COMPREPLY=($(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> >> @@ -238,13 +243,11 @@ __gitcomp ()\n> >>\n> >>       case \"$cur_\" in\n> >>       --*=)\n> >> -             COMPREPLY=()\n> >> +             __gitcompadd\n> >>               ;;\n> >>       *)\n> >>               local IFS=$'\\n'\n> >> -             COMPREPLY=($(compgen -P \"${2-}\" \\\n> >> -                     -W \"$(__gitcomp_1 \"${1-}\" \"${4-}\")\" \\\n> >> -                     -- \"$cur_\"))\n> >> +             __gitcompadd \"$(__gitcomp_1 \"${1-}\" \"${4-}\")\" \"${2-}\" \"$cur_\" \"\"\n> >>               ;;\n> >>       esac\n> >>  }\n> >> @@ -261,7 +264,7 @@ __gitcomp ()\n> >>  __gitcomp_nl ()\n> >>  {\n> >>       local IFS=$'\\n'\n> >> -     COMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$1\" -- \"${3-$cur}\"))\n> >> +     __gitcompadd \"$1\" \"${2-}\" \"${3-$cur}\" \"${4- }\"\n> >>  }\n> >\n> > I feel hesitant about this change.  One of the ways I'm exploring to\n> > fix the issues with shell metacharacters and expansion in compgen is\n> > to actually replace compgen.  We already iterate over all possible\n> > completion words in __gitcomp_1(), so it doesn't make much of a\n> > difference to do the filtering for the current word while we are at\n> > it.  However, the way __gitcompadd() encapsulates COMPREPLY=($(compgen\n> > ...)), and tha basic idea of never touching COMPREPLY directly make\n> > this basically impossible.\n> \n> How is it impossible? You can still replace compgen, all you have to\n> do is modify __gitcompadd and replace that code with whatever custom\n> code you want. You can change the arguments and everything. The only\n> limitation is that it should be the only place where COMPREPLY is\n> modified, and all is good. Well, it doesn't have to be only _one_\n> place, but the less functions that do this, the better.\n\nThat's exactly the problem: there isn't, there can't be one single\n\"whatever custom code I want\".\n\nThe compgen() in __gitcomp() will be replaced by an enhanced version\nof the loop in __gitcomp_1(), while in __gitcomp_nl() it will be\nreplaced by a little awk scriptlet.  And then there is the oddball\n$(git ls-tree |sed magic) in __git_complete_revlist_file(), where\npossible completion words are filenames possibly containing newlines,\ntherefore requiring yet another approach.\n"}]}