Subject: Re: [PATCH v3 07/16] refs: plumb `exclude_patterns` argument throughout

From: Taylor Blau <[email-masked]>
Date: Tue, 20 Jun 2023 07:52:53 -0400

On Tue, Jun 13, 2023 at 04:42:05PM -0700, Junio C Hamano wrote:
> > diff --git a/ref-filter.c b/ref-filter.c
> > index d44418efb7..717c3c4bcf 100644
> > --- a/ref-filter.c
> > +++ b/ref-filter.c
> > @@ -2209,12 +2209,13 @@ static int for_each_fullref_in_pattern(struct ref_filter *filter,
> >
> >  	if (!filter->name_patterns[0]) {
> >  		/* no patterns; we have to look at everything */
> > -		return for_each_fullref_in("", cb, cb_data);
> > +		return refs_for_each_fullref_in(get_main_ref_store(the_repository),
> > +						 "", NULL, cb, cb_data);
> >  	}
>
> Is this merely "while at it", or was there a reason why refs_*
> variant must be used here?

Good question. This changes later in the series (via
"refs/packed-backend.c: implement jump lists to avoid excluded
pattern(s)") to pass the excluded patterns from the ref_filter.

There's no need to change it in this patch, since the functionality at
this point is equivalent in the pre- and post-image. I think this
staging is a consequence of having written much of this series before
committing anything, and then trying to segment it out into meaningful
patches after the fact.

> It is curious that we do not teach the exclude_patterns to some
> functions like for_each_fullref_in() while adding exclude_patterns to
> others, making the API surface uneven.

We could plumb it through in more places, but my preference would be to
modify the API if/as new callers need to pass a list of excluded
patterns. The API has an enormous amount of surface area (and many
functions which take a ton of arguments), so I'd rather keep it small as
long as possible, even at the expense of some unevenness in its
interface.

Thanks,
Taylor