Re: [PATCH v2 1/4] completion: simplify __git_remotes
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:52:53
Felipe Contreras wrote:
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? 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.
quoted hunk
Signed-off-by: Felipe Contreras <redacted> --- 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