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

From: Taylor Blau <me@ttaylorr.com>
Date: Jun 20, 2023, 11:52 UTC
Message-ID: <ZJGTFcYf7G05KHad@nand.local>
In-Reply-To: <xmqqsfav2alu.fsf@gitster.g>

On Tue, Jun 13, 2023 at 04:42:05PM -0700, Junio C Hamano wrote:

Show 15 quoted lines

> > 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