[PATCH] contrib/completion: suppress stderror in bash completion of git remotes

[PATCH] contrib/completion: suppress stderror in bash completion of git remotes

[Matt Korostoff]

In some system configurations there is a bug with the
__git_remotes function.  Specifically, there is a problem
with line 415, `test -d "$d/remotes" && ls -1 "$d/remotes"`.
While `test -d` is meant to prevent listing the remotes
directory if it does not exist, in some system, `ls` will
run regardless.

This results in an error in which typing `git push or` + `tab`
prints out `ls: .git/remotes: No such file or directory`.
This can be fixed by simply directing stderror of this line
to /dev/null.
---
 contrib/completion/git-completion.bash |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2fece98..72251cc 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -412,7 +412,7 @@ __git_refs_remotes ()
 __git_remotes ()
 {
 	local i IFS=$'\n' d="$(__gitdir)"
-	test -d "$d/remotes" && ls -1 "$d/remotes"
+	test -d "$d/remotes" && ls -1 "$d/remotes" 2>/dev/null
 	for i in $(git --git-dir="$d" config --get-regexp 'remote\..*\.url' 2>/dev/null); do
 		i="${i#remote.}"
 		echo "${i/.url*/}"
-- 
1.7.10.2 (Apple Git-33)

Re: [PATCH] contrib/completion: suppress stderror in bash completion of git remotes

[Matt Korostoff]

Here are some screen shots demonstrating the issue I'm describing here:

before this patch:
https://cloud.githubusercontent.com/assets/1197335/6108333/1f3b10fa-b040-11e4-9164-3c7769dae110.gif

after this patch:
https://cloud.githubusercontent.com/assets/1197335/6108340/3878cad0-b040-11e4-9994-dcd5c4d62bba.gif

On Mon, Feb 9, 2015 at 3:58 PM, Matt Korostoff <mkorostoff@gmail.com> wrote:
> In some system configurations there is a bug with the
> __git_remotes function.  Specifically, there is a problem
> with line 415, `test -d "$d/remotes" && ls -1 "$d/remotes"`.
> While `test -d` is meant to prevent listing the remotes
> directory if it does not exist, in some system, `ls` will
> run regardless.
>
> This results in an error in which typing `git push or` + `tab`
> prints out `ls: .git/remotes: No such file or directory`.
> This can be fixed by simply directing stderror of this line
> to /dev/null.
> ---
>  contrib/completion/git-completion.bash |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 2fece98..72251cc 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -412,7 +412,7 @@ __git_refs_remotes ()
>  __git_remotes ()
>  {
>         local i IFS=$'\n' d="$(__gitdir)"
> -       test -d "$d/remotes" && ls -1 "$d/remotes"
> +       test -d "$d/remotes" && ls -1 "$d/remotes" 2>/dev/null
>         for i in $(git --git-dir="$d" config --get-regexp 'remote\..*\.url' 2>/dev/null); do
>                 i="${i#remote.}"
>                 echo "${i/.url*/}"
> --
> 1.7.10.2 (Apple Git-33)
>

Re: [PATCH] contrib/completion: suppress stderror in bash completion of git remotes

[Junio C Hamano]

Matt Korostoff <mkorostoff@gmail.com> writes:

> In some system configurations there is a bug with the
> __git_remotes function.  Specifically, there is a problem
> with line 415, `test -d "$d/remotes" && ls -1 "$d/remotes"`.
> While `test -d` is meant to prevent listing the remotes
> directory if it does not exist, in some system, `ls` will
> run regardless.

What's "some system"?

Is this a platform's bug (e.g. "test -d" does not work correctly)?

Is this an configuration error of user's Git repository?

Is this something else?

I _think_ you would see the problem if $d/remotes is a directory
whose contents cannot be listed (e.g. "chmod a= $d/remotes"), and
that would not be a platform's bug (i.e. "test -d" would happily say
"Yes there is a directory", and "ls" would fail with "Permission
denied").  But I wonder if we rather want the user to notice that
misconfiguration so that the user can correct it, instead of hiding
the error message from "ls".

> This results in an error in which typing `git push or` + `tab`
> prints out `ls: .git/remotes: No such file or directory`.
> This can be fixed by simply directing stderror of this line
> to /dev/null.
> ---
>  contrib/completion/git-completion.bash |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 2fece98..72251cc 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -412,7 +412,7 @@ __git_refs_remotes ()
>  __git_remotes ()
>  {
>  	local i IFS=$'\n' d="$(__gitdir)"
> -	test -d "$d/remotes" && ls -1 "$d/remotes"
> +	test -d "$d/remotes" && ls -1 "$d/remotes" 2>/dev/null
>  	for i in $(git --git-dir="$d" config --get-regexp 'remote\..*\.url' 2>/dev/null); do
>  		i="${i#remote.}"
>  		echo "${i/.url*/}"

Re: [PATCH] contrib/completion: suppress stderror in bash completion of git remotes

[Matt Korostoff]

Thank you for your detailed reply Junio.  I'll try to address your
concerns individually, but let me also offer a general thought that
this is probably a good use case to handle even if the root cause is
solvable outside of git.  That is to say, I would think we'd still
want git autocompletion working for users running on imperfect
platforms.

> What's "some system"?

My apologies, that was a typo. I meant to write "some systems."  I'm
not sure what the root cause is, but I can tell you I'm running a
rather vanilla development environment (git 1.7.10, bash 3.2, osx
10.8).  I wish I could supply a list of the version combinations that
result in such an event, but I'm not sure how I would do acquire a
list like that.

> Is this a platform's bug (e.g. "test -d" does not work correctly)?

I don't believe so—here's a simple test-of-the-test I threw together
https://gist.github.com/MKorostoff/f203e414847d43b21de4 which does not
throw this error.

> Is this an configuration error of user's Git repository?

I think I have a pretty generic git configuration (here it is, though
note I've had to redact some identifying information
https://gist.github.com/MKorostoff/f8358f72b968249a3925).  Still, I'd
think we would want to handle such a misconfiguration explicitly,
rather than throw a seemingly unrelated error during autocompletion

> Is this something else?

It would be very helpful if you could supply a few more details on the
type of something you're looking for

> I _think_ you would see the problem if $d/remotes is a directory
> whose contents cannot be listed

I can confirm, that is indeed the behavior.  Animated gif example here
http://i.imgur.com/qcPxAub.gif

> But I wonder if we rather want the user to notice that
> misconfiguration so that the user can correct it

While I wholeheartedly agree that such user feedback would be
valuable, I'm just not sure that it makes sense for this feedback to
interrupt the user's typing mid-word.

Sorry if anyone receives this twice, my first attempt to send was
rejected for including HTML.

On Mon, Feb 9, 2015 at 4:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Matt Korostoff <mkorostoff@gmail.com> writes:
>
>> In some system configurations there is a bug with the
>> __git_remotes function.  Specifically, there is a problem
>> with line 415, `test -d "$d/remotes" && ls -1 "$d/remotes"`.
>> While `test -d` is meant to prevent listing the remotes
>> directory if it does not exist, in some system, `ls` will
>> run regardless.
>
> What's "some system"?
>
> Is this a platform's bug (e.g. "test -d" does not work correctly)?
>
> Is this an configuration error of user's Git repository?
>
> Is this something else?
>
> I _think_ you would see the problem if $d/remotes is a directory
> whose contents cannot be listed (e.g. "chmod a= $d/remotes"), and
> that would not be a platform's bug (i.e. "test -d" would happily say
> "Yes there is a directory", and "ls" would fail with "Permission
> denied").  But I wonder if we rather want the user to notice that
> misconfiguration so that the user can correct it, instead of hiding
> the error message from "ls".
>
>> This results in an error in which typing `git push or` + `tab`
>> prints out `ls: .git/remotes: No such file or directory`.
>> This can be fixed by simply directing stderror of this line
>> to /dev/null.
>> ---
>>  contrib/completion/git-completion.bash |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>> index 2fece98..72251cc 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -412,7 +412,7 @@ __git_refs_remotes ()
>>  __git_remotes ()
>>  {
>>       local i IFS=$'\n' d="$(__gitdir)"
>> -     test -d "$d/remotes" && ls -1 "$d/remotes"
>> +     test -d "$d/remotes" && ls -1 "$d/remotes" 2>/dev/null
>>       for i in $(git --git-dir="$d" config --get-regexp 'remote\..*\.url' 2>/dev/null); do
>>               i="${i#remote.}"
>>               echo "${i/.url*/}"

Re: [PATCH] contrib/completion: suppress stderror in bash

[SZEDER Gábor]

Hi,


Quoting Junio C Hamano <gitster@pobox.com>:

> Matt Korostoff <mkorostoff@gmail.com> writes:

>> diff --git a/contrib/completion/git-completion.bash
>> b/contrib/completion/git-completion.bash
>> index 2fece98..72251cc 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -412,7 +412,7 @@ __git_refs_remotes ()
>>  __git_remotes ()
>>  {
>>  	local i IFS=$'\n' d="$(__gitdir)"
>> -	test -d "$d/remotes" && ls -1 "$d/remotes"
>> +	test -d "$d/remotes" && ls -1 "$d/remotes" 2>/dev/null
>>  	for i in $(git --git-dir="$d" config --get-regexp
>> 'remote\..*\.url' 2>/dev/null); do
>>  		i="${i#remote.}"
>>  		echo "${i/.url*/}"

Do I smell some bitrotting here?

This function just lists all the defined remotes, first by listing the  
directories under refs/remotes to get the "legacy" remotes and then  
loops over 'git config's output to get the "modern" ones.  This  
predates the arrival of the 'git remote' command in January 2007, so  
it was really a long time ago.

We should just run 'git remote' instead, shouldn't we?


Cheers,
Gábor

[PATCH] contrib/completion: deprecate __git_remotes in bash completion

[Matt Korostoff]

Bash auto completion supplies a function __git_remotes which
lists the git remotes of a repository by reading the
.git/remotes directory.  As of git 1.7.6 this is handled
natively by the `git remote` command.  This function is
now deprecated.

Signed-off-by: Matt Korostoff <MKorostoff@gmail.com>
---

*****

Great point Gabor! I would hesitate a little about removing the function 
entirely, because users may have external scripts that rely on this function,
but certainly for our internal purposes the __git_remotes shell function can be
replaced with the native.  Here's a short screen cast of the included patch 
working on my local http://i.imgur.com/6ZSMXCO.gif

*****

 contrib/completion/git-completion.bash |   30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2fece98..8b41871 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -409,14 +409,14 @@ __git_refs_remotes ()
 	done
 }
 
+# Deprecated wrapper function around git remote.  Supplied for legacy purposes,
+# to avoid breaking any external scripts which rely on this function.
+# Originally this function was used to programmatically generate a list of git
+# remotes by reading the .git/remotes directory, but as of git 1.7.6 that is
+# natively handled by the git remote command
 __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
+	git remote
 }
 
 __git_list_merge_strategies ()
@@ -558,7 +558,7 @@ __git_complete_remote_or_refspec ()
 		((c++))
 	done
 	if [ -z "$remote" ]; then
-		__gitcomp_nl "$(__git_remotes)"
+		__gitcomp_nl "$(git remote)"
 		return
 	fi
 	if [ $no_complete_refspec = 1 ]; then
@@ -927,7 +927,7 @@ _git_archive ()
 		return
 		;;
 	--remote=*)
-		__gitcomp_nl "$(__git_remotes)" "" "${cur##--remote=}"
+		__gitcomp_nl "$(git remote)" "" "${cur##--remote=}"
 		return
 		;;
 	--*)
@@ -1397,7 +1397,7 @@ _git_ls_files ()
 
 _git_ls_remote ()
 {
-	__gitcomp_nl "$(__git_remotes)"
+	__gitcomp_nl "$(git remote)"
 }
 
 _git_ls_tree ()
@@ -1639,7 +1639,7 @@ _git_push ()
 {
 	case "$prev" in
 	--repo)
-		__gitcomp_nl "$(__git_remotes)"
+		__gitcomp_nl "$(git remote)"
 		return
 		;;
 	--recurse-submodules)
@@ -1649,7 +1649,7 @@ _git_push ()
 	esac
 	case "$cur" in
 	--repo=*)
