Problem completing remotes when .git/remotes exits

I have an empty .git/remotes directory. Trying to complete the name of
a remote always reports an error:

git@master:1023> git fetch <TAB>ls: invalid option -- ' '
Try `ls --help' for more information.

I have these:

	alias ls='ls $LS_OPTIONS'
and
	LS_OPTIONS='-N --color=tty -T 0'

I instrumented __git_remotes with set -x, which shows:

git@master:1006> git fetch <TAB>+++ __gitdir

+++ '[' -z '' ']'
+++ '[' -n '' ']'
+++ '[' -n '' ']'
+++ '[' -d .git ']'
+++ echo .git
++ local i 'IFS=

' d=.git
++ test -d .git/remotes
++ ls '-N --color=tty -T 0' -1 .git/remotes
ls: invalid option -- ' '
Try `ls --help' for more information.
...

Notice that the expansion of $LS_OPTIONS is not split at the blanks,
obviously, because $IFS does not contain a blank at that moment.

The patch below helps, but it looks like a work-around rather than a
solution. Ideas?

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index be800e0..824f5b6 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash

@@ -369,8 +369,9 @@ __git_refs_remotes ()
 
 __git_remotes ()
 {
-	local i IFS=$'\n' d="$(__gitdir)"
+	local i d="$(__gitdir)"
 	test -d "$d/remotes" && ls -1 "$d/remotes"
+	local IFS=$'\n'
 	for i in $(git --git-dir="$d" config --get-regexp 'remote\..*\.url' 2>/dev/null); do
 		i="${i#remote.}"
 		echo "${i/.url*/}"

Hi,


On Wed, Sep 19, 2012 at 09:55:28PM +0200, Johannes Sixt wrote:

I've got two alternative solutions for this issue.

The first one is less intrusive: use the 'command' builtin to tell
the shell to ignore shell functions and aliases and just run the ls
command.

diff --git a/contrib/completion/git-completion.bash

b/contrib/completion/git-completion.bash
index be800e09..bcde9472 100644

--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash

@@ -370,7 +370,7 @@ __git_refs_remotes ()
 __git_remotes ()
 {
 	local i IFS=$'\n' d="$(__gitdir)"
-	test -d "$d/remotes" && ls -1 "$d/remotes"
+	test -d "$d/remotes" && command 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*/}"


But then it got me thinking...  Notice how much effort we spend just
to get the list of remotes?  We could just run 'git remote' directly
instead...

diff --git a/contrib/completion/git-completion.bash

b/contrib/completion/git-completion.bash
index be800e09..1daeaccf 100644

--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash

@@ -369,12 +369,8 @@ __git_refs_remotes ()
 
 __git_remotes ()
 {
-	local i IFS=$'\n' d="$(__gitdir)"
-	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*/}"
-	done
+	local d="$(__gitdir)"
+	git --git-dir="$d" remote
 }
 
 __git_list_merge_strategies ()

I prefer the second one ;)

Best,
Gábor

On Wed, Sep 26, 2012 at 01:00:45AM +0200, SZEDER Gábor wrote:

But then it got me thinking...  Notice how much effort we spend just
to get the list of remotes?  We could just run 'git remote' directly
instead...

Actually, we can't, because 'git remote' doesn't seem to list remotes
stored under .git/remotes.  Is that intentional?

Anyway, we could still use 'git remote' to replace at least the config
query and the for loop to spare a few lines of code and a subshell.

On Wed, Sep 26, 2012 at 03:09:38AM +0200, SZEDER Gábor wrote:

On Wed, Sep 26, 2012 at 01:00:45AM +0200, SZEDER Gábor wrote:

quoted

But then it got me thinking...  Notice how much effort we spend just
to get the list of remotes?  We could just run 'git remote' directly
instead...

Actually, we can't, because 'git remote' doesn't seem to list remotes
stored under .git/remotes.  Is that intentional?

Looks like a bug, as it seems to have been lost in translation in
211c8968 (Make git-remote a builtin, 2008-02-29).

list_remote() in git-remote.perl looks for remotes in config and in
.git/remotes/, too.  The builtin implementation uses
remote.c:for_each_remote() from the start, which only looks at the
config.