Subject: Re: [PATCH v4 05/16] ref-filter.c: parameterize match functions over patterns

From: Taylor Blau <[email-masked]>
Date: Mon, 3 Jul 2023 13:22:28 -0400

On Mon, Jul 03, 2023 at 01:18:06PM -0400, Taylor Blau wrote:
> On Mon, Jul 03, 2023 at 01:27:24AM -0400, Jeff King wrote:
> > On Tue, Jun 20, 2023 at 10:21:21AM -0400, Taylor Blau wrote:
> >
> > > Once we start passing either in, `match_pattern()` will have little to
> > > do with a particular `struct ref_filter *` instance. To clarify this,
> > > drop it from the argument list, and replace it with the only bit of the
> > > `ref_filter` that we care about (`filter->ignore_case`).
> >
> > Makes sense, but...
> >
> > > @@ -2134,9 +2134,10 @@ static int match_pattern(const struct ref_filter *filter, const char *refname)
> > >   * matches a pattern "refs/heads/" but not "refs/heads/m") or a
> > >   * wildcard (e.g. the same ref matches "refs/heads/m*", too).
> > >   */
> > > -static int match_name_as_path(const struct ref_filter *filter, const char *refname)
> > > +static int match_name_as_path(const struct ref_filter *filter,
> > > +			      const char **pattern,
> > > +			      const char *refname)
> >
> > ...wouldn't we then want to do the same for match_name_as_path()?
>
> Yes, definitely :-). I'm not sure how I missed this, since the patch
> message even says that `match_name_as_path()` gets the same treatment as
> the other function.
>
> But in any case, I obviously agree (and the diff below makes sense to
> me). Applied.

Ah, this is missing one more spot (that we wouldn't complain about
during a non-DEVELOPER build). This needs to go on top, but I otherwise
agree:

--- 8< ---
diff --git a/ref-filter.c b/ref-filter.c
index de85904b8d..845173a904 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2176,7 +2176,8 @@ static int filter_exclude_match(struct ref_filter *filter, const char *refname)
 	if (!filter->exclude.nr)
 		return 0;
 	if (filter->match_as_path)
-		return match_name_as_path(filter, filter->exclude.v, refname);
+		return match_name_as_path(filter->exclude.v, refname,
+					  filter->ignore_case);
 	return match_pattern(filter->exclude.v, refname, filter->ignore_case);
 }
--- >8 ---

Thanks,
Taylor