-		__gitcomp_nl "$(__git_remotes)" "" "${cur##--repo=}"
+		__gitcomp_nl "$(git remote)" "" "${cur##--repo=}"
 		return
 		;;
 	--recurse-submodules=*)
@@ -1797,7 +1797,7 @@ _git_config ()
 {
 	case "$prev" in
 	branch.*.remote|branch.*.pushremote)
-		__gitcomp_nl "$(__git_remotes)"
+		__gitcomp_nl "$(git remote)"
 		return
 		;;
 	branch.*.merge)
@@ -1809,7 +1809,7 @@ _git_config ()
 		return
 		;;
 	remote.pushdefault)
-		__gitcomp_nl "$(__git_remotes)"
+		__gitcomp_nl "$(git remote)"
 		return
 		;;
 	remote.*.fetch)
@@ -1944,7 +1944,7 @@ _git_config ()
 		;;
 	remote.*)
 		local pfx="${cur%.*}." cur_="${cur#*.}"
-		__gitcomp_nl "$(__git_remotes)" "$pfx" "$cur_" "."
+		__gitcomp_nl "$(git remote)" "$pfx" "$cur_" "."
 		__gitcomp_nl_append "pushdefault" "$pfx" "$cur_"
 		return
 		;;
@@ -2250,7 +2250,7 @@ _git_remote ()
 
 	case "$subcommand" in
 	rename|remove|set-url|show|prune)
