{"thread":{"id":"29352","subject":"Zsh completion regression","startedAt":"2012-01-12T11:52:27Z","lastAt":"2012-01-25T04:06:38Z","messageCount":21,"participants":["Stefan Haller","Matthieu Moy","SZEDER Gábor","Junio C Hamano","Felipe Contreras"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"182428","messageId":"1kdr5xk.1sopzul1hygnbrM%lists@haller-berlin.de","threadId":"29352","inReplyTo":null,"subject":"Zsh completion regression","fromName":"Stefan Haller","fromEmail":"lists@haller-berlin.de","sentAt":"2012-01-12T11:52:27Z","receivedAt":"2012-01-12T11:52:27Z","isPatch":false,"sender":{"key":"lists@haller-berlin.de","avatar":null},"body":"I'm using zsh 4.3.11.\n\nWhen I type \"git log mas\", it completes to \"git log master\\ \" (a\nbackslash, a space, and then the cursor).\n\nBisecting points to \"a31e626 completion: optimize refs completion.\"\nBefore this commit, I get \"git log master\" (with no space at the end).\n\nMy completion-fu is not strong enough to dig into this myself, but I'll\nbe happy to help test, or do whatever else helps.\n\n\n-- \nStefan Haller\nBerlin, Germany\nhttp://www.haller-berlin.de/\n"},{"id":"182434","messageId":"vpq62ghj7fn.fsf@bauges.imag.fr","threadId":"29352","inReplyTo":"1kdr5xk.1sopzul1hygnbrM%lists@haller-berlin.de","subject":"Re: Zsh completion regression","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-01-12T14:56:28Z","receivedAt":"2012-01-12T14:56:28Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"lists@haller-berlin.de (Stefan Haller) writes:\n\n> I'm using zsh 4.3.11.\n>\n> When I type \"git log mas\", it completes to \"git log master\\ \" (a\n> backslash, a space, and then the cursor).\n\nSame here (although I've been too lazy to bisect myself).\n\nThe following patch makes the situation better, but is not really a fix:\n\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -525,7 +525,7 @@ __gitcomp ()\n __gitcomp_nl ()\n {\n local s=$'\\n' IFS=' '$'\\t'$'\\n'\n- local cur_=\"$cur\" suffix=\" \"\n+ local cur_=\"$cur\" suffix=\"\"\n \n if [ $# -gt 2 ]; then\n cur_=\"$3\"\n\nWith this, the trailing space isn't added, but e.g. \"git checkout\nmaster\" does not add the trailing space, at all.\n\nThe problem is a little bit below:\n\n\tIFS=$s\n\tCOMPREPLY=($(compgen -P \"${2-}\" -S \"$suffix\" -W \"$1\" -- \"$cur_\"))\n\nThe -S \"$suffix\" adds a space to the completion, but ZSH escapes the\nspace (which sounds sensible in general, but is not at all what we\nexpect). My completion-fu isn't good enough to get any further either\nunfortunately.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"182551","messageId":"20120114132343.GW30469@goldbirke","threadId":"29352","inReplyTo":"vpq62ghj7fn.fsf@bauges.imag.fr","subject":"Re: Zsh completion regression","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-01-14T13:23:43Z","receivedAt":"2012-01-14T13:23:43Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Hi,\n\n\nOn Thu, Jan 12, 2012 at 03:56:28PM +0100, Matthieu Moy wrote:\n> lists@haller-berlin.de (Stefan Haller) writes:\n> \n> > I'm using zsh 4.3.11.\n> >\n> > When I type \"git log mas\", it completes to \"git log master\\ \" (a\n> > backslash, a space, and then the cursor).\n\nStefan, thanks for reporting and bisecting.\n\n> The following patch makes the situation better, but is not really a fix:\n> \n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -525,7 +525,7 @@ __gitcomp ()\n> __gitcomp_nl ()\n> {\n> local s=$'\\n' IFS=' '$'\\t'$'\\n'\n> - local cur_=\"$cur\" suffix=\" \"\n> + local cur_=\"$cur\" suffix=\"\"\n> \n> if [ $# -gt 2 ]; then\n> cur_=\"$3\"\n> \n> With this, the trailing space isn't added,\n\nYeah, this would be a regression for Bash users, because then they\nwill need to manually append that space.\n\n> but e.g. \"git checkout\n> master\" does not add the trailing space, at all.\n\nI'm not sure what you mean; did you got a trailing space after\n'master' before a31e6262 (completion: optimize refs completion,\n2011-10-15)?\nI'm not a zsh user myself, but just tried it and found that in\na31e626^ 'git checkout master' doesn't add the trailing space,\nand neither does 'git checkout mas', which is in sync with what\nStefan reported (\"Before this commit, I get \"git log master\" (with no\nspace at the end).\").\n\n> The problem is a little bit below:\n> \n> \tIFS=$s\n> \tCOMPREPLY=($(compgen -P \"${2-}\" -S \"$suffix\" -W \"$1\" -- \"$cur_\"))\n> \n> The -S \"$suffix\" adds a space to the completion, but ZSH escapes the\n> space (which sounds sensible in general, but is not at all what we\n> expect).\n\nSo it seems we have two issues here with zsh:\n\n- Spaces appended with 'compgen -S \" \"' in __gitcomp_nl() are quoted.\n This is the regression caused by a31e6262.\n- Spaces appended in __gitcomp_1() (by echo \"$word \") are stripped.\n This is a long-standing issue; if those spaces weren't stripped, the\n quoting issue would have been noted earlier, I suspect.\n\nWe could fix the regression by not appending a space suffix to\ncompletion words in __gitcomp_nl(), but only when the completion\nscript is running under zsh to avoid hurting bash users, like this:\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 2d02a7f3..49393243 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -601,6 +601,9 @@ __gitcomp_nl ()\n \t\t\tsuffix=\"$4\"\n \t\tfi\n \tfi\n+\tif [ -n \"${ZSH_VERSION-}\" ] && [ \"$suffix\" = \" \" ]; then\n+\t\tsuffix=\"\"\n+\tfi\n \n \tIFS=$s\n \tCOMPREPLY=($(compgen -P \"${2-}\" -S \"$suffix\" -W \"$1\" -- \"$cur_\"))\n\n\nAlthough it wouldn't append a space at all for zsh users, that's\nnothing new, it's just restoring old behavior. And then later someone\nmore knowledgable about the quirks of zsh's completion and in\nparticular zsh's bash completion emulation can come up with a proper\nfix to the issue of quoted spaces and stripped trailing spaces.\n\n\nHowever.\n\nWhile playing around with zsh, I noticed that git completion works\nwithout even sourcing git's bash completion script. As it turned out,\nzsh ships its own git completion script[1], and from my cursory tests\nit seems to be quite capable: it supports commands, aliases, options,\nrefs, ref1..ref2, config variables, ... and I also saw a __git_ps1()\nequivalent for zsh.\n\nSo, is there any reason why you are still using git's bash completion\nunder zsh (which has some quirks and lacks some features) instead of\nzsh's own? Perhaps it would make sense to point zsh users to zsh's\ngit completion and drop zsh compatibility from git's bash completion.\nWe did similar with vim config files: git included a vim syntax\nhighlight config file for commit messages under contrib/vim/, but\neventually we dropped it after vim started shipping more capable\ngit-specific config files (for git config files, rebase instruction\nsheets, etc.).\n\n\n[1] http://zsh.git.sourceforge.net/git/gitweb.cgi?p=zsh/zsh;a=blob;f=Completion/Unix/Command/_git;hb=HEAD\n\n\nBest,\nGábor\n"},{"id":"182554","messageId":"vpq7h0ufj87.fsf@bauges.imag.fr","threadId":"29352","inReplyTo":"20120114132343.GW30469@goldbirke","subject":"Re: Zsh completion regression","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-01-14T14:32:08Z","receivedAt":"2012-01-14T14:32:08Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"SZEDER Gábor writes:\n\n>> but e.g. \"git checkout\n>> master\" does not add the trailing space, at all.\n>\n> I'm not sure what you mean; did you got a trailing space after\n> 'master' before a31e6262 (completion: optimize refs completion,\n> 2011-10-15)?\n\nNo. My above sentence should read \"... not add the trailing space, at\nall, even for bash users\". IOW, your understanding is correct.\n\n> We could fix the regression by not appending a space suffix to\n> completion words in __gitcomp_nl(), but only when the completion\n> script is running under zsh to avoid hurting bash users, like this:\n>\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 2d02a7f3..49393243 100755\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -601,6 +601,9 @@ __gitcomp_nl ()\n> \t\t\tsuffix=\"$4\"\n> \t\tfi\n> \tfi\n> +\tif [ -n \"${ZSH_VERSION-}\" ] && [ \"$suffix\" = \" \" ]; then\n> +\t\tsuffix=\"\"\n> +\tfi\n> \n> \tIFS=$s\n> \tCOMPREPLY=($(compgen -P \"${2-}\" -S \"$suffix\" -W \"$1\" -- \"$cur_\"))\n\nI hate to see special case for different shells, but if no one finds a\nbetter solution, then yes, this is the way to go. Not having the space\nmay be irritating, but having the quoted space hurts really much more (I\nhave to delete the space and the backslash manually to continue).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"182555","messageId":"20120114152030.GX30469@goldbirke","threadId":"29352","inReplyTo":"vpq7h0ufj87.fsf@bauges.imag.fr","subject":"Re: Zsh completion regression","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-01-14T15:20:30Z","receivedAt":"2012-01-14T15:20:30Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Hi,\n\n\nOn Sat, Jan 14, 2012 at 03:32:08PM +0100, Matthieu Moy wrote:\n> SZEDER Gábor writes:\n> > We could fix the regression by not appending a space suffix to\n> > completion words in __gitcomp_nl(), but only when the completion\n> > script is running under zsh to avoid hurting bash users, like this:\n> >\n> > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> > index 2d02a7f3..49393243 100755\n> > --- a/contrib/completion/git-completion.bash\n> > +++ b/contrib/completion/git-completion.bash\n> > @@ -601,6 +601,9 @@ __gitcomp_nl ()\n> > \t\t\tsuffix=\"$4\"\n> > \t\tfi\n> > \tfi\n> > +\tif [ -n \"${ZSH_VERSION-}\" ] && [ \"$suffix\" = \" \" ]; then\n> > +\t\tsuffix=\"\"\n> > +\tfi\n> > \n> > \tIFS=$s\n> > \tCOMPREPLY=($(compgen -P \"${2-}\" -S \"$suffix\" -W \"$1\" -- \"$cur_\"))\n> \n> I hate to see special case for different shells,\n\nI agree and thought about that, too. We are dealing with zsh quirks\nin the completion script either by\n\n- having whole functions with different definitions in zsh and\n in bash (currently__git_shopt() and _get_comp_words_by_ref()), or \n- having similar shell-specific cases inside functions (currently\n _git() and _gitk()).\n\nWe use the former when the implementations for the two shells differ\nsignificantly, and the latter when the shell-specific parts are small\nand most of the function's implementation can be used in both shells.\nI think this regression fix fits into the latter category.\n\n> but if no one finds a\n> better solution, then yes, this is the way to go. Not having the space\n> may be irritating, but having the quoted space hurts really much more (I\n> have to delete the space and the backslash manually to continue).\n\nI have no better idea for fixing this regression, and no idea at all\nfor a proper fix (i.e. to make zsh behave the same way in this respect\nas bash). I can imagine that it's irritating, that's why I aimed for\nthis minimal regression fix, which, maybe with a Tested-by from you or\nStefan, can even go into maint, so affected users can get it faster.\n\n\nBest,\nGábor\n"},{"id":"182560","messageId":"1326567336-2173-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"29352","inReplyTo":"20120114152030.GX30469@goldbirke","subject":"[PATCH] bash-completion: don't add quoted space for ZSH (fix regression)","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2012-01-14T18:55:36Z","receivedAt":"2012-01-14T18:55:36Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Commit a31e626 (completion: optimize refs completion) introduced a\nregression for ZSH users: ref names were completed with a quoted trailing\nspace (i.e. \"git checkout ma\" completes to \"git checkout master\\ \"). The\nspace is convenient for bash users since we use \"-o nospace\", but a\nquoted space is worse than nothing. The absence of trailing space for ZSH\nis a long-standing issue, that this patch is not fixing. We just fix the\nregression by not appending a space when the shell is ZSH.\n\nOriginal-patch-by: SZEDER Gábor \nReported-by: Stefan Haller \nSigned-off-by: Matthieu Moy \n---\n contrib/completion/git-completion.bash | 6 ++++++\n 1 files changed, 6 insertions(+), 0 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex b0062ba..488e1f4 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -534,6 +534,12 @@ __gitcomp_nl ()\n \t\tfi\n \tfi\n \n+\t# ZSH would quote the trailing space added with -S. bash users\n+\t# will appreciate the extra space to compensate the use of -o nospace.\n+\tif [ -n \"${ZSH_VERSION-}\" ] && [ \"$suffix\" = \" \" ]; then\n+\t\tsuffix=\"\"\n+\tfi\n+\n \tIFS=$s\n \tCOMPREPLY=($(compgen -P \"${2-}\" -S \"$suffix\" -W \"$1\" -- \"$cur_\"))\n }\n-- \n1.7.9.rc0.25.gb9a1f.dirty\n"},{"id":"182565","messageId":"1kdvsqu.foqepp1ofdg7nM%lists@haller-berlin.de","threadId":"29352","inReplyTo":"20120114152030.GX30469@goldbirke","subject":"Re: Zsh completion regression","fromName":"Stefan Haller","fromEmail":"lists@haller-berlin.de","sentAt":"2012-01-14T21:36:31Z","receivedAt":"2012-01-14T21:36:31Z","isPatch":false,"sender":{"key":"lists@haller-berlin.de","avatar":null},"body":"SZEDER Gábor wrote:\n\n> I can imagine that it's irritating, that's why I aimed for\n> this minimal regression fix, which, maybe with a Tested-by from you or\n> Stefan, can even go into maint, so affected users can get it faster.\n\nThe patch works well for me, so feel free to add a Tested-by from me.\n\n\n-- \nStefan Haller\nBerlin, Germany\nhttp://www.haller-berlin.de/\n"},{"id":"182566","messageId":"1kdvsvw.10asu1tpmtz3qM%lists@haller-berlin.de","threadId":"29352","inReplyTo":"20120114132343.GW30469@goldbirke","subject":"Re: Zsh completion regression","fromName":"Stefan Haller","fromEmail":"lists@haller-berlin.de","sentAt":"2012-01-14T21:36:38Z","receivedAt":"2012-01-14T21:36:38Z","isPatch":false,"sender":{"key":"lists@haller-berlin.de","avatar":null},"body":"SZEDER Gábor wrote:\n\n> However.\n> \n> While playing around with zsh, I noticed that git completion works\n> without even sourcing git's bash completion script. As it turned out,\n> zsh ships its own git completion script[1], and from my cursory tests\n> it seems to be quite capable: it supports commands, aliases, options,\n> refs, ref1..ref2, config variables, ... and I also saw a __git_ps1()\n> equivalent for zsh.\n> \n> So, is there any reason why you are still using git's bash completion\n> under zsh (which has some quirks and lacks some features) instead of\n> zsh's own? Perhaps it would make sense to point zsh users to zsh's\n> git completion and drop zsh compatibility from git's bash completion.\n> We did similar with vim config files: git included a vim syntax\n> highlight config file for commit messages under contrib/vim/, but\n> eventually we dropped it after vim started shipping more capable\n> git-specific config files (for git config files, rebase instruction\n> sheets, etc.).\n\nLast time I compared the two, the bash completion script was a lot more\ncomplete and powerful. The zsh script had a few annoying limitations for\nthings that I use every day, and that worked with the bash script, so I\nswitched to that. Unfortunately I forgot the details, it's a while ago.\n\n\n-- \nStefan Haller\nBerlin, Germany\nhttp://www.haller-berlin.de/\n"},{"id":"182569","messageId":"7vsjjhwvdy.fsf@alter.siamese.dyndns.org","threadId":"29352","inReplyTo":"1326567336-2173-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH] bash-completion: don't add quoted space for ZSH (fix regression)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-15T02:29:45Z","receivedAt":"2012-01-15T02:29:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index b0062ba..488e1f4 100755\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -534,6 +534,12 @@ __gitcomp_nl ()\n> \t\tfi\n> \tfi\n> \n> +\t# ZSH would quote the trailing space added with -S. bash users\n> +\t# will appreciate the extra space to compensate the use of -o nospace.\n> +\tif [ -n \"${ZSH_VERSION-}\" ] && [ \"$suffix\" = \" \" ]; then\n> +\t\tsuffix=\"\"\n> +\tfi\n\nThis should take care of the SP set by the beginning of the helper\nfunction\n\n local cur_=\"$cur\" suffix=\" \"\n\nbut is that the right thing to do if suffix came from \"$4\"?\n\nAs far as I can see, \"$4\" is used to append \".\" in very limited cases, and\nnobody explicitly passes SP as \"$4\" when calling this, so it may be easier\nto read if you moved this before that \"if we have 3 or more args, use the\nfourth one as the suffix\" block, i.e. something like this?\n\n contrib/completion/git-completion.bash | 5 ++++-\n 1 files changed, 4 insertions(+), 1 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex b0062ba..4ad75ed 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -525,7 +525,10 @@ __gitcomp ()\n __gitcomp_nl ()\n {\n \tlocal s=$'\\n' IFS=' '$'\\t'$'\\n'\n-\tlocal cur_=\"$cur\" suffix=\" \"\n+\tlocal cur_=\"$cur\"\n+\t# Because we use '-o nospace' under bash, we need to compensate\n+\t# for it by appending SP after completed word ourselves.\n+\tlocal suffix=\"${BASH_VERSION+ }\"\n \n \tif [ $# -gt 2 ]; then\n \t\tcur_=\"$3\"\n"},{"id":"182613","messageId":"vpqhazv3m17.fsf@bauges.imag.fr","threadId":"29352","inReplyTo":"7vsjjhwvdy.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] bash-completion: don't add quoted space for ZSH (fix regression)","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-01-16T11:49:08Z","receivedAt":"2012-01-16T11:49:08Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> but is that the right thing to do if suffix came from \"$4\"?\n>\n> As far as I can see, \"$4\" is used to append \".\" in very limited cases, and\n> nobody explicitly passes SP as \"$4\" when calling this, so it may be easier\n> to read if you moved this before that \"if we have 3 or more args, use the\n> fourth one as the suffix\" block, i.e. something like this?\n\nWhy not, but in case someone explicitely passes \" \" as $4 in the future,\nit's likely to be better to strip it for the same reason we strip it here.\n\nI don't care much either way in this case.\n\n> +\t# Because we use '-o nospace' under bash, we need to compensate\n> +\t# for it by appending SP after completed word ourselves.\n> +\tlocal suffix=\"${BASH_VERSION+ }\"\n\nNot sure why you reworded the comment, but I don't think it's a good\nidea to remove the \"ZSH would quote the trailing space added with -S\"\nthat I had added, because this is really the reason we do a special case\nhere. Your version is misleading, because we use -o nospace for ZSH too.\n\nSo, overall, I prefer my version ;-).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"182641","messageId":"7vsjjfuuwk.fsf@alter.siamese.dyndns.org","threadId":"29352","inReplyTo":"vpqhazv3m17.fsf@bauges.imag.fr","subject":"Re: [PATCH] bash-completion: don't add quoted space for ZSH (fix regression)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-16T22:47:39Z","receivedAt":"2012-01-16T22:47:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Junio C Hamano writes:\n>\n>> but is that the right thing to do if suffix came from \"$4\"?\n>>\n>> As far as I can see, \"$4\" is used to append \".\" in very limited cases, and\n>> nobody explicitly passes SP as \"$4\" when calling this, so it may be easier\n>> to read if you moved this before that \"if we have 3 or more args, use the\n>> fourth one as the suffix\" block, i.e. something like this?\n>\n> Why not, but in case someone explicitely passes \" \" as $4 in the future,\n> it's likely to be better to strip it for the same reason we strip it here.\n\nI doubt that would be sufficent or appropriate. If some caller _WANTS_ to\nadd a SP, shouldn't we be devising a way to tell zsh to add it without\nquoting, instead of silently stripping?\n\n> I don't care much either way in this case.\n>\n>> +\t# Because we use '-o nospace' under bash, we need to compensate\n>> +\t# for it by appending SP after completed word ourselves.\n>> +\tlocal suffix=\"${BASH_VERSION+ }\"\n>\n> Not sure why you reworded the comment, but I don't think it's a good\n> idea to remove the \"ZSH would quote the trailing space added with -S\"\n> that I had added, because this is really the reason we do a special case\n> here. Your version is misleading, because we use -o nospace for ZSH too.\n\nOk, use of \"-o nospace\" in Zsh is what I missed. I thought the issue was\nabout the nospace emulation.\n\nSo does that mean we would be forcing zsh users to add SP themselves? I\nwonder if we can do better than that.\n"},{"id":"182682","messageId":"vpqr4yy1pv6.fsf@bauges.imag.fr","threadId":"29352","inReplyTo":"7vsjjfuuwk.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] bash-completion: don't add quoted space for ZSH (fix regression)","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-01-17T12:21:33Z","receivedAt":"2012-01-17T12:21:33Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> Matthieu Moy writes:\n>\n>> Junio C Hamano writes:\n>>\n>>> but is that the right thing to do if suffix came from \"$4\"?\n>>>\n>>> As far as I can see, \"$4\" is used to append \".\" in very limited cases, and\n>>> nobody explicitly passes SP as \"$4\" when calling this, so it may be easier\n>>> to read if you moved this before that \"if we have 3 or more args, use the\n>>> fourth one as the suffix\" block, i.e. something like this?\n>>\n>> Why not, but in case someone explicitely passes \" \" as $4 in the future,\n>> it's likely to be better to strip it for the same reason we strip it here.\n>\n> I doubt that would be sufficent or appropriate. If some caller _WANTS_ to\n> add a SP, shouldn't we be devising a way to tell zsh to add it without\n> quoting,\n\nYes, this is the point. But up to now, nobody found such a way so we're\njust trying to work around it in the less painfull way for the user.\n\nIf someone _wants_ to add a SP, then he still can't do it portably with\nyour patch, because the space will be quoted for bash users, and not for\nZSH users, so one of them will be unhappy.\n\n> So does that mean we would be forcing zsh users to add SP themselves? \n\nYes, but we already do so. From my commit message:\n\n The absence of trailing space for ZSH is a long-standing issue, that\n this patch is not fixing.\n\n;-).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"182688","messageId":"7vty3urwtj.fsf@alter.siamese.dyndns.org","threadId":"29352","inReplyTo":"vpqr4yy1pv6.fsf@bauges.imag.fr","subject":"Re: [PATCH] bash-completion: don't add quoted space for ZSH (fix regression)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-17T18:46:48Z","receivedAt":"2012-01-17T18:46:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n>> I doubt that would be sufficent or appropriate. If some caller _WANTS_ to\n>> add a SP, shouldn't we be devising a way to tell zsh to add it without\n>> quoting,\n>\n> Yes, this is the point. But up to now, nobody found such a way so we're\n> just trying to work around it in the less painfull way for the user.\n\nOk, I take it that the original patch is meant as a small step in making\nit a usable state, by not adding useless \"quoted SP\". In the ideal world\nit may be better to add SP but we do not know how without zsh interfering\nwith our attempt to do so and adding an unwanted quoting, so we are taking\nthe second best approach that we at least know works.\n\nWhich is fine by me, and as you said, the completion script always asked\nzsh users to add SP themselves, so it is not even a regression.\n\nThe real reason I am continuing this thread is to keep it alive so that a\nzsh guru would jump in from somewhere and show us \"here is how to tell Zsh\nnot to quote $suffix\"; that does not seem to be happening yet, so let's\nuse your patch as-is.\n"},{"id":"182692","messageId":"CAMP44s2nYMmfC36+pGaYfOZUQy3fLMYDuaSriPYjHBeApmsRVg@mail.gmail.com","threadId":"29352","inReplyTo":"1326567336-2173-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH] bash-completion: don't add quoted space for ZSH (fix regression)","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-01-17T19:18:22Z","receivedAt":"2012-01-17T19:18:22Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Jan 14, 2012 at 8:55 PM, Matthieu Moy wrote:\n> Commit a31e626 (completion: optimize refs completion) introduced a\n> regression for ZSH users: ref names were completed with a quoted trailing\n> space (i.e. \"git checkout ma\" completes to \"git checkout master\\ \"). The\n> space is convenient for bash users since we use \"-o nospace\", but a\n> quoted space is worse than nothing. The absence of trailing space for ZSH\n> is a long-standing issue, that this patch is not fixing. We just fix the\n> regression by not appending a space when the shell is ZSH.\n\nI have this issue with the script from v1.7.8.3, and I think it\nstarted with a zsh update.\n\n-- \nFelipe Contreras\n"},{"id":"182694","messageId":"CAMP44s0T15idhwb6Eae5vdMxf25KK9MhY57mBf+BFN=OSC6Lhg@mail.gmail.com","threadId":"29352","inReplyTo":"CAMP44s2nYMmfC36+pGaYfOZUQy3fLMYDuaSriPYjHBeApmsRVg@mail.gmail.com","subject":"Re: [PATCH] bash-completion: don't add quoted space for ZSH (fix regression)","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-01-17T20:03:08Z","receivedAt":"2012-01-17T20:03:08Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Jan 17, 2012 at 9:18 PM, Felipe Contreras\n wrote:\n> On Sat, Jan 14, 2012 at 8:55 PM, Matthieu Moy wrote:\n>> Commit a31e626 (completion: optimize refs completion) introduced a\n>> regression for ZSH users: ref names were completed with a quoted trailing\n>> space (i.e. \"git checkout ma\" completes to \"git checkout master\\ \"). The\n>> space is convenient for bash users since we use \"-o nospace\", but a\n>> quoted space is worse than nothing. The absence of trailing space for ZSH\n>> is a long-standing issue, that this patch is not fixing. We just fix the\n>> regression by not appending a space when the shell is ZSH.\n>\n> I have this issue with the script from v1.7.8.3, and I think it\n> started with a zsh update.\n\nYeah, works fine with zsh 4.3.11, not 4.3.14 or 15.\n\n-- \nFelipe Contreras\n"},{"id":"182696","messageId":"7vzkdmqebh.fsf@alter.siamese.dyndns.org","threadId":"29352","inReplyTo":"CAMP44s0T15idhwb6Eae5vdMxf25KK9MhY57mBf+BFN=OSC6Lhg@mail.gmail.com","subject":"Re: [PATCH] bash-completion: don't add quoted space for ZSH (fix regression)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-17T20:11:46Z","receivedAt":"2012-01-17T20:11:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> On Tue, Jan 17, 2012 at 9:18 PM, Felipe Contreras\n> wrote:\n>> On Sat, Jan 14, 2012 at 8:55 PM, Matthieu Moy wrote:\n>>> Commit a31e626 (completion: optimize refs completion) introduced a\n>>> regression for ZSH users: ref names were completed with a quoted trailing\n>>> space (i.e. \"git checkout ma\" completes to \"git checkout master\\ \"). The\n>>> space is convenient for bash users since we use \"-o nospace\", but a\n>>> quoted space is worse than nothing. The absence of trailing space for ZSH\n>>> is a long-standing issue, that this patch is not fixing. We just fix the\n>>> regression by not appending a space when the shell is ZSH.\n>>\n>> I have this issue with the script from v1.7.8.3, and I think it\n>> started with a zsh update.\n>\n> Yeah, works fine with zsh 4.3.11, not 4.3.14 or 15.\n\nAs I was planning to queue Matthieu's patch as-is as a regression fix\nbefore v1.7.9-rc2, I would appreciate if you can clarify this report a\nbit. Do you mean with the patch more recent versions of zsh still does not\nlike the workaround and adds quoted space at the end?\n"},{"id":"182704","messageId":"CAMP44s3GMGMD5Y9Z=Uu_e55_eZOG2zY76u8B=ORKsMx6yoXW5Q@mail.gmail.com","threadId":"29352","inReplyTo":"7vzkdmqebh.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] bash-completion: don't add quoted space for ZSH (fix regression)","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-01-17T23:04:12Z","receivedAt":"2012-01-17T23:04:12Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Jan 17, 2012 at 10:11 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> On Tue, Jan 17, 2012 at 9:18 PM, Felipe Contreras\n>> wrote:\n>>> On Sat, Jan 14, 2012 at 8:55 PM, Matthieu Moy wrote:\n>>>> Commit a31e626 (completion: optimize refs completion) introduced a\n>>>> regression for ZSH users: ref names were completed with a quoted trailing\n>>>> space (i.e. \"git checkout ma\" completes to \"git checkout master\\ \"). The\n>>>> space is convenient for bash users since we use \"-o nospace\", but a\n>>>> quoted space is worse than nothing. The absence of trailing space for ZSH\n>>>> is a long-standing issue, that this patch is not fixing. We just fix the\n>>>> regression by not appending a space when the shell is ZSH.\n>>>\n>>> I have this issue with the script from v1.7.8.3, and I think it\n>>> started with a zsh update.\n>>\n>> Yeah, works fine with zsh 4.3.11, not 4.3.14 or 15.\n>\n> As I was planning to queue Matthieu's patch as-is as a regression fix\n> before v1.7.9-rc2, I would appreciate if you can clarify this report a\n> bit. Do you mean with the patch more recent versions of zsh still does not\n> like the workaround and adds quoted space at the end?\n\nI am saying I am seeing the issue (or at least the same symptom) even\nbefore a31e626 with recent zsh versions. I will try a patched version\nof the script and a31e626, but I'm guessing the result would be the\nsame.\n\n-- \nFelipe Contreras\n"},{"id":"182706","messageId":"7v1uqxq4jm.fsf@alter.siamese.dyndns.org","threadId":"29352","inReplyTo":"CAMP44s3GMGMD5Y9Z=Uu_e55_eZOG2zY76u8B=ORKsMx6yoXW5Q@mail.gmail.com","subject":"Re: [PATCH] bash-completion: don't add quoted space for ZSH (fix regression)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-17T23:42:53Z","receivedAt":"2012-01-17T23:42:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n>> As I was planning to queue Matthieu's patch as-is as a regression fix\n>> before v1.7.9-rc2, I would appreciate if you can clarify this report a\n>> bit. Do you mean with the patch more recent versions of zsh still does not\n>> like the workaround and adds quoted space at the end?\n>\n> I am saying I am seeing the issue (or at least the same symptom) even\n> before a31e626 with recent zsh versions. I will try a patched version\n> of the script and a31e626, but I'm guessing the result would be the\n> same.\n\nOK, so the issue the patch addresses may not be a regression in the\nupcoming v1.7.9 we want to fix quickly, but if you tried the patch we\nmight see improvements, or we might not see any improvements.\n\nI'll hold onto Matthieu's patch for now, then. It would be nice to get\nthis minor detail fixed before the release, though, as any fix would be\nof limited scope, affecting zsh users for which it is already broken.\n\nThanks.\n"},{"id":"182719","messageId":"vpqlip5qvcm.fsf@bauges.imag.fr","threadId":"29352","inReplyTo":"7v1uqxq4jm.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] bash-completion: don't add quoted space for ZSH (fix regression)","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-01-18T08:16:09Z","receivedAt":"2012-01-18T08:16:09Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> OK, so the issue the patch addresses may not be a regression in the\n> upcoming v1.7.9 we want to fix quickly,\n\nI'm running ZSH 4.3.10 (Debian stable), and for me it is a regression.\nIt seems there is another bug elsewhere affecting more recent ZSH (I\ndon't have a recent ZSH version installed to test), but fixing the\nregression for old ZSH is still worth it. I'm not even sur the issue\nwith recent ZSH is related.\n\nAt worse, my patch is not intrusive and can easily be reworked later.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"183059","messageId":"CAMP44s2x2kJEJsQLZCJiegZY863X=kVO5xof9QBUin35i-BzhA@mail.gmail.com","threadId":"29352","inReplyTo":"vpqlip5qvcm.fsf@bauges.imag.fr","subject":"Re: [PATCH] bash-completion: don't add quoted space for ZSH (fix regression)","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-01-25T01:39:40Z","receivedAt":"2012-01-25T01:39:40Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Jan 18, 2012 at 10:16 AM, Matthieu Moy\n wrote:\n> Junio C Hamano writes:\n>\n>> OK, so the issue the patch addresses may not be a regression in the\n>> upcoming v1.7.9 we want to fix quickly,\n>\n> I'm running ZSH 4.3.10 (Debian stable), and for me it is a regression.\n> It seems there is another bug elsewhere affecting more recent ZSH (I\n> don't have a recent ZSH version installed to test), but fixing the\n> regression for old ZSH is still worth it. I'm not even sur the issue\n> with recent ZSH is related.\n>\n> At worse, my patch is not intrusive and can easily be reworked later.\n\nI believe I have found a more generic and simpler fix that works for\nboth the regression in v1.7.9, and users of zsh >= 4.3.12.\n\nPatch sent.\n\n-- \nFelipe Contreras\n"},{"id":"183065","messageId":"7vsjj41l4h.fsf@alter.siamese.dyndns.org","threadId":"29352","inReplyTo":"CAMP44s2x2kJEJsQLZCJiegZY863X=kVO5xof9QBUin35i-BzhA@mail.gmail.com","subject":"Re: [PATCH] bash-completion: don't add quoted space for ZSH (fix regression)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-25T04:06:38Z","receivedAt":"2012-01-25T04:06:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n>> At worse, my patch is not intrusive and can easily be reworked later.\n>\n> I believe I have found a more generic and simpler fix that works for\n> both the regression in v1.7.9, and users of zsh >= 4.3.12.\n>\n> Patch sent.\n\nMatthieu, care to take a look?\n\nIt looks fairly straightforward and would have absolutely no impact on\nbash users.\n\nThanks, Felipe.\n\n-- >8 --\nFrom: Felipe Contreras \nSubject: [PATCH] git-completion: workaround zsh COMPREPLY bug\n\nzsh adds a backslash (foo\\ ) for each item in the COMPREPLY array if IFS\ndoesn't contain spaces. This issue has been reported[1], but there is no\nsolution yet.\n\nThis wasn't a problem due to another bug[2], which was fixed in zsh\nversion 4.3.12. After this change, 'git checkout ma' would resolve\nto 'git checkout master\\ '.\n\nAditionally, the introduction of __gitcomp_nl in commit a31e626\n(completion: optimize refs completion) in git also made the problem\napparent, as Matthieu Moy reported.\n\nThe simplest and most generic solution is to hide all the changes we do\nto IFS, so that \"foo \\nbar \" is recognized by zsh as \"foo bar\". This\nworks on versions of git before and after the introduction of\n__gitcomp_nl (a31e626), and versions of zsh before and after 4.3.12.\n\nOnce zsh is fixed, we should conditionally disable this workaround to\nhave the same benefits as bash users.\n\n[1] http://www.zsh.org/mla/workers/2012/msg00053.html\n[2] http://zsh.git.sourceforge.net/git/gitweb.cgi?p=zsh/zsh;a=commitdiff;h=2e25dfb8fd38dbef0a306282ffab1d343ce3ad8d\n\nSigned-off-by: Felipe Contreras \n---\n contrib/completion/git-completion.bash | 8 ++++++++\n 1 files changed, 8 insertions(+), 0 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex b0062ba..c83c734 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2631,6 +2631,10 @@ _git ()\n \t\t# workaround zsh's bug that leaves 'words' as a special\n \t\t# variable in versions < 4.3.12\n \t\ttypeset -h words\n+\n+\t\t# another workaround for zsh because it would quote spaces in\n+\t\t# the COMPREPLY array if IFS doesn't contain spaces\n+\t\ttypeset -h IFS\n \tfi\n \n \tlocal cur words cword prev\n@@ -2687,6 +2691,10 @@ _gitk ()\n \t\t# workaround zsh's bug that leaves 'words' as a special\n \t\t# variable in versions < 4.3.12\n \t\ttypeset -h words\n+\n+\t\t# another workaround for zsh because it would quote spaces in\n+\t\t# the COMPREPLY array if IFS doesn't contain spaces\n+\t\ttypeset -h IFS\n \tfi\n \n \tlocal cur words cword prev\n"}]}