Quoting Jeff King <peff@xxxxxxxx>:
On Sat, Feb 13, 2016 at 12:21:22AM +0100, SZEDER Gábor wrote:
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.
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.
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).
Things are looking up!
A single 'master' branch and 10 remotes with 10k remote branches each,
i.e. a total of 100001 refs, all packed. To uniquely complete
'master ' after 'git checkout m<TAB>' on Linux in current git.git, i.e.
with 'refname:short':
$ cur=m ; time __gitcomp_nl "$(__git_refs '' 1)"
real 0m7.641s
user 0m5.888s
sys 0m1.832s
Using 'refname:strip=2' for both 'git for-each-ref' in __git_refs():
$ cur=m ; time __gitcomp_nl "$(__git_refs '' 1)"
real 0m2.848s
user 0m2.308s
sys 0m0.596s
Quick'n'dirty PoC using 'refname:strip', '**' globbing and a few more
tricks to let 'git for-each-ref' do the filtering instead of the
shell loop behind __gitcomp_nl():
$ cur=m ; time IFS=$'\n' COMPREPLY=( $(__git_refs_PoC '' 1) )
real 0m0.247s
user 0m0.208s
sys 0m0.032s
Not bad for a Friday night, huh? :)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html