Re: [PATCH v2 1/4] completion: simplify __git_remotes

Jonathan Nieder [off-list ref] writes:

Felipe Contreras wrote:

quoted

From: Felipe Contreras <redacted>

There's no need for all that complicated code that requires nullglob,
and the complexities related to such option.

As an advantage, this would allow us to get rid of __git_shopt, which is
used only in this fuction to enable 'nullglob' in zsh.

That is all a longwinded way to say "zsh doesn't support the same
interface as bash for setting the nullglob option, so let's avoid
it and use 'ls' which is simpler", right?

;-)

quoted

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 1496c6d..086e38d 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash

@@ -644,12 +644,7 @@ __git_refs_remotes ()
 __git_remotes ()
 {
 	local i ngoff IFS=$'\n' d="$(__gitdir)"
-	__git_shopt -q nullglob || ngoff=1
-	__git_shopt -s nullglob
-	for i in "$d/remotes"/*; do
-		echo ${i#$d/remotes/}
-	done
-	[ "$ngoff" ] && __git_shopt -u nullglob
+	test -d "$d/remotes" && ls -1 "$d/remotes"

Yeah, very nice reduction of unnecessary code.

The original loop might have been justifiable if it were doing something
more meaningful inside (e.g. making sure the file really describes a
remote), but as far as I can tell, it merely is a poor-man's emulation of
"ls -1".

You updated it to make the code say what it wanted to say in the way it
should have said from day one ;-).