{"thread":{"id":"37687","subject":"[PATCH] completion: ignore chpwd_functions when cding","startedAt":"2014-10-08T03:53:14Z","lastAt":"2014-10-16T18:10:14Z","messageCount":16,"participants":["Brandon Turner","Junio C Hamano","Øystein Walle"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"250328","messageId":"1412740394-34061-1-git-send-email-bt@brandonturner.net","threadId":"37687","inReplyTo":null,"subject":"[PATCH] completion: ignore chpwd_functions when cding","fromName":"Brandon Turner","fromEmail":"bt@brandonturner.net","sentAt":"2014-10-08T03:53:14Z","receivedAt":"2014-10-08T03:53:14Z","isPatch":true,"sender":{"key":"bt@brandonturner.net","avatar":"https://gravatar.com/avatar/6d077ef330be095c42599e0b5d25bf1debd15aedc75ec154d233e82d8428b6c7?d=mp&s=160"},"body":"Software, such as RVM (ruby version manager), may set chpwd functions\nthat result in an endless loop when cding.  chpwd functions should be\nignored.\n\nSigned-off-by: Brandon Turner <bt@brandonturner.net>\n---\nFor an example of this bug, see:\nhttps://github.com/wayneeseguin/rvm/issues/3076\n\n contrib/completion/git-completion.bash | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 06bf262..996de31 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -283,7 +283,8 @@ __git_ls_files_helper ()\n {\n \t(\n \t\ttest -n \"${CDPATH+set}\" && unset CDPATH\n-\t\tcd \"$1\"\n+\t\t(( ${#chpwd_functions} )) && chpwd_functions=()\n+\t\tbuiltin cd \"$1\"\n \t\tif [ \"$2\" == \"--committable\" ]; then\n \t\t\tgit diff-index --name-only --relative HEAD\n \t\telse\n-- \n2.1.2\n"},{"id":"250377","messageId":"xmqqh9zebfc6.fsf@gitster.dls.corp.google.com","threadId":"37687","inReplyTo":"1412740394-34061-1-git-send-email-bt@brandonturner.net","subject":"Re: [PATCH] completion: ignore chpwd_functions when cding","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-08T18:12:41Z","receivedAt":"2014-10-08T18:12:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Turner <bt@brandonturner.net> writes:\n\n> Software, such as RVM (ruby version manager), may set chpwd functions\n> that result in an endless loop when cding.  chpwd functions should be\n> ignored.\n>\n> Signed-off-by: Brandon Turner <bt@brandonturner.net>\n> ---\n\nCan you mention that this is abomination limited only to zsh\nsomewhere in the log message?  Or does bash share the same glitch?\n\nIf this is limited to zsh, I wonder if we can take advantage of the\nfact that we have git-completion.bash and git-completion.zsh to\navoid contaminating shared part of the code.\n\nThanks.\n\n> For an example of this bug, see:\n> https://github.com/wayneeseguin/rvm/issues/3076\n>\n>  contrib/completion/git-completion.bash | 3 ++-\n>  1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 06bf262..996de31 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -283,7 +283,8 @@ __git_ls_files_helper ()\n>  {\n>  \t(\n>  \t\ttest -n \"${CDPATH+set}\" && unset CDPATH\n> -\t\tcd \"$1\"\n> +\t\t(( ${#chpwd_functions} )) && chpwd_functions=()\n> +\t\tbuiltin cd \"$1\"\n>  \t\tif [ \"$2\" == \"--committable\" ]; then\n>  \t\t\tgit diff-index --name-only --relative HEAD\n>  \t\telse\n"},{"id":"250401","messageId":"1412804988-56858-1-git-send-email-bt@brandonturner.net","threadId":"37687","inReplyTo":"xmqqh9zebfc6.fsf@gitster.dls.corp.google.com","subject":"[PATCH v2] completion: ignore chpwd_functions when cding","fromName":"Brandon Turner","fromEmail":"bt@brandonturner.net","sentAt":"2014-10-08T21:49:47Z","receivedAt":"2014-10-08T21:49:47Z","isPatch":true,"sender":{"key":"bt@brandonturner.net","avatar":"https://gravatar.com/avatar/6d077ef330be095c42599e0b5d25bf1debd15aedc75ec154d233e82d8428b6c7?d=mp&s=160"},"body":"Software, such as RVM (ruby version manager), may set chpwd functions\nthat result in an endless loop when cding.  chpwd functions should be\nignored.\n\nI have only noticed the RVM bug on ZSH, bash seems unaffected.  However\nthis change seems safe to apply to both bash and zsh as we cannot\ncontrol what functions users add to chpwd_functions.\n\nSigned-off-by: Brandon Turner <bt@brandonturner.net>\n---\nThis addresses Junio's request to update the log message.  The patch\nstill applies to bash and zsh.\n\nFor more information on the RVM bug, see:\nhttps://github.com/wayneeseguin/rvm/issues/3076\n contrib/completion/git-completion.bash | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 06bf262..996de31 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -283,7 +283,8 @@ __git_ls_files_helper ()\n {\n \t(\n \t\ttest -n \"${CDPATH+set}\" && unset CDPATH\n-\t\tcd \"$1\"\n+\t\t(( ${#chpwd_functions} )) && chpwd_functions=()\n+\t\tbuiltin cd \"$1\"\n \t\tif [ \"$2\" == \"--committable\" ]; then\n \t\t\tgit diff-index --name-only --relative HEAD\n \t\telse\n-- \n2.1.2\n"},{"id":"250400","messageId":"1412804988-56858-2-git-send-email-bt@brandonturner.net","threadId":"37687","inReplyTo":"xmqqh9zebfc6.fsf@gitster.dls.corp.google.com","subject":"[PATCH v3] completion: ignore chpwd_functions when cding on zsh","fromName":"Brandon Turner","fromEmail":"bt@brandonturner.net","sentAt":"2014-10-08T21:49:48Z","receivedAt":"2014-10-08T21:49:48Z","isPatch":true,"sender":{"key":"bt@brandonturner.net","avatar":"https://gravatar.com/avatar/6d077ef330be095c42599e0b5d25bf1debd15aedc75ec154d233e82d8428b6c7?d=mp&s=160"},"body":"Software, such as RVM (ruby version manager), may set chpwd functions\nthat result in an endless loop when cding.  chpwd functions should be\nignored.\n\nAs I've only seen this so far on ZSH, I'm applying this change only to\nthe git-completion.zsh overrides.\n\nSigned-off-by: Brandon Turner <bt@brandonturner.net>\n---\nThis applies the patch to zsh only using git-completion.zsh.\n\nFor more details on the RVM bug, see:\nhttps://github.com/wayneeseguin/rvm/issues/3076\n contrib/completion/git-completion.zsh | 15 +++++++++++++++\n 1 file changed, 15 insertions(+)\n\ndiff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh\nindex 9f6f0fa..12ac984 100644\n--- a/contrib/completion/git-completion.zsh\n+++ b/contrib/completion/git-completion.zsh\n@@ -93,6 +93,21 @@ __gitcomp_file ()\n \tcompadd -Q -p \"${2-}\" -f -- ${=1} && _ret=0\n }\n \n+__git_ls_files_helper ()\n+{\n+\t(\n+\t\ttest -n \"${CDPATH+set}\" && unset CDPATH\n+\t\t(( ${#chpwd_functions} )) && chpwd_functions=()\n+\t\tbuiltin cd \"$1\"\n+\t\tif [ \"$2\" == \"--committable\" ]; then\n+\t\t\tgit diff-index --name-only --relative HEAD\n+\t\telse\n+\t\t\t# NOTE: $2 is not quoted in order to support multiple options\n+\t\t\tgit ls-files --exclude-standard $2\n+\t\tfi\n+\t) 2>/dev/null\n+}\n+\n __git_zsh_bash_func ()\n {\n \temulate -L ksh\n-- \n2.1.2\n"},{"id":"250399","messageId":"CAMUzdXkQcbwhmJRYog+X_9B_cdWv6FkTrVFT__Dsw2k+qKN0=g@mail.gmail.com","threadId":"37687","inReplyTo":"xmqqh9zebfc6.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] completion: ignore chpwd_functions when cding","fromName":"Brandon Turner","fromEmail":"bt@brandonturner.net","sentAt":"2014-10-08T21:50:16Z","receivedAt":"2014-10-08T21:50:16Z","isPatch":true,"sender":{"key":"bt@brandonturner.net","avatar":"https://gravatar.com/avatar/6d077ef330be095c42599e0b5d25bf1debd15aedc75ec154d233e82d8428b6c7?d=mp&s=160"},"body":"On Wed, Oct 8, 2014 at 1:12 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>\n>\n> Can you mention that this is abomination limited only to zsh\n> somewhere in the log message?  Or does bash share the same glitch?\n>\n> If this is limited to zsh, I wonder if we can take advantage of the\n> fact that we have git-completion.bash and git-completion.zsh to\n> avoid contaminating shared part of the code.\n\nI'm going to submit two versions of the patch:\nv2 - addresses the log message, but the patch still applies to bash\nand zsh\nv3 - moves the change to git-completion.zsh as an override, bash is\nunaffected\n\n>From my testing, bash is unaffected, so v3 would be an ok fix.  Since\nwe cannot control what functions users add to chpwd_functions, v2 might\nmake more sense.\n"},{"id":"250416","messageId":"loom.20141009T093007-811@post.gmane.org","threadId":"37687","inReplyTo":"1412804988-56858-2-git-send-email-bt@brandonturner.net","subject":"Re: [PATCH v3] completion: ignore chpwd_functions when cding on zsh","fromName":"Øystein Walle","fromEmail":"oystwa@gmail.com","sentAt":"2014-10-09T07:34:30Z","receivedAt":"2014-10-09T07:34:30Z","isPatch":true,"sender":{"key":"oystwa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/794585?v=4"},"body":"Brandon Turner <bt <at> brandonturner.net> writes:\n\n> \n> Software, such as RVM (ruby version manager), may set chpwd functions\n> that result in an endless loop when cding.  chpwd functions should be\n> ignored.\n\nNow that it has moved to the zsh-specific script you can achieve this more\nsimply by using cd -q.\n\nØsse\n"},{"id":"250433","messageId":"xmqqlhop6rmj.fsf@gitster.dls.corp.google.com","threadId":"37687","inReplyTo":"loom.20141009T093007-811@post.gmane.org","subject":"Re: [PATCH v3] completion: ignore chpwd_functions when cding on zsh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-09T18:10:44Z","receivedAt":"2014-10-09T18:10:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Øystein Walle <oystwa@gmail.com> writes:\n\n> Brandon Turner <bt <at> brandonturner.net> writes:\n>\n>> \n>> Software, such as RVM (ruby version manager), may set chpwd functions\n>> that result in an endless loop when cding.  chpwd functions should be\n>> ignored.\n>\n> Now that it has moved to the zsh-specific script you can achieve this more\n> simply by using cd -q.\n\n;-)\n\nIs the way we defeat CDPATH for POSIX shells sufficient, or does it\nalso need to be customized for zsh?\n"},{"id":"250436","messageId":"1412881298-64117-1-git-send-email-bt@brandonturner.net","threadId":"37687","inReplyTo":"xmqqlhop6rmj.fsf@gitster.dls.corp.google.com","subject":"[PATCH v4] completion: ignore chpwd_functions when cding on zsh","fromName":"Brandon Turner","fromEmail":"bt@brandonturner.net","sentAt":"2014-10-09T19:01:38Z","receivedAt":"2014-10-09T19:01:38Z","isPatch":true,"sender":{"key":"bt@brandonturner.net","avatar":"https://gravatar.com/avatar/6d077ef330be095c42599e0b5d25bf1debd15aedc75ec154d233e82d8428b6c7?d=mp&s=160"},"body":"Software, such as RVM (ruby version manager), may set chpwd functions\nthat result in an endless loop when cding.  chpwd functions should be\nignored.\n\nAs I've only seen this so far on ZSH, I'm applying this change only to\nthe git-completion.zsh overrides.\n\nSigned-off-by: Brandon Turner <bt@brandonturner.net>\n---\nAs Øystein pointed out, on zsh we can use \"cd -q\" to ignore\nchpwd_functions.\n\nJunio - from my testing, unsetting CDPATH is sufficient on zsh.\n\n contrib/completion/git-completion.zsh | 14 ++++++++++++++\n 1 file changed, 14 insertions(+)\n\ndiff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh\nindex 9f6f0fa..04ed348 100644\n--- a/contrib/completion/git-completion.zsh\n+++ b/contrib/completion/git-completion.zsh\n@@ -93,6 +93,20 @@ __gitcomp_file ()\n \tcompadd -Q -p \"${2-}\" -f -- ${=1} && _ret=0\n }\n \n+__git_ls_files_helper ()\n+{\n+\t(\n+\t\ttest -n \"${CDPATH+set}\" && unset CDPATH\n+\t\tcd -q \"$1\"\n+\t\tif [ \"$2\" == \"--committable\" ]; then\n+\t\t\tgit diff-index --name-only --relative HEAD\n+\t\telse\n+\t\t\t# NOTE: $2 is not quoted in order to support multiple options\n+\t\t\tgit ls-files --exclude-standard $2\n+\t\tfi\n+\t) 2>/dev/null\n+}\n+\n __git_zsh_bash_func ()\n {\n \temulate -L ksh\n-- \n2.1.2\n"},{"id":"250442","messageId":"loom.20141009T211723-95@post.gmane.org","threadId":"37687","inReplyTo":"xmqqlhop6rmj.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3] completion: ignore chpwd_functions when cding on zsh","fromName":"Øystein Walle","fromEmail":"oystwa@gmail.com","sentAt":"2014-10-09T19:21:59Z","receivedAt":"2014-10-09T19:21:59Z","isPatch":true,"sender":{"key":"oystwa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/794585?v=4"},"body":"Junio C Hamano <gitster <at> pobox.com> writes:\n\n> \n> Øystein Walle <oystwa <at> gmail.com> writes:\n> \n> > Brandon Turner <bt <at> brandonturner.net> writes:\n> >\n> >> \n> >> Software, such as RVM (ruby version manager), may set chpwd functions\n> >> that result in an endless loop when cding.  chpwd functions should be\n> >> ignored.\n> >\n> > Now that it has moved to the zsh-specific script you can achieve this more\n> > simply by using cd -q.\n> \n> \n> \n> Is the way we defeat CDPATH for POSIX shells sufficient, or does it\n> also need to be customized for zsh?\n> \n\nThat is fine (to the best of my knowledge). If the current directory is not\npart of CDPATH at all then Zsh will try the current directory first, so if\nanything Zsh should fail more seldom here than others (but not *never*, so\nthe hack is still needed).\n\nØsse\n"},{"id":"250451","messageId":"loom.20141009T214418-680@post.gmane.org","threadId":"37687","inReplyTo":"1412881298-64117-1-git-send-email-bt@brandonturner.net","subject":"Re: [PATCH v4] completion: ignore chpwd_functions when cding on zsh","fromName":"Øystein Walle","fromEmail":"oystwa@gmail.com","sentAt":"2014-10-09T19:47:08Z","receivedAt":"2014-10-09T19:47:08Z","isPatch":true,"sender":{"key":"oystwa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/794585?v=4"},"body":"Brandon Turner <bt <at> brandonturner.net> writes:\n\n> +__git_ls_files_helper ()\n> +{\n> +\t(\n> +\t\ttest -n \"${CDPATH+set}\" && unset CDPATH\n> +\t\tcd -q \"$1\"\n> +\t\tif [ \"$2\" == \"--committable\" ]; then\n> +\t\t\tgit diff-index --name-only --relative HEAD\n> +\t\telse\n> +\t\t\t# NOTE: $2 is not quoted in order to support multiple options\n> +\t\t\tgit ls-files --exclude-standard $2\n> +\t\tfi\n> +\t) 2>/dev/null\n> +}\n> +\n\n(Sorry about this; I should've caught it the first time around). Zsh\ndoes not split string expansions into several words by default. For\nexample:\n\n    $ str1='hello world'\n    $ str2='goodbye moon'\n    $ printf '%s\\n' $str1 $str2\n    hello world\n    goodbye moon\n\nThis can be enabled on a \"per-expansion basis\" by using = while\nexpanding: \n\n    $ str1='hello world'\n    $ str2='goodbye moon'\n    $ printf '%s\\n' $=str1 $str2\n    hello\n    world\n    goodbye moon\n\nSo the $2 in your patch should be $=2.\n\nBUT: Over a year ago Git learned the -C argument. Couldn't we use that\nhere? That way we would not have to unset CDPATH and can get rid of the\nsubshell and cd -q. If we allow the other functions to use several\narguments to pass options with we can get rid of the whole seperation\nbetween bash and zsh altogether.\n\nØsse\n"},{"id":"250453","messageId":"xmqqwq8957yb.fsf@gitster.dls.corp.google.com","threadId":"37687","inReplyTo":"loom.20141009T214418-680@post.gmane.org","subject":"Re: [PATCH v4] completion: ignore chpwd_functions when cding on zsh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-09T20:01:00Z","receivedAt":"2014-10-09T20:01:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Øystein Walle <oystwa@gmail.com> writes:\n\n> BUT: Over a year ago Git learned the -C argument. Couldn't we use that\n> here? That way we would not have to unset CDPATH and can get rid of the\n> subshell and cd -q. If we allow the other functions to use several\n> arguments to pass options with we can get rid of the whole seperation\n> between bash and zsh altogether.\n\nWow, that is an excellent suggestion.  It would look like the\nattached, right?\n\nBy stepping away further and further from the originally proposed\nsolution and trying to identify the real problem that needs to be\nsolved, you reached a better solution ;-).\n\n contrib/completion/git-completion.bash | 16 ++++++----------\n 1 file changed, 6 insertions(+), 10 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 5ea5b82..f22de9d 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -281,16 +281,12 @@ __gitcomp_file ()\n # argument, and using the options specified in the second argument.\n __git_ls_files_helper ()\n {\n-\t(\n-\t\ttest -n \"${CDPATH+set}\" && unset CDPATH\n-\t\tcd \"$1\"\n-\t\tif [ \"$2\" == \"--committable\" ]; then\n-\t\t\tgit diff-index --name-only --relative HEAD\n-\t\telse\n-\t\t\t# NOTE: $2 is not quoted in order to support multiple options\n-\t\t\tgit ls-files --exclude-standard $2\n-\t\tfi\n-\t) 2>/dev/null\n+\tif [ \"$2\" == \"--committable\" ]; then\n+\t\tgit -C \"$1\" diff-index --name-only --relative HEAD\n+\telse\n+\t\t# NOTE: $2 is not quoted in order to support multiple options\n+\t\tgit -C \"$1\" ls-files --exclude-standard $2\n+\tfi 2>/dev/null\n }\n \n \n"},{"id":"250457","messageId":"xmqqk34955we.fsf@gitster.dls.corp.google.com","threadId":"37687","inReplyTo":"1412881298-64117-1-git-send-email-bt@brandonturner.net","subject":"Re: [PATCH v4] completion: ignore chpwd_functions when cding on zsh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-09T20:45:21Z","receivedAt":"2014-10-09T20:45:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Turner <bt@brandonturner.net> writes:\n\n> As Øystein pointed out, on zsh we can use \"cd -q\" to ignore\n> chpwd_functions.\n>\n> Junio - from my testing, unsetting CDPATH is sufficient on zsh.\n\nLet's do this instead, though.\n\nBugs are mine; as I do not use zsh myself, some testing is very much\nappreciated.\n\nThanks.\n\n-- >8 --\nSubject: [PATCH] completion: use \"git -C $there\" instead of (cd $there && git ...)\n\nWe have had \"git -C $there\" to first go to a different directory\nand run a Git command without changing the arguments for quite some\ntime.  Use it instead of (cd $there && git ...) in the completion\nscript.\n\nThis allows us to lose the work-around for misfeatures of modern\ninteractive-minded shells that make \"cd\" unusable in scripts (e.g.\nend users' $CDPATH taking us to unexpected places in any POSIX\nshell, and chpwd functions spewing unwanted output in zsh).\n\nBased on Øystein Walle's idea, which was raised during the\ndiscussion on the solution by Brandon Turner for a problem zsh users\nhad with RVM which mucks with chpwd_functions in users' environments\n(https://github.com/wayneeseguin/rvm/issues/3076).\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n contrib/completion/git-completion.bash | 16 ++++++----------\n 1 file changed, 6 insertions(+), 10 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex dba3c15..42f7308 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -263,16 +263,12 @@ __gitcomp_file ()\n # argument, and using the options specified in the second argument.\n __git_ls_files_helper ()\n {\n-\t(\n-\t\ttest -n \"${CDPATH+set}\" && unset CDPATH\n-\t\tcd \"$1\"\n-\t\tif [ \"$2\" == \"--committable\" ]; then\n-\t\t\tgit diff-index --name-only --relative HEAD\n-\t\telse\n-\t\t\t# NOTE: $2 is not quoted in order to support multiple options\n-\t\t\tgit ls-files --exclude-standard $2\n-\t\tfi\n-\t) 2>/dev/null\n+\tif [ \"$2\" == \"--committable\" ]; then\n+\t\tgit -C \"$1\" diff-index --name-only --relative HEAD\n+\telse\n+\t\t# NOTE: $2 is not quoted in order to support multiple options\n+\t\tgit -C \"$1\" ls-files --exclude-standard $2\n+\tfi 2>/dev/null\n }\n \n \n-- \n2.1.2-464-g5e996a3\n"},{"id":"250459","messageId":"CAMUzdX=SmeEFmxd_LPPaB9qkwqXfkiC=CU7DnMf_gR=007xcbQ@mail.gmail.com","threadId":"37687","inReplyTo":"xmqqk34955we.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v4] completion: ignore chpwd_functions when cding on zsh","fromName":"Brandon Turner","fromEmail":"bt@brandonturner.net","sentAt":"2014-10-09T22:04:10Z","receivedAt":"2014-10-09T22:04:10Z","isPatch":true,"sender":{"key":"bt@brandonturner.net","avatar":"https://gravatar.com/avatar/6d077ef330be095c42599e0b5d25bf1debd15aedc75ec154d233e82d8428b6c7?d=mp&s=160"},"body":"On Thu, Oct 9, 2014 at 3:45 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Bugs are mine; as I do not use zsh myself, some testing is very much\n> appreciated.\n\nI've tested this patch in zsh and it fixes the original problem.  I've\nalso tested various scenarios in bash and zsh (CDPATH set, different\nplaces within repos, etc.) and see no problems.\n\nThanks for all of the help Junio and Øystein.\n"},{"id":"250461","messageId":"xmqqbnpk6ggl.fsf@gitster.dls.corp.google.com","threadId":"37687","inReplyTo":"CAMUzdX=SmeEFmxd_LPPaB9qkwqXfkiC=CU7DnMf_gR=007xcbQ@mail.gmail.com","subject":"Re: [PATCH v4] completion: ignore chpwd_functions when cding on zsh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-09T22:11:54Z","receivedAt":"2014-10-09T22:11:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Turner <bt@brandonturner.net> writes:\n\n> On Thu, Oct 9, 2014 at 3:45 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>\n>> Bugs are mine; as I do not use zsh myself, some testing is very much\n>> appreciated.\n>\n> I've tested this patch in zsh and it fixes the original problem.  I've\n> also tested various scenarios in bash and zsh (CDPATH set, different\n> places within repos, etc.) and see no problems.\n>\n> Thanks for all of the help Junio and Øystein.\n\nActually the patch was slightly wrong.  It did not quite matter as\n\"cd ''\" is a no-op, but \"git -C '' cmd\" is not that lenient (which\nmay be something we may want to fix) and breaks t9902 by exposing\nan existing breakage in the callchain.\n\nHere is a replacement.\n\n-- >8 --\nFrom: Junio C Hamano <gitster@pobox.com>\nDate: Thu, 9 Oct 2014 13:45:21 -0700\nSubject: [PATCH] completion: use \"git -C $there\" instead of (cd $there && git\n ...)\nMIME-Version: 1.0\nContent-Type: text/plain; charset=UTF-8\nContent-Transfer-Encoding: 8bit\n\nWe have had \"git -C $there\" to first go to a different directory\nand run a Git command without changing the arguments for quite some\ntime.  Use it instead of (cd $there && git ...) in the completion\nscript.\n\nThis allows us to lose the work-around for misfeatures of modern\ninteractive-minded shells that make \"cd\" unusable in scripts (e.g.\nend users' $CDPATH taking us to unexpected places in any POSIX\nshell, and chpwd functions spewing unwanted output in zsh).\n\nBased on Øystein Walle's idea, which was raised during the\ndiscussion on the solution by Brandon Turner for a problem zsh users\nhad with RVM which mucks with chpwd_functions in users' environments\n(https://github.com/wayneeseguin/rvm/issues/3076).\n\nAs $root variable, which is used to direct where to chdir to, is set\nto \".\" based on if $2 to __git_index_files is set (not if it is empty),\nthe only caller of the function is fixed not to pass the optional $2\nwhen it does not want us to switch to a different directory.  Otherwise\nwe would end up doing \"git -C '' command...\", which would not work.\n\nMaybe we would want \"git -C '' command...\" to mean \"do not chdir\nanywhere\", but that is a spearate topic.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n contrib/completion/git-completion.bash | 18 +++++++-----------\n 1 file changed, 7 insertions(+), 11 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex dba3c15..6077925 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -263,16 +263,12 @@ __gitcomp_file ()\n # argument, and using the options specified in the second argument.\n __git_ls_files_helper ()\n {\n-\t(\n-\t\ttest -n \"${CDPATH+set}\" && unset CDPATH\n-\t\tcd \"$1\"\n-\t\tif [ \"$2\" == \"--committable\" ]; then\n-\t\t\tgit diff-index --name-only --relative HEAD\n-\t\telse\n-\t\t\t# NOTE: $2 is not quoted in order to support multiple options\n-\t\t\tgit ls-files --exclude-standard $2\n-\t\tfi\n-\t) 2>/dev/null\n+\tif [ \"$2\" == \"--committable\" ]; then\n+\t\tgit -C \"$1\" diff-index --name-only --relative HEAD\n+\telse\n+\t\t# NOTE: $2 is not quoted in order to support multiple options\n+\t\tgit -C \"$1\" ls-files --exclude-standard $2\n+\tfi 2>/dev/null\n }\n \n \n@@ -504,7 +500,7 @@ __git_complete_index_file ()\n \t\t;;\n \tesac\n \n-\t__gitcomp_file \"$(__git_index_files \"$1\" \"$pfx\")\" \"$pfx\" \"$cur_\"\n+\t__gitcomp_file \"$(__git_index_files \"$1\" ${pfx:+\"$pfx\"})\" \"$pfx\" \"$cur_\"\n }\n \n __git_complete_file ()\n-- \n2.1.2-466-g338ee7a\n"},{"id":"250462","messageId":"CAMUzdXkWNxW8Py6ATwtvqJ7s75dsP8vz6gMjk6tQa6gTGvcdWw@mail.gmail.com","threadId":"37687","inReplyTo":"xmqqbnpk6ggl.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v4] completion: ignore chpwd_functions when cding on zsh","fromName":"Brandon Turner","fromEmail":"bt@brandonturner.net","sentAt":"2014-10-09T22:30:22Z","receivedAt":"2014-10-09T22:30:22Z","isPatch":true,"sender":{"key":"bt@brandonturner.net","avatar":"https://gravatar.com/avatar/6d077ef330be095c42599e0b5d25bf1debd15aedc75ec154d233e82d8428b6c7?d=mp&s=160"},"body":"On Thu, Oct 9, 2014 at 5:11 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Actually the patch was slightly wrong.  It did not quite matter as\n> \"cd ''\" is a no-op, but \"git -C '' cmd\" is not that lenient (which\n> may be something we may want to fix) and breaks t9902 by exposing\n> an existing breakage in the callchain.\n>\n> Here is a replacement.\n\nI did some more testing on this iteration as well - looks good. :-)\n"},{"id":"250761","messageId":"loom.20141016T200635-928@post.gmane.org","threadId":"37687","inReplyTo":"CAMUzdXkWNxW8Py6ATwtvqJ7s75dsP8vz6gMjk6tQa6gTGvcdWw@mail.gmail.com","subject":"Re: [PATCH v4] completion: ignore chpwd_functions when cding on zsh","fromName":"Øystein Walle","fromEmail":"oystwa@gmail.com","sentAt":"2014-10-16T18:10:14Z","receivedAt":"2014-10-16T18:10:14Z","isPatch":true,"sender":{"key":"oystwa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/794585?v=4"},"body":"Brandon Turner <bt <at> brandonturner.net> writes:\n\n> \n> On Thu, Oct 9, 2014 at 5:11 PM, Junio C Hamano <gitster <at> pobox.com> wrote:\n> > Actually the patch was slightly wrong.  It did not quite matter as\n> > \"cd ''\" is a no-op, but \"git -C '' cmd\" is not that lenient (which\n> > may be something we may want to fix) and breaks t9902 by exposing\n> > an existing breakage in the callchain.\n> >\n> > Here is a replacement.\n> \n> I did some more testing on this iteration as well - looks good. \n> \n\nSorry, for the late reply, guys...\n\nI've tested it too and it seems to work just fine.\n\nAs for my comments about using $=2 instead of $2: When zsh runs through\nthis code it does so while emulating ksh. Thus the reliance on splitting\nunquoted expansions is not a problem. I was unaware of this until ten\nminutes ago. \n\nØsse\n"}]}