{"thread":{"id":"29528","subject":"[PATCH v4 0/4] completion: couple of cleanups","startedAt":"2012-02-02T20:30:21Z","lastAt":"2012-02-06T23:57:01Z","messageCount":11,"participants":["Felipe Contreras","Junio C Hamano","SZEDER Gábor"],"isPatch":true,"patchVersion":4,"patchTotal":4},"messages":[{"id":"183657","messageId":"1328214625-3576-1-git-send-email-felipe.contreras@gmail.com","threadId":"29528","inReplyTo":null,"subject":"[PATCH v4 0/4] completion: couple of cleanups","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-02-02T20:30:21Z","receivedAt":"2012-02-02T20:30:21Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"And an improvement for zsh.\n\nv4:\n\nSame as v3, but with an improved commit message with input from Thomas Rast and sending directly to Junio.\n\nv3:\n\nJunio: I see you already picked most of them for 'pu', but I've made further changes based on the feedback:\n\n * completion: be nicer with zsh\n Improved the code-style\n\n * completion: simplify __gitcomp*\n Fix\n Improved commit message\n\nFelipe Contreras (4):\n completion: work around zsh option propagation bug\n completion: simplify __git_remotes\n completion: remove unused code\n completion: simplify __gitcomp*\n\n contrib/completion/git-completion.bash | 66 +++++---------------------------\n 1 files changed, 10 insertions(+), 56 deletions(-)\n\n-- \n1.7.9\n"},{"id":"183755","messageId":"7v1uqbpsyh.fsf@alter.siamese.dyndns.org","threadId":"29528","inReplyTo":"1328214625-3576-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v4 1/4] completion: work around zsh option propagation bug","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-03T20:23:02Z","receivedAt":"2012-02-03T20:23:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> Right now when listing commands in zsh (git ), all of them\n> will show up, instead of only porcelain ones.\n\nJonathan's rewrite goes straight to the root cause instead, which is\nanother way to describe the problem.\n\nExplaining user-visible symptoms at the beginning like you did is a good\nstrategy that I would want to see more contributors follow, though.\n\n> Basically, in zsh, this:\n>\n> for i in $__git_all_commands\n>\n> Should be:\n>\n> for i in ${=__git_all_commands}\n>\n> Otherwise there's no word-splitting expansion (unless SH_WORD_SPLIT is\n> set). sh emulation should take care of that, but the subshell is messing\n> up with that. So __git_list_porcelain_commands does not do any\n> filtering.\n\nLet me step back a bit and see if we are on the same page wrt the root\ncause of the problem and for whom we are explaining the change.\n\nThe adaptation of the bash completion script to zsh is done by asking zsh\nto obey POSIXy word splitting rules to honor $IFS that is in effect when\nthe words are split. However zsh does not do a good job at it in some\ncases, and your patch works it around by avoiding a construct known to be\ntroublesome to zsh.\n\nAm I correct so far? If so, especially if the first sentence of the above\nparagraph is correct, then how would it help others to teach \"this is the\nright way to do a word-split if we were writing in native zsh\" when we are\nnot?\n\nWhile it probably is a good description to have in a bug report given to\nzsh folks, it is useless for people who read the history of Git. Unless\nyou are writing zsh-native completion script and this patch is not about\ngit-completion.bash, that is.\n\nThe readers need to read the solution described in order to understand why\nthe updated construct is written in an unnatural (to people who write to\nPOSIXy shells) way, or to avoid reintroducing a similar problem elsewhere\nin the future. I find that what Jonathan gave you helps them much better:\n\n\t...\n fn () {\n var='one two'\n printf '%s\\n' $var\n }\n x=$(fn)\n : ${y=$(fn)}\n\n printing \"$x\" results in two lines as expected, but printing \"$y\" results\n in a single line because $var is expanded as a single word when evaluating\n fn to compute y.\n\n So avoid the construct, and use an explicit 'test -n \"$foo\" || foo=$(bar)'\n instead.\n\nSo I'll take the first two lines of the message (good bits), and simplify\nthe \"This fixes a bug tht caused...\" from the last paragraph (or perhaps\neven drop it).\n\nThanks.\n"},{"id":"183756","messageId":"7vty37oedr.fsf@alter.siamese.dyndns.org","threadId":"29528","inReplyTo":"1328214625-3576-5-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v4 4/4] completion: simplify __gitcomp*","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-03T20:23:12Z","receivedAt":"2012-02-03T20:23:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> @@ -495,11 +495,7 @@ fi\n> # 4: A suffix to be appended to each possible completion word (optional).\n> __gitcomp ()\n> {\n> -\tlocal cur_=\"$cur\"\n> -\n> -\tif [ $# -gt 2 ]; then\n> -\t\tcur_=\"$3\"\n> -\tfi\n> +\tlocal cur_=\"${3:-$cur}\"\n> \tcase \"$cur_\" in\n> \t--*=)\n> \t\tCOMPREPLY=()\n\nI think this rewrite is wrong, even though it may not make a difference to\nthe current callers (I didn't check). Drop the colon from ${3:-...}.\n\n> @@ -524,18 +520,8 @@ __gitcomp ()\n> # appended.\n> __gitcomp_nl ()\n> {\n> -\tlocal s=$'\\n' IFS=' '$'\\t'$'\\n'\n> -\tlocal cur_=\"$cur\" suffix=\" \"\n> -\n> -\tif [ $# -gt 2 ]; then\n> -\t\tcur_=\"$3\"\n> -\t\tif [ $# -gt 3 ]; then\n> -\t\t\tsuffix=\"$4\"\n> -\t\tfi\n> -\tfi\n> -\n> -\tIFS=$s\n> -\tCOMPREPLY=($(compgen -P \"${2-}\" -S \"$suffix\" -W \"$1\" -- \"$cur_\"))\n> +\tlocal IFS=$'\\n'\n> +\tCOMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$1\" -- \"${3:-$cur}\"))\n\nSo is this.\n\nFixing the above two gives me what I've already sent in $gmane/189683,\nso...\n"},{"id":"183826","messageId":"20120204135404.GF16099@goldbirke","threadId":"29528","inReplyTo":"7vty37oedr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v4 4/4] completion: simplify __gitcomp*","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-02-04T13:54:04Z","receivedAt":"2012-02-04T13:54:04Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Hi,\n\n\nOn Fri, Feb 03, 2012 at 12:23:12PM -0800, Junio C Hamano wrote:\n> Felipe Contreras writes:\n> \n> > @@ -495,11 +495,7 @@ fi\n> > # 4: A suffix to be appended to each possible completion word (optional).\n> > __gitcomp ()\n> > {\n> > -\tlocal cur_=\"$cur\"\n> > -\n> > -\tif [ $# -gt 2 ]; then\n> > -\t\tcur_=\"$3\"\n> > -\tfi\n> > +\tlocal cur_=\"${3:-$cur}\"\n> > \tcase \"$cur_\" in\n> > \t--*=)\n> > \t\tCOMPREPLY=()\n> \n> I think this rewrite is wrong, even though it may not make a difference to\n> the current callers (I didn't check). Drop the colon from ${3:-...}.\n> \n> > @@ -524,18 +520,8 @@ __gitcomp ()\n> > # appended.\n> > __gitcomp_nl ()\n> > {\n> > -\tlocal s=$'\\n' IFS=' '$'\\t'$'\\n'\n> > -\tlocal cur_=\"$cur\" suffix=\" \"\n> > -\n> > -\tif [ $# -gt 2 ]; then\n> > -\t\tcur_=\"$3\"\n> > -\t\tif [ $# -gt 3 ]; then\n> > -\t\t\tsuffix=\"$4\"\n> > -\t\tfi\n> > -\tfi\n> > -\n> > -\tIFS=$s\n> > -\tCOMPREPLY=($(compgen -P \"${2-}\" -S \"$suffix\" -W \"$1\" -- \"$cur_\"))\n> > +\tlocal IFS=$'\\n'\n> > +\tCOMPREPLY=($(compgen -P \"${2-}\" -S \"${4- }\" -W \"$1\" -- \"${3:-$cur}\"))\n> \n> So is this.\n> \n> Fixing the above two gives me what I've already sent in $gmane/189683,\n> so...\n> \n\nGood point, I missed this when pointed out the similar issue with $4\nearlier.\n\nAnd it does make a difference, it breaks the completion of a single\nword in multiple steps, e.g. git log --pretty= master... In\nsuch cases we pass \"${cur##--pretty=}\" and \"${cur_#*..}\" as third\nargument to __gitcomp() and __gitcomp_nl(), which can be empty strings\nwhen the user hits TAB right after the '=' and '..'. Replacing that\nempty string with $cur is bad, because none of the possible completion\nwords (i.e. $1) will match it, and bash will fall back to filename\ncompletion.\n\nWithout the colon, i.e. using \"${3-$cur}\", it works as expected.\n\n\nBest,\nGábor\n"},{"id":"183912","messageId":"7vmx8xggan.fsf@alter.siamese.dyndns.org","threadId":"29528","inReplyTo":"20120204135404.GF16099@goldbirke","subject":"Re: [PATCH v4 4/4] completion: simplify __gitcomp*","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-05T20:45:52Z","receivedAt":"2012-02-05T20:45:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n> And it does make a difference, it breaks the completion of a single\n> word in multiple steps, e.g. git log --pretty= master... In\n> such cases we pass \"${cur##--pretty=}\" and \"${cur_#*..}\" as third\n> argument to __gitcomp() and __gitcomp_nl(), which can be empty strings\n> when the user hits TAB right after the '=' and '..'.\n\nAfter saying \"this rewrite is wrong\", I was actually wondering if I should\nhave said \"this rewrite is not faithful to the original\". Based on your\nanalysis, the difference does break the callers, so the rewrite is indeed\nwrong.\n\nThanks for following up.\n"},{"id":"183920","messageId":"CAMP44s3ECM2e08wsjMzu9Y4xDRw+KVNbMdCeAmxiPn3jNtC59Q@mail.gmail.com","threadId":"29528","inReplyTo":"7vmx8xggan.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v4 4/4] completion: simplify __gitcomp*","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-02-05T21:14:03Z","receivedAt":"2012-02-05T21:14:03Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"2012/2/5 Junio C Hamano :\n> SZEDER Gábor writes:\n>\n>> And it does make a difference, it breaks the completion of a single\n>> word in multiple steps, e.g. git log --pretty= master...  In\n>> such cases we pass \"${cur##--pretty=}\" and \"${cur_#*..}\" as third\n>> argument to __gitcomp() and __gitcomp_nl(), which can be empty strings\n>> when the user hits TAB right after the '=' and '..'.\n>\n> After saying \"this rewrite is wrong\", I was actually wondering if I should\n> have said \"this rewrite is not faithful to the original\".  Based on your\n> analysis, the difference does break the callers, so the rewrite is indeed\n> wrong.\n\nThat's why we need tests for the completion stuff as well. I was\nthinking on doing that, but if I have to write a peer-reviewed essay\nwith an introduction for the people that are not familiar with the\ncode in each of the patches, I'd rather not.\n\n-- \nFelipe Contreras\n"},{"id":"184056","messageId":"20120206205315.GI16099@goldbirke","threadId":"29528","inReplyTo":"1328214625-3576-3-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v4 2/4] completion: simplify __git_remotes","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-02-06T20:53:15Z","receivedAt":"2012-02-06T20:53:15Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Hi,\n\n\nOn Thu, Feb 02, 2012 at 10:30:23PM +0200, Felipe Contreras wrote:\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index b435b6d..f86b734 100755\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -644,12 +644,7 @@ __git_refs_remotes ()\n> __git_remotes ()\n> {\n> \tlocal i ngoff IFS=$'\\n' d=\"$(__gitdir)\"\n\nYou could also remove the ngoff variable, because with this patch it's\nnot used anymore.\n\n> -\t__git_shopt -q nullglob || ngoff=1\n> -\t__git_shopt -s nullglob\n> -\tfor i in \"$d/remotes\"/*; do\n> -\t\techo ${i#$d/remotes/}\n> -\tdone\n> -\t[ \"$ngoff\" ] && __git_shopt -u nullglob\n> +\ttest -d \"$d/remotes\" && ls -1 \"$d/remotes\"\n> \tfor i in $(git --git-dir=\"$d\" config --get-regexp 'remote\\..*\\.url' 2>/dev/null); do\n> \t\ti=\"${i#remote.}\"\n> \t\techo \"${i/.url*/}\"\n> -- \n> 1.7.9\n> \n"},{"id":"184057","messageId":"CAMP44s2vwQK4Mp=qJN-_60NRBY42fxzFLHbh9e9j=vL5mrMryg@mail.gmail.com","threadId":"29528","inReplyTo":"20120206205315.GI16099@goldbirke","subject":"Re: [PATCH v4 2/4] completion: simplify __git_remotes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-02-06T21:04:29Z","receivedAt":"2012-02-06T21:04:29Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"2012/2/6 SZEDER Gábor :\n> On Thu, Feb 02, 2012 at 10:30:23PM +0200, Felipe Contreras wrote:\n>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n>> index b435b6d..f86b734 100755\n>> --- a/contrib/completion/git-completion.bash\n>> +++ b/contrib/completion/git-completion.bash\n>> @@ -644,12 +644,7 @@ __git_refs_remotes ()\n>>  __git_remotes ()\n>>  {\n>>       local i ngoff IFS=$'\\n' d=\"$(__gitdir)\"\n>\n> You could also remove the ngoff variable, because with this patch it's\n> not used anymore.\n\nRight, I thought I did that... The change must have been lost in one\nof the many revisions =/\n\n-- \nFelipe Contreras\n"},{"id":"184072","messageId":"CAMP44s3SruBpM74BjWuTLfS=_66p7r6rkjJ+ObLr4bLq0nERNA@mail.gmail.com","threadId":"29528","inReplyTo":"7v1uqbpsyh.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v4 1/4] completion: work around zsh option propagation bug","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-02-06T22:59:34Z","receivedAt":"2012-02-06T22:59:34Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Feb 3, 2012 at 10:23 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> Right now when listing commands in zsh (git ), all of them\n>> will show up, instead of only porcelain ones.\n>\n> Jonathan's rewrite goes straight to the root cause instead, which is\n> another way to describe the problem.\n>\n> Explaining user-visible symptoms at the beginning like you did is a good\n> strategy that I would want to see more contributors follow, though.\n>\n>> Basically, in zsh, this:\n>>\n>>  for i in $__git_all_commands\n>>\n>> Should be:\n>>\n>>  for i in ${=__git_all_commands}\n>>\n>> Otherwise there's no word-splitting expansion (unless SH_WORD_SPLIT is\n>> set). sh emulation should take care of that, but the subshell is messing\n>> up with that. So __git_list_porcelain_commands does not do any\n>> filtering.\n>\n> Let me step back a bit and see if we are on the same page wrt the root\n> cause of the problem and for whom we are explaining the change.\n>\n> The adaptation of the bash completion script to zsh is done by asking zsh\n> to obey POSIXy word splitting rules to honor $IFS that is in effect when\n> the words are split.  However zsh does not do a good job at it in some\n> cases, and your patch works it around by avoiding a construct known to be\n> troublesome to zsh.\n\nTroublesome to zsh emulation, yeah.\n\n> Am I correct so far?  If so, especially if the first sentence of the above\n> paragraph is correct, then how would it help others to teach \"this is the\n> right way to do a word-split if we were writing in native zsh\" when we are\n> not?\n\nBecause without that explanation it's quite difficult to know what\npart of the code would behave differently in zsh, and how. Most people\nare not familiar with shell features, and would have no idea what\n\"word splitting\" means in a practical context.\n\n> While it probably is a good description to have in a bug report given to\n> zsh folks, it is useless for people who read the history of Git.\n\nOf course it's not. It tells you that there is indeed an issue in zsh,\nand not in the way we are using it, as it has been acknowledged by zsh\ndevelopers.\n\n> The readers need to read the solution described in order to understand why\n> the updated construct is written in an unnatural (to people who write to\n> POSIXy shells) way, or to avoid reintroducing a similar problem elsewhere\n> in the future.\n\nDoesn't this explain that?\n\n---\nsh emulation should take care of that, but the subshell is messing\nup with that.\n---\n\nGranted, for people not familiar with shell features \"subshell\" should\nbe accompanied with $(foo).\n\n> I find that what Jonathan gave you helps them much better:\n>        ...\n>        fn () {\n>                var='one two'\n>                printf '%s\\n' $var\n>        }\n>        x=$(fn)\n>        : ${y=$(fn)}\n>\n>    printing \"$x\" results in two lines as expected, but printing \"$y\" results\n>    in a single line because $var is expanded as a single word when evaluating\n>    fn to compute y.\n>\n>    So avoid the construct, and use an explicit 'test -n \"$foo\" || foo=$(bar)'\n>    instead.\n>\n> So I'll take the first two lines of the message (good bits), and simplify\n> the \"This fixes a bug tht caused...\" from the last paragraph (or perhaps\n> even drop it).\n\nI'm not sure about it, because this relies on knowledge of how printf\nworks, and it's not used that often; an example with 'for' would be\nmuch more clear IMO.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"184075","messageId":"7vvcnj4kif.fsf@alter.siamese.dyndns.org","threadId":"29528","inReplyTo":"CAMP44s3SruBpM74BjWuTLfS=_66p7r6rkjJ+ObLr4bLq0nERNA@mail.gmail.com","subject":"Re: [PATCH v4 1/4] completion: work around zsh option propagation bug","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-06T23:20:08Z","receivedAt":"2012-02-06T23:20:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n>> I find that what Jonathan gave you helps them much better:\n>> ...\n>> fn () {\n>> var='one two'\n>> printf '%s\\n' $var\n>> }\n>> x=$(fn)\n>> : ${y=$(fn)}\n>>\n>> printing \"$x\" results in two lines as expected, but printing \"$y\" results\n>> in a single line because $var is expanded as a single word when evaluating\n>> fn to compute y.\n>>\n>> So avoid the construct, and use an explicit 'test -n \"$foo\" || foo=$(bar)'\n>> instead.\n>>\n>> So I'll take the first two lines of the message (good bits), and simplify\n>> the \"This fixes a bug tht caused...\" from the last paragraph (or perhaps\n>> even drop it).\n>\n> I'm not sure about it, because this relies on knowledge of how printf\n> works, and it's not used that often; an example with 'for' would be\n> much more clear IMO.\n\nMeaning, replace the fn() definition with something like:\n\n\tfn () {\n\t\tvar='one two'\n for v in $var\n do\n \techo \"$v\"\n\t\tdone\n\t}\n\nI can see that may make the issue easier to see; as you pointed out, it\nrequires no implicit knowledge that printf \"loops\" over the arguments\nand applies the format string as manu times as needed to eat them.\nLet me update the log message before I merge it to 'next'.\n\nMy main point was to illustrate the problematic pattern for people who\nwrite for bash, not for zsh, and that does not change with the above\nimprovement, though ;-).\n"},{"id":"184078","messageId":"CAMP44s1urg0-1GZqLytg0h=qT77_zajwsKwNt9HnVfRZoOECqA@mail.gmail.com","threadId":"29528","inReplyTo":"7vvcnj4kif.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v4 1/4] completion: work around zsh option propagation bug","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-02-06T23:57:01Z","receivedAt":"2012-02-06T23:57:01Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Feb 7, 2012 at 1:20 AM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>>> I find that what Jonathan gave you helps them much better:\n>>>        ...\n>>>        fn () {\n>>>                var='one two'\n>>>                printf '%s\\n' $var\n>>>        }\n>>>        x=$(fn)\n>>>        : ${y=$(fn)}\n>>>\n>>>    printing \"$x\" results in two lines as expected, but printing \"$y\" results\n>>>    in a single line because $var is expanded as a single word when evaluating\n>>>    fn to compute y.\n>>>\n>>>    So avoid the construct, and use an explicit 'test -n \"$foo\" || foo=$(bar)'\n>>>    instead.\n>>>\n>>> So I'll take the first two lines of the message (good bits), and simplify\n>>> the \"This fixes a bug tht caused...\" from the last paragraph (or perhaps\n>>> even drop it).\n>>\n>> I'm not sure about it, because this relies on knowledge of how printf\n>> works, and it's not used that often; an example with 'for' would be\n>> much more clear IMO.\n>\n> Meaning, replace the fn() definition with something like:\n>\n>        fn () {\n>                var='one two'\n>                for v in $var\n>                do\n>                        echo \"$v\"\n>                done\n>        }\n\nYes.\n\n> I can see that may make the issue easier to see; as you pointed out, it\n> requires no implicit knowledge that printf \"loops\" over the arguments\n> and applies the format string as manu times as needed to eat them.\n> Let me update the log message before I merge it to 'next'.\n>\n> My main point was to illustrate the problematic pattern for people who\n> write for bash, not for zsh, and that does not change with the above\n> improvement, though ;-).\n\nTrue, but I think that's an addendum. Most likely the people working\non this script will be thinking on bash terms, and would not notice if\nthey introduce code that is difficult for zsh, even if it's carefully\nexplained in this commit message; it would be zsh people that find the\nbug, thus that's why I address the zsh side _first_.\n\nSo, IMO it should look like (looks like a fix has been submitted, so\nI've updated accordingly):\n\n---\ncompletion: work around zsh option propagation bug\n\nRight now when listing commands in zsh (git ), all of them\nwill show up, instead of only porcelain ones.\n\nThis is caused by a bug in zsh[1] that causes subshells to loose the\nword splitting option (SH_WORD_SPLIT) since 4.3.0[2]. It will probably\nbe fixed in the next release (4.3.16).\n\nBasically, in zsh, this:\n\n for i in $__git_all_commands\n\nShould be:\n\n for i in ${=__git_all_commands}\n\nOtherwise there's no word-splitting expansion (unless SH_WORD_SPLIT is\nset). sh emulation should take care of that, but the subshell is\nmessing things up.\n\nThe visible result is that __git_list_porcelain_commands don't do any filtering.\n\nSpecifically, the issue is with subshells in parmeter expansion\n(e.g. '${var=$(func)'):\n\n fn () {\n var='one two'\n for v in $var; do\n echo \"$v\"\n done\n }\n\n x=$(fn)\n : ${y=$(fn)}\n\nPrinting \"$x\" results in two lines as expected, but printing \"$y\"\nresults in a single line because $var is expanded as a single word\n(there's no word-splitting expansion).\n\nSo avoid the construct, and use an explicit '[ -n \"$foo\" ] ||\nfoo=$(bar)' instead.\n\n[1] http://article.gmane.org/gmane.comp.shells.zsh.devel/24296\n[2] http://article.gmane.org/gmane.comp.shells.zsh.devel/24338\n---\n\nCheers.\n\n-- \nFelipe Contreras\n"}]}