From: Junio C Hamano Date: Mon, 30 Jan 2012 18:27:30 GMT Subject: Re: [PATCH v2 1/4] completion: simplify __git_remotes Message-ID: <7v8vkpcagd.fsf@alter.siamese.dyndns.org> In-Reply-To: <20120130173446.GF10618@burratino> Jonathan Nieder writes: > Felipe Contreras wrote: > >> From: Felipe Contreras >> >> 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? ;-) >> 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 ;-).