-		__gitcomp_nl "$(__git_remotes)"
+		__gitcomp_nl "$(git remote)"
 		;;
 	set-head|set-branches)
 		__git_complete_remote_or_refspec
-- 
1.7.10.2 (Apple Git-33)

Re: [PATCH] contrib/completion: suppress stderror in bash

[Junio C Hamano]

SZEDER Gábor <szeder@ira.uka.de> writes:

>>> @@ -412,7 +412,7 @@ __git_refs_remotes ()
>>>  __git_remotes ()
>>>  {
>>>  	local i IFS=$'\n' d="$(__gitdir)"
>>> -	test -d "$d/remotes" && ls -1 "$d/remotes"
>>> +	test -d "$d/remotes" && ls -1 "$d/remotes" 2>/dev/null
>>>  	for i in $(git --git-dir="$d" config --get-regexp
>>> 'remote\..*\.url' 2>/dev/null); do
>>>  		i="${i#remote.}"
>>>  		echo "${i/.url*/}"
>
> Do I smell some bitrotting here?
>
> This function just lists all the defined remotes, first by listing the  
> directories under refs/remotes to get the "legacy" remotes and then  
> loops over 'git config's output to get the "modern" ones.  This  
> predates the arrival of the 'git remote' command in January 2007, so  
> it was really a long time ago.
>
> We should just run 'git remote' instead, shouldn't we?

Perhaps.  Is it sufficient to just make __git_remotes() a thin
wrapper around, i.e.

	__git_remotes ()
	{
		git remotes
	}

or do we need to munge its output further (I didn't look)?

Re: [PATCH] contrib/completion: suppress stderror in bash

[SZEDER Gábor]

Hi,

Quoting Junio C Hamano <gitster@pobox.com>:

> SZEDER Gábor <szeder@ira.uka.de> writes:
>
>>>> @@ -412,7 +412,7 @@ __git_refs_remotes ()
>>>> __git_remotes ()
>>>> {
>>>> 	local i IFS=$'\n' d="$(__gitdir)"
>>>> -	test -d "$d/remotes" && ls -1 "$d/remotes"
>>>> +	test -d "$d/remotes" && ls -1 "$d/remotes" 2>/dev/null
>>>> 	for i in $(git --git-dir="$d" config --get-regexp
>>>> 'remote\..*\.url' 2>/dev/null); do
>>>> 		i="${i#remote.}"
>>>> 		echo "${i/.url*/}"
>>
>> Do I smell some bitrotting here?
>>
>> This function just lists all the defined remotes, first by listing the
>> directories under refs/remotes to get the "legacy" remotes and then
>> loops over 'git config's output to get the "modern" ones.  This
>> predates the arrival of the 'git remote' command in January 2007, so
>> it was really a long time ago.
>>
>> We should just run 'git remote' instead, shouldn't we?
>
> Perhaps.  Is it sufficient to just make __git_remotes() a thin
> wrapper around, i.e.
>
> 	__git_remotes ()
> 	{
> 		git remotes
> 	}
>
> or do we need to munge its output further (I didn't look)?

Well, just like in other cases where we run git from the completion
script, we need a '--git-dir="$(__gitdir)"' as well, because the user can
specify the path to a different repo via $GIT_DIR or on the command
line.
Other than that it seems we are OK.  Docs say "With no arguments,
shows a list of existing remotes." and as far as I can tell, on
MSysGit, it does so without any funny formatting.

But beware, I spent most of last year cycling to Mongolia and my git-fu
became a somewhat rusty... and I'm not quite up to speed yet.

Best,
Gábor

Re: [PATCH] contrib/completion: suppress stderror in bash

[SZEDER Gábor]

Hi,


Quoting SZEDER Gábor <szeder@ira.uka.de>:

> Hi,
>
> Quoting Junio C Hamano <gitster@pobox.com>:
>
>> SZEDER Gábor <szeder@ira.uka.de> writes:
>>
>>>>> @@ -412,7 +412,7 @@ __git_refs_remotes ()
>>>>> __git_remotes ()
>>>>> {
>>>>> 	local i IFS=$'\n' d="$(__gitdir)"
>>>>> -	test -d "$d/remotes" && ls -1 "$d/remotes"
>>>>> +	test -d "$d/remotes" && ls -1 "$d/remotes" 2>/dev/null
>>>>> 	for i in $(git --git-dir="$d" config --get-regexp
>>>>> 'remote\..*\.url' 2>/dev/null); do
>>>>> 		i="${i#remote.}"
>>>>> 		echo "${i/.url*/}"
>>>
>>> Do I smell some bitrotting here?
>>>
>>> This function just lists all the defined remotes, first by listing the
>>> directories under refs/remotes to get the "legacy" remotes and then
>>> loops over 'git config's output to get the "modern" ones.  This
>>> predates the arrival of the 'git remote' command in January 2007, so
>>> it was really a long time ago.
>>>
>>> We should just run 'git remote' instead, shouldn't we?
>>
>> Perhaps.  Is it sufficient to just make __git_remotes() a thin
>> wrapper around, i.e.
>>
>> 	__git_remotes ()
>> 	{
>> 		git remotes
>> 	}
>>
>> or do we need to munge its output further (I didn't look)?
>
> Well, just like in other cases where we run git from the completion
> script, we need a '--git-dir="$(__gitdir)"' as well, because the user can
> specify the path to a different repo via $GIT_DIR or on the command
> line.
> Other than that it seems we are OK.  Docs say "With no arguments,
> shows a list of existing remotes." and as far as I can tell, on
> MSysGit, it does so without any funny formatting.

Oh, look what forgotten treasure did I stumble upon in the vaults:

    
https://github.com/szeder/git/commit/e4e3760c15b485b9ff4768e13050f4b19b5968b8

A two and a half year old commit in my old git repo doing the same...   
completely forgotten :)

Unfortunately, however, it's not quite that simple, because 'git  
remote' doesn't list remotes under '$GIT_DIR/remotes'.  Or at least I  
would have expected the following test to work, but it does not:

diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 17c6330..6a4c139 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -734,6 +734,15 @@ Pull: refs/heads/master:refs/heads/origin
  Pull: refs/heads/next:refs/heads/origin2
  EOF

+test_expect_success 'list remote in $GIT_DIR/remotes' '
+	mkdir .git/remotes &&
+	test_when_finished "rm -rf .git/remotes" &&
+	cat remotes_origin >.git/remotes/remote_from_file &&
+	git remote >actual &&
+	echo remote_from_file >expect &&
+	test_cmp expect actual
+'
+
  test_expect_success 'migrate a remote from named file in $GIT_DIR/remotes' '
  	git clone one five &&
  	origin_url=$(pwd)/one &&

because listing remotes is implemented by for_each_remote(), which  
only reads remotes from the config file.

Now, considering how old 'git remote' is, there were plenty of time  
for someone to miss this functionality and complain about it, but  
since it's still not implemented is probably a good sign that noone  
has actually missed it.  And I don't think it's worth implementing it  
now just to shave off two more lines from the completion script.

Anyway, 'git remote' could still replace that 'git config' query.  I  
have the patches ready and it seems I got send-email working, so  
they'll follow in a minute or two.

Best,
Gábor

[PATCH 1/2] completion: add a test for __git_remotes() helper function

[SZEDER Gábor]

The test checks that both remotes under '$GIT_DIR/remotes' and remotes
in the config file are listed.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 t/t9902-completion.sh | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index f10a752..7a883d1 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -351,6 +351,25 @@ test_expect_success '__gitcomp_nl - doesnt fail because of invalid variable name
 	__gitcomp_nl "$invalid_variable_name"
 '
 
+test_expect_success '__git_remotes - list remotes from $GIT_DIR/remotes and from config file' '
+	cat >expect <<-EOF &&
+	remote_from_file_1
+	remote_from_file_2
+	remote_in_config_1
+	remote_in_config_2
+	EOF
+	test_when_finished "rm -rf .git/remotes" &&
+	mkdir -p .git/remotes &&
+	>.git/remotes/remote_from_file_1 &&
+	>.git/remotes/remote_from_file_2 &&
+	test_when_finished "git remote remove remote_in_config_1" &&
+	git remote add remote_in_config_1 git://remote_1 &&
+	test_when_finished "git remote remove remote_in_config_2" &&
+	git remote add remote_in_config_2 git://remote_2 &&
+	__git_remotes >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'basic' '
 	run_completion "git " &&
 	# built-in
-- 
2.1.2.1369.g8751039

[PATCH 2/2] completion: simplify __git_remotes()

[SZEDER Gábor]

The __git_remotes() helper function lists the remotes from the config
file by processing the output of a 'git config' query.  A simple 'git
remote' produces the exact same output, so run that instead.

Remotes under '$GIT_DIR/remotes' are still listed by running 'ls -1',
because 'git remote' unfortunately ignores them.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c21190d..f5ae5e3 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -411,12 +411,9 @@ __git_refs_remotes ()
 
 __git_remotes ()
 {
-	local i IFS=$'\n' d="$(__gitdir)"
+	local 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
+	git --git-dir="$d" remote
 }
 
 __git_list_merge_strategies ()
-- 
2.1.2.1369.g8751039