Felipe Contreras wrote:
> From: Felipe Contreras <felipe.contreras@xxxxxxxxx>
>
> 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?
There's a potential speed regression involved --- using "ls" involves
an extra fork/exec. I believe you have thought about this and done a
little to mitigate it; could you explain this in the commit message so
future coders know what features of your code need to be preserved?
Please consider squashing this with patch 2/4. It will make both
patches way easier to understand on their own.
Cc-ing Gábor, who I imagine is more familiar with this code than I am.
> Signed-off-by: Felipe Contreras <felipe.contreras@xxxxxxxxx>
> ---
> contrib/completion/git-completion.bash | 7 +------
> 1 files changed, 1 insertions(+), 6 deletions(-)
>
> 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"
> for i in $(git --git-dir="$d" config --get-regexp 'remote\..*\.url' 2>/dev/null); do
> i="${i#remote.}"
> echo "${i/.url*/}"
> --
> 1.7.8
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html