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

From: SZEDER Gábor <szeder@ira.uka.de>
Date: Feb 10, 2015, 19:16 UTC
Message-ID: <20150210201648.Horde.QMVdqmJIJnvLkf0U46NdYA1@webmail.informatik.kit.edu>
In-Reply-To: <xmqqegpxwrdc.fsf@gitster.dls.corp.google.com>

Hi,

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

Show 32 quoted lines

> 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