{"thread":{"id":"33154","subject":"[RFC/PATCH] git-completion.bash: remove bashism to fix ZSH compatibility","startedAt":"2013-03-11T12:21:27Z","lastAt":"2013-03-11T19:09:50Z","messageCount":12,"participants":["Matthieu Moy","Junio C Hamano","Manlio Perillo","Paul Smith"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"211039","messageId":"1363004487-1193-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"33154","inReplyTo":null,"subject":"[RFC/PATCH] git-completion.bash: remove bashism to fix ZSH compatibility","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2013-03-11T12:21:27Z","receivedAt":"2013-03-11T12:21:27Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"The function-wide redirection used for __git_ls_files_helper and\n__git_diff_index_helper work only with bash. Using ZSH, trying to\ncomplete an inexistant directory gave this:\n\n  git add no-such-dir/__git_ls_files_helper:cd:2: no such file or directory: no-such-dir/\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\nThese two instances seem to be the only ones in the file.\n\nI'm not sure whether the 2>/dev/null would be needed for the command\non the RHS of the && too (git ls-files and git diff-index).\n\n contrib/completion/git-completion.bash | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex b62bec0..0640274 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -300,8 +300,8 @@ __git_index_file_list_filter ()\n __git_ls_files_helper ()\n {\n \t# NOTE: $2 is not quoted in order to support multiple options\n-\tcd \"$1\" && git ls-files --exclude-standard $2\n-} 2>/dev/null\n+\tcd \"$1\" 2>/dev/null && git ls-files --exclude-standard $2\n+}\n \n \n # Execute git diff-index, returning paths relative to the directory\n@@ -309,8 +309,8 @@ __git_ls_files_helper ()\n # specified in the second argument.\n __git_diff_index_helper ()\n {\n-\tcd \"$1\" && git diff-index --name-only --relative \"$2\"\n-} 2>/dev/null\n+\tcd \"$1\" 2>/dev/null && git diff-index --name-only --relative \"$2\"\n+}\n \n # __git_index_files accepts 1 or 2 arguments:\n # 1: Options to pass to ls-files (required).\n-- \n1.8.2.rc3.16.g0a33571.dirty\n"},{"id":"211052","messageId":"7v38w1c3ms.fsf@alter.siamese.dyndns.org","threadId":"33154","inReplyTo":"1363004487-1193-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [RFC/PATCH] git-completion.bash: remove bashism to fix ZSH compatibility","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-11T16:17:31Z","receivedAt":"2013-03-11T16:17:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n> The function-wide redirection used for __git_ls_files_helper and\n> __git_diff_index_helper work only with bash. Using ZSH, trying to\n> complete an inexistant directory gave this:\n>\n>   git add no-such-dir/__git_ls_files_helper:cd:2: no such file or directory: no-such-dir/\n>\n> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n> ---\n\nThis is not bash-ism but POSIX.1, even though it is not very well\nknown.  I recall commenting on this exact pattern during the review.\n\n  http://thread.gmane.org/gmane.comp.version-control.git/213232/focus=213286\n\nAfter all, I was right when I said that some implementations may get\nit wrong and we shouldn't use the construct X-<.\n\n> These two instances seem to be the only ones in the file.\n>\n> I'm not sure whether the 2>/dev/null would be needed for the command\n> on the RHS of the && too (git ls-files and git diff-index).\n\nIt would not hurt to discard their standard error.\n\n>  contrib/completion/git-completion.bash | 8 ++++----\n>  1 file changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index b62bec0..0640274 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -300,8 +300,8 @@ __git_index_file_list_filter ()\n>  __git_ls_files_helper ()\n>  {\n>  \t# NOTE: $2 is not quoted in order to support multiple options\n> -\tcd \"$1\" && git ls-files --exclude-standard $2\n> -} 2>/dev/null\n> +\tcd \"$1\" 2>/dev/null && git ls-files --exclude-standard $2\n> +}\n>  \n>  \n>  # Execute git diff-index, returning paths relative to the directory\n> @@ -309,8 +309,8 @@ __git_ls_files_helper ()\n>  # specified in the second argument.\n>  __git_diff_index_helper ()\n>  {\n> -\tcd \"$1\" && git diff-index --name-only --relative \"$2\"\n> -} 2>/dev/null\n> +\tcd \"$1\" 2>/dev/null && git diff-index --name-only --relative \"$2\"\n> +}\n>  \n>  # __git_index_files accepts 1 or 2 arguments:\n>  # 1: Options to pass to ls-files (required).\n"},{"id":"211055","messageId":"7vobepany3.fsf@alter.siamese.dyndns.org","threadId":"33154","inReplyTo":"7v38w1c3ms.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] git-completion.bash: remove bashism to fix ZSH compatibility","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-11T16:41:40Z","receivedAt":"2013-03-11T16:41:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> After all, I was right when I said that some implementations may get\n> it wrong and we shouldn't use the construct X-<.\n>\n>> These two instances seem to be the only ones in the file.\n>>\n>> I'm not sure whether the 2>/dev/null would be needed for the command\n>> on the RHS of the && too (git ls-files and git diff-index).\n>\n> It would not hurt to discard their standard error.\n\nSo here is an updated based on your patch.\n\n-- >8 --\nFrom: Matthieu Moy <Matthieu.Moy@imag.fr>\nDate: Mon, 11 Mar 2013 13:21:27 +0100\nSubject: [PATCH] git-completion.bash: zsh does not implement function\n redirection correctly\n\nA recent change added functions whose entire standard error stream\nis redirected to /dev/null using a construct that is valid POSIX.1\nbut is not widely used:\n\n\tfuncname () {\n\t\tfuncbody\n\t} 2>/dev/null\n\nEven though this file is \"git-completion.bash\", zsh completion\nsupport dot-sources it (instead of asking bash to grok it like tcsh\ncompletion does), and zsh does not implement this redirection\ncorrectly.\n\nWith zsh, trying to complete an inexistant directory gave this:\n\n  git add no-such-dir/__git_ls_files_helper:cd:2: no such file or directory: no-such-dir/\n\nIt is easy to work around by refraining from using this construct.\nThe correct thing to do in the longer term may be to stop dot-sourcing\nthe source meant for bash into zsh, but this patch should suffice as\na band-aid in the meantime.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n contrib/completion/git-completion.bash | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 51b8b3b..3d4cc7c 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -300,8 +300,8 @@ __git_index_file_list_filter ()\n __git_ls_files_helper ()\n {\n \t# NOTE: $2 is not quoted in order to support multiple options\n-\tcd \"$1\" && git ls-files --exclude-standard $2\n-} 2>/dev/null\n+\tcd \"$1\" 2>/dev/null && git ls-files --exclude-standard $2 2>/dev/null\n+}\n \n \n # Execute git diff-index, returning paths relative to the directory\n@@ -309,8 +309,8 @@ __git_ls_files_helper ()\n # specified in the second argument.\n __git_diff_index_helper ()\n {\n-\tcd \"$1\" && git diff-index --name-only --relative \"$2\"\n-} 2>/dev/null\n+\tcd \"$1\" 2>/dev/null && git diff-index --name-only --relative \"$2\" 2>/dev/null\n+}\n \n # __git_index_files accepts 1 or 2 arguments:\n # 1: Options to pass to ls-files (required).\n-- \n1.8.2-rc3-271-g00e868e\n"},{"id":"211057","messageId":"vpqtxohubmb.fsf@grenoble-inp.fr","threadId":"33154","inReplyTo":"7vobepany3.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] git-completion.bash: remove bashism to fix ZSH compatibility","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-03-11T16:47:40Z","receivedAt":"2013-03-11T16:47:40Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> So here is an updated based on your patch.\n\nPerfect, thanks.\n\n> The correct thing to do in the longer term may be to stop dot-sourcing\n> the source meant for bash into zsh, but this patch should suffice as\n> a band-aid in the meantime.\n\nI disagree with this particular part though. I think using the same code\nfor bash and zsh makes sense, and it implies restricting to the common\nsubset. I don't consider it \"band-aid\", but \"nice code factoring\" ;-).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"211059","messageId":"7vfw01an1b.fsf@alter.siamese.dyndns.org","threadId":"33154","inReplyTo":"vpqtxohubmb.fsf@grenoble-inp.fr","subject":"Re: [RFC/PATCH] git-completion.bash: remove bashism to fix ZSH compatibility","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-11T17:01:20Z","receivedAt":"2013-03-11T17:01:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> So here is an updated based on your patch.\n>\n> Perfect, thanks.\n>\n>> The correct thing to do in the longer term may be to stop dot-sourcing\n>> the source meant for bash into zsh, but this patch should suffice as\n>> a band-aid in the meantime.\n>\n> I disagree with this particular part though. I think using the same code\n> for bash and zsh makes sense, and it implies restricting to the common\n> subset.\n\nHaving to restrict to the common subset means that whenever bash\nadds new and useful features that this script could take advantage\nof to improve the user experience, they cannot be employed until zsh\ncatches up (and worse yet, it is outside the control of this script\nif zsh may ever catch up in the specific feature).\n"},{"id":"211061","messageId":"513E0FB4.40607@gmail.com","threadId":"33154","inReplyTo":"7v38w1c3ms.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] git-completion.bash: remove bashism to fix ZSH compatibility","fromName":"Manlio Perillo","fromEmail":"manlio.perillo@gmail.com","sentAt":"2013-03-11T17:09:08Z","receivedAt":"2013-03-11T17:09:08Z","isPatch":true,"sender":{"key":"manlio.perillo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6217088?v=4"},"body":"-----BEGIN PGP SIGNED MESSAGE-----\nHash: SHA1\n\nIl 11/03/2013 17:17, Junio C Hamano ha scritto:\n> Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n> \n>> The function-wide redirection used for __git_ls_files_helper and\n>> __git_diff_index_helper work only with bash. Using ZSH, trying to\n>> complete an inexistant directory gave this:\n>>\n>>   git add no-such-dir/__git_ls_files_helper:cd:2: no such file or directory: no-such-dir/\n>>\n>> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n>> ---\n> \n> This is not bash-ism but POSIX.1, even though it is not very well\n> known.  I recall commenting on this exact pattern during the review.\n> \n\nYes, I was plainning to send another patch to fix this (and your other\nsuggestion regarding the CDPATH environment variable, if I remember\ncorrectly), but I was busy with other things; sorry.\n\n\n\n> [...]\n\n\nRegards  Manlio\n-----BEGIN PGP SIGNATURE-----\nVersion: GnuPG v1.4.10 (GNU/Linux)\nComment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/\n\niEYEARECAAYFAlE+D7QACgkQscQJ24LbaURBTgCffpMCPjmcsP53/WE/VIQ2FIIc\nfiIAn3obBJ1yrHVUEmslz32ezvESCZ4G\n=7nia\n-----END PGP SIGNATURE-----\n"},{"id":"211062","messageId":"513E1099.9070104@gmail.com","threadId":"33154","inReplyTo":"7vfw01an1b.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] git-completion.bash: remove bashism to fix ZSH compatibility","fromName":"Manlio Perillo","fromEmail":"manlio.perillo@gmail.com","sentAt":"2013-03-11T17:12:57Z","receivedAt":"2013-03-11T17:12:57Z","isPatch":true,"sender":{"key":"manlio.perillo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6217088?v=4"},"body":"-----BEGIN PGP SIGNED MESSAGE-----\nHash: SHA1\n\nIl 11/03/2013 18:01, Junio C Hamano ha scritto:\n> [...]\n> Having to restrict to the common subset means that whenever bash\n> adds new and useful features that this script could take advantage\n> of to improve the user experience, they cannot be employed until zsh\n> catches up (and worse yet, it is outside the control of this script\n> if zsh may ever catch up in the specific feature).\n> \n\nMaybe, to avoid this problem and code duplication (the main reason bash\nscript is sourced, as far as I can tell), it may be useful to add\nadditional reusable git commands, for use in shell completion?\n\nE.g:\n\tgit suggest <cmd> *args\n\nreturns a line separed list of filenames affected by cmd.\n\n\n\nRegards  Manlio\n-----BEGIN PGP SIGNATURE-----\nVersion: GnuPG v1.4.10 (GNU/Linux)\nComment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/\n\niEYEARECAAYFAlE+EJkACgkQscQJ24LbaURjNwCfdW73fET/n4FRGftKcSJPsK7M\nnu4An1CC0dspGxLe5zqR9BdXBBDHWl/Y\n=11j7\n-----END PGP SIGNATURE-----\n"},{"id":"211063","messageId":"7v8v5talzu.fsf@alter.siamese.dyndns.org","threadId":"33154","inReplyTo":"513E0FB4.40607@gmail.com","subject":"Re: [RFC/PATCH] git-completion.bash: remove bashism to fix ZSH compatibility","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-11T17:23:49Z","receivedAt":"2013-03-11T17:23:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Manlio Perillo <manlio.perillo@gmail.com> writes:\n\n> Yes, I was plainning to send another patch to fix this (and your other\n> suggestion regarding the CDPATH environment variable, if I remember\n> correctly),...\n\nAhh, thanks for reminding me of this.  You are right; these two\nfunctions are broken when the user has CDPATH set, I think.\n\nHere is a reroll.\n\n-- >8 --\nFrom: Matthieu Moy <Matthieu.Moy@imag.fr>\nDate: Mon, 11 Mar 2013 13:21:27 +0100\nSubject: [PATCH] git-completion.bash: zsh does not implement function\n redirection correctly\n\nA recent change added functions whose entire standard error stream\nis redirected to /dev/null using a construct that is valid POSIX.1\nbut is not widely used:\n\n\tfuncname () {\n\t\tcd \"$1\" && run some command \"$2\"\n\t} 2>/dev/null\n\nEven though this file is \"git-completion.bash\", zsh completion\nsupport dot-sources it (instead of asking bash to grok it like tcsh\ncompletion does), and zsh does not implement this redirection\ncorrectly.\n\nWith zsh, trying to complete an inexistant directory gave this:\n\n  git add no-such-dir/__git_ls_files_helper:cd:2: no such file or directory: no-such-dir/\n\nAlso these functions use \"cd\" to first go somewhere else before\nrunning a command, but the location the caller wants them to go that\nis given as an argument to them should not be affected by CDPATH\nvariable the users may have set for their interactive session.\n\nTo fix both of these, wrap the body of the function in a subshell,\nunset CDPATH at the beginning of the subshell, and redirect the\nstandard error stream of the subshell to /dev/null.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n contrib/completion/git-completion.bash | 16 +++++++++++-----\n 1 file changed, 11 insertions(+), 5 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 51b8b3b..430566d 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -299,9 +299,12 @@ __git_index_file_list_filter ()\n # the second argument.\n __git_ls_files_helper ()\n {\n-\t# NOTE: $2 is not quoted in order to support multiple options\n-\tcd \"$1\" && git ls-files --exclude-standard $2\n-} 2>/dev/null\n+\t(\n+\t\ttest -n \"${CDPATH+set}\" && unset CDPATH\n+\t\t# NOTE: $2 is not quoted in order to support multiple options\n+\t\tcd \"$1\" && git ls-files --exclude-standard $2\n+\t) 2>/dev/null\n+}\n \n \n # Execute git diff-index, returning paths relative to the directory\n@@ -309,8 +312,11 @@ __git_ls_files_helper ()\n # specified in the second argument.\n __git_diff_index_helper ()\n {\n-\tcd \"$1\" && git diff-index --name-only --relative \"$2\"\n-} 2>/dev/null\n+\t(\n+\t\ttest -n \"${CDPATH+set}\" && unset CDPATH\n+\t\tcd \"$1\" && git diff-index --name-only --relative \"$2\"\n+\t) 2>/dev/null\n+}\n \n # __git_index_files accepts 1 or 2 arguments:\n # 1: Options to pass to ls-files (required).\n-- \n1.8.2-rc3-219-ge56455f\n"},{"id":"211064","messageId":"vpqppz5u8te.fsf@grenoble-inp.fr","threadId":"33154","inReplyTo":"7v8v5talzu.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] git-completion.bash: remove bashism to fix ZSH compatibility","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-03-11T17:48:13Z","receivedAt":"2013-03-11T17:48:13Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Ahh, thanks for reminding me of this.  You are right; these two\n> functions are broken when the user has CDPATH set, I think.\n>\n> Here is a reroll.\n\nThanks. Even nicer that the previous since the CDPATH implied the\nsubshell anyway.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"211065","messageId":"7vwqtd95bm.fsf@alter.siamese.dyndns.org","threadId":"33154","inReplyTo":"vpqppz5u8te.fsf@grenoble-inp.fr","subject":"Re: [RFC/PATCH] git-completion.bash: remove bashism to fix ZSH compatibility","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-11T18:09:17Z","receivedAt":"2013-03-11T18:09:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Ahh, thanks for reminding me of this.  You are right; these two\n>> functions are broken when the user has CDPATH set, I think.\n>>\n>> Here is a reroll.\n>\n> Thanks. Even nicer that the previous since the CDPATH implied the\n> subshell anyway.\n\nActually, \"cd\", not CDPATH, is what implies that the caller must be\ncalling us in a subshell, e.g.\n\n\tresult=$(__git_ls_files_helper dir/ args...)\n\nOtherwise the user's shell would have been taken to an unexpected\nplace, with or without CDPATH.\n\nSo strictly speaking there is no reason for an extra subshell here,\nbut writing this in the way the patch does makes our intention\ncrystal clear, I think.\n\nIn any case, let's queue this fix for the 1.8.2 final.  The CDPATH\nthing will affect not just zsh but bash users.\n"},{"id":"211066","messageId":"1363025984.24833.19.camel@pdsdesk","threadId":"33154","inReplyTo":"7vwqtd95bm.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] git-completion.bash: remove bashism to fix ZSH compatibility","fromName":"Paul Smith","fromEmail":"paul@mad-scientist.net","sentAt":"2013-03-11T18:19:44Z","receivedAt":"2013-03-11T18:19:44Z","isPatch":true,"sender":{"key":"paul@mad-scientist.net","avatar":"https://avatars.githubusercontent.com/u/109636?v=4"},"body":"On Mon, 2013-03-11 at 11:09 -0700, Junio C Hamano wrote:\n> So strictly speaking there is no reason for an extra subshell here,\n> but writing this in the way the patch does makes our intention\n> crystal clear, I think.\n\nIf you're concerned about the extra processing of the new shell you can\nuse {} instead of ():\n\n        {\n            test -n \"${CDPATH+set}\" && unset CDPATH\n            # NOTE: $2 is not quoted in order to support multiple options\n            cd \"$1\" && git ls-files --exclude-standard $2\n        } 2>/dev/null\n\nZsh does support this properly in my testing.  It's only redirection of\nan entire function body, as the original, that is working differently in\nzsh and bash.\n"},{"id":"211068","messageId":"513E2BFE.3030409@gmail.com","threadId":"33154","inReplyTo":"7vwqtd95bm.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] git-completion.bash: remove bashism to fix ZSH compatibility","fromName":"Manlio Perillo","fromEmail":"manlio.perillo@gmail.com","sentAt":"2013-03-11T19:09:50Z","receivedAt":"2013-03-11T19:09:50Z","isPatch":true,"sender":{"key":"manlio.perillo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6217088?v=4"},"body":"-----BEGIN PGP SIGNED MESSAGE-----\nHash: SHA1\n\nIl 11/03/2013 19:09, Junio C Hamano ha scritto:\n> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n> \n>> Junio C Hamano <gitster@pobox.com> writes:\n>>\n>>> Ahh, thanks for reminding me of this.  You are right; these two\n>>> functions are broken when the user has CDPATH set, I think.\n>>>\n>>> Here is a reroll.\n>>\n>> Thanks. Even nicer that the previous since the CDPATH implied the\n>> subshell anyway.\n> \n> Actually, \"cd\", not CDPATH, is what implies that the caller must be\n> calling us in a subshell, e.g.\n> \n> \tresult=$(__git_ls_files_helper dir/ args...)\n> \n> Otherwise the user's shell would have been taken to an unexpected\n> place, with or without CDPATH.\n> \n\nRight; this is the reason I used the `{` grouping, instead of `(`.\n\nHowever, since the `{` is already specified when the function is\ndefined, I did not add another `{}` grouping.\n\n> [...]\n\n\nRegards  Manlio Perillo\n-----BEGIN PGP SIGNATURE-----\nVersion: GnuPG v1.4.10 (GNU/Linux)\nComment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/\n\niEYEARECAAYFAlE+K/4ACgkQscQJ24LbaUQqvwCgmReHb4VtMJDT+tv+XF9RPmXE\nDlEAnjhsgXszSBVG1iW0WCLM6212+fdA\n=SYzh\n-----END PGP SIGNATURE-----\n"}]}