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

From: Jeff King <peff@peff.net>
Date: Feb 13, 2016, 16:57 UTC
Message-ID: <20160213165722.GA30144@sigill.intra.peff.net>
In-Reply-To: <20160213020712.Horde.SM-rQbc5Jx1UwdYxdvNFNJx@webmail.informatik.kit.edu>

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

Show 12 quoted lines

> >So I think switching to :strip is an improvement in both correctness
> >_and_ performance.
> 
> Right.  I was more worried about __git_refs(), because it asks for
> everything under refs/heads/, refs/tags/ and refs/remotes/, and its
> output is used in a lot more places and fed to a lot more commands than
> the output of __git_heads() (or __git_tags(), for that matter).  But I
> thought that a branch-tag ambiguity would cause git to error out
> complaining, just like in the case of ref-path ambiguity.  Successfully
> avoiding ambiguous refs for many years, I wasn't aware that 'git
> rev-parse' doesn't barf, but only warns and resolves the ambiguity in
> favor of the tag.

Yeah, switching to :strip would arguably be a regression when completing all refs. Right now, you'd get "heads/foo" and "tags/foo" as part of your completion (but _not_ just "foo"), and either works as a non-ambiguous ref.

With :strip, you'd just get "foo" twice, and if you use the result of the completion, it will always point to the tag.

So it is arguably worse. I still think it is worth trading off for performance, but it is worth acknowledging in the commit message there that it is a tradeoff.

Show 5 quoted lines

> >I think it does already, since 4917e1e (Makefile: promote wildmatch to
> >be the default fnmatch implementation, 2013-05-30).
> 
> Things are looking up!
> [...vast improvement in times...]

Very cool. I look forward to seeing the final patch. :)

I have noticed in my pathological 10-million-ref bare repositories (don't ask) that the __git_ps1() prompt is quite slow, too. And I wondered if it could be related.

But I don't think it is. It's just literally that painful to look at the packed-refs at all, and "git rev-parse HEAD" has to look at them to resolve.

-Peff