Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref

From: Jeff King <peff@peff.net>
Date: Feb 12, 2016, 23:40 UTC
Message-ID: <20160212234041.GA15688@sigill.intra.peff.net>
In-Reply-To: <20160213002122.Horde.mxoPmZIuCikpV2PO97l11AI@webmail.informatik.kit.edu>

On Sat, Feb 13, 2016 at 12:21:22AM +0100, SZEDER Gábor wrote:

Show 8 quoted lines

> I think in this case we should opt for performance instead of correctness,
> and use Peff's 'refname:strip=2'.  Ambiguous refs will only hurt you, if,
> well, your repo actually has ambiguous refs AND you happen to want to do
> something with one of those refs.  I suspect that's rather uncommon, and
> even then you could simply rename one of those refs.  OTOH, as shown in
> the ticket, you don't need that many refs to make refs completion
> unacceptably slow on Windows, and it will bite every time you attempt to
> complete a ref.

I'm not even sure that this is a correctness tradeoff at all. For example, in the function __git_heads(), we are asking for-each-ref to tell us about everything under refs/heads/. If you have a refs/heads/foo and refs/tags/foo, we don't care; we are trying to print the unqualified branch names. And in fact having refname:short print "heads/foo" in this case may be actively wrong. For instance, in _git_branch(), you cannot use the resulting completion of "heads/foo", as that command wants unqualified names in "refs/heads/", and you do not have "refs/heads/heads/foo".

So I think switching to :strip is an improvement in both correctness _and_ performance.

> Now, if 'git for-each-ref' could understand '**' globbing, not just
> fnmatch...

I think it does already, since 4917e1e (Makefile: promote wildmatch to be the default fnmatch implementation, 2013-05-30).

-Peff