Subject: Re: [PATCH 1/1] ls-refs.c: minimize number of refs visited

From: Taylor Blau <[email-masked]>
Date: Tue, 19 Jan 2021 17:23:36 -0500

On Tue, Jan 19, 2021 at 05:15:30PM -0500, Jeff King wrote:
> On Tue, Jan 19, 2021 at 04:59:18PM -0500, Jeff King wrote:
>
> > > > What does "delimited by /" mean?
> > >
> > > Ah, I just meant that it looks for the longest common prefix where it
> > > will only split at '/' characters. But, that's not right at all:
> > > find_longest_prefixes_1() will happily split anywhere there is a
> > > difference.
> >
> > Right. We thought in early revisions of the ref-filter work that we
> > might have to split on path components, but it turns out that the
> > underlying ref code is happy to take arbitrary prefixes. And it's that
> > code which defines our strategy; Even if the ls-refs code wanted to
> > allow only full path components, it should be using the limiting from
> > for_each_ref_in() only as an optimization, and applying its own
> > filtering to the output.
>
> Having now looked carefully at the ls-refs code, it's a pure
> prefix-match, too. So I think we _could_ rely on for_each_fullref_in()
> returning us the correct full results, and not checking it further in
> send_ref(). But I kind of like keeping it there as an extra check (and
> one which could in theory grow more logic later).

Hmm. What if the caller asks for:

  ref-prefix refs/tags/a
  ref-prefix refs/tags/b

?

The LCP between those two is refs/tags, so send_ref() will presumably
get lots of reuslts that it doesn't care about (assuming there are tags
besides a and b).

Thanks,
Taylor