Re: [PATCH v4] completion: ignore chpwd_functions when cding on zsh

[Junio C Hamano <gitster@pobox.com>]

Brandon Turner <bt@brandonturner.net> writes:

> On Thu, Oct 9, 2014 at 3:45 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Bugs are mine; as I do not use zsh myself, some testing is very much
>> appreciated.
>
> I've tested this patch in zsh and it fixes the original problem.  I've
> also tested various scenarios in bash and zsh (CDPATH set, different
> places within repos, etc.) and see no problems.
>
> Thanks for all of the help Junio and Øystein.

Actually the patch was slightly wrong.  It did not quite matter as
"cd ''" is a no-op, but "git -C '' cmd" is not that lenient (which
may be something we may want to fix) and breaks t9902 by exposing
an existing breakage in the callchain.

Here is a replacement.

-- >8 --
From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 9 Oct 2014 13:45:21 -0700
Subject: [PATCH] completion: use "git -C $there" instead of (cd $there && git
 ...)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We have had "git -C $there" to first go to a different directory
and run a Git command without changing the arguments for quite some
time.  Use it instead of (cd $there && git ...) in the completion
script.

This allows us to lose the work-around for misfeatures of modern
interactive-minded shells that make "cd" unusable in scripts (e.g.
end users' $CDPATH taking us to unexpected places in any POSIX
shell, and chpwd functions spewing unwanted output in zsh).

Based on Øystein Walle's idea, which was raised during the
discussion on the solution by Brandon Turner for a problem zsh users
had with RVM which mucks with chpwd_functions in users' environments
(https://github.com/wayneeseguin/rvm/issues/3076).

As $root variable, which is used to direct where to chdir to, is set
to "." based on if $2 to __git_index_files is set (not if it is empty),
the only caller of the function is fixed not to pass the optional $2
when it does not want us to switch to a different directory.  Otherwise
we would end up doing "git -C '' command...", which would not work.

Maybe we would want "git -C '' command..." to mean "do not chdir
anywhere", but that is a spearate topic.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 contrib/completion/git-completion.bash | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index dba3c15..6077925 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -263,16 +263,12 @@ __gitcomp_file ()
 # argument, and using the options specified in the second argument.
 __git_ls_files_helper ()
 {
-	(
-		test -n "${CDPATH+set}" && unset CDPATH
-		cd "$1"
-		if [ "$2" == "--committable" ]; then
-			git diff-index --name-only --relative HEAD
-		else
-			# NOTE: $2 is not quoted in order to support multiple options
-			git ls-files --exclude-standard $2
-		fi
-	) 2>/dev/null
+	if [ "$2" == "--committable" ]; then
+		git -C "$1" diff-index --name-only --relative HEAD
+	else
+		# NOTE: $2 is not quoted in order to support multiple options
+		git -C "$1" ls-files --exclude-standard $2
+	fi 2>/dev/null
 }
 
 
@@ -504,7 +500,7 @@ __git_complete_index_file ()
 		;;
 	esac
 
-	__gitcomp_file "$(__git_index_files "$1" "$pfx")" "$pfx" "$cur_"
+	__gitcomp_file "$(__git_index_files "$1" ${pfx:+"$pfx"})" "$pfx" "$cur_"
 }
 
 __git_complete_file ()
-- 
2.1.2-466-g338ee7a