diff mbox series

[05/15] ref-filter.c: parameterize match functions over patterns

Message ID c54000f5f5122c4ca3ac9b16828a8fd77050768c.1683581621.git.me@ttaylorr.com (mailing list archive)
State New, archived
Headers show
Series refs: implement skip lists for packed backend | expand

Commit Message

Taylor Blau May 8, 2023, 9:59 p.m. UTC 
From: Jeff King <peff@peff.net>

`match_pattern()` and `match_name_as_path()` both take a `struct
ref_filter *`, and then store a stack variable `patterns` pointing at
`filter->patterns`.

The subsequent patch will add a new array of patterns to match over (the
excluded patterns, via a new `git for-each-ref --exclude` option),
treating the return value of these functions differently depending on
which patterns are being used to match.

Tweak `match_pattern()` and `match_name_as_path()` to take an array of
patterns to prepare for passing either in.

Co-authored-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 ref-filter.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

Comments

Junio C Hamano May 8, 2023, 10:36 p.m. UTC | #1 
Taylor Blau <me@ttaylorr.com> writes:

> -static int match_pattern(const struct ref_filter *filter, const char *refname)
> +static int match_pattern(const struct ref_filter *filter,
> +			 const char **patterns,
> +			 const char *refname)
>  {
> -	const char **patterns = filter->name_patterns;
>  	unsigned flags = 0;
>  
>  	if (filter->ignore_case)
> @@ -2132,9 +2133,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)
>  {
> -	const char **pattern = filter->name_patterns;
>  	int namelen = strlen(refname);
>  	unsigned flags = WM_PATHNAME;
>  

These hint that we'd eventually lose .name_patterns member from the
structure so that we can pass pattern array that is not necessarily
tied to any instance of a filter?

> @@ -2163,8 +2165,8 @@ static int filter_pattern_match(struct ref_filter *filter, const char *refname)
>  	if (!*filter->name_patterns)
>  		return 1; /* No pattern always matches */
>  	if (filter->match_as_path)
> -		return match_name_as_path(filter, refname);
> -	return match_pattern(filter, refname);
> +		return match_name_as_path(filter, filter->name_patterns, refname);
> +	return match_pattern(filter, filter->name_patterns, refname);

And we are not there yet, so we hoist the use of .name_patterns
member one level up to the only caller?

Without seeing how it evolves, we can tell this does not make
anything break, but we cannot tell how this helps anything (yet).

Let's read on.
Taylor Blau May 9, 2023, 8:13 p.m. UTC | #2 
On Mon, May 08, 2023 at 03:36:45PM -0700, Junio C Hamano wrote:
> > @@ -2132,9 +2133,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)
> >  {
> > -	const char **pattern = filter->name_patterns;
> >  	int namelen = strlen(refname);
> >  	unsigned flags = WM_PATHNAME;
> >
>
> These hint that we'd eventually lose .name_patterns member from the
> structure so that we can pass pattern array that is not necessarily
> tied to any instance of a filter?

Right. We'll pass in the excluded patterns in the next commit.

> And we are not there yet, so we hoist the use of .name_patterns
> member one level up to the only caller?
>
> Without seeing how it evolves, we can tell this does not make
> anything break, but we cannot tell how this helps anything (yet).

Hmm. I tried to allude to this in the patch message with the paragraph:

    The subsequent patch will add a new array of patterns to match over (the
    excluded patterns, via a new `git for-each-ref --exclude` option),
    treating the return value of these functions differently depending on
    which patterns are being used to match.

    Tweak `match_pattern()` and `match_name_as_path()` to take an array of
    patterns to prepare for passing either in.

...but I'm happy to add detail or clarify it if you think that these
paragraphs could use it.

Thanks,
Taylor
diff mbox series

Patch

diff --git a/ref-filter.c b/ref-filter.c
index 9ea92b9637..6c5eed144f 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2102,9 +2102,10 @@  static int get_ref_atom_value(struct ref_array_item *ref, int atom,
  * matches a pattern "refs/heads/mas") or a wildcard (e.g. the same ref
  * matches "refs/heads/mas*", too).
  */
-static int match_pattern(const struct ref_filter *filter, const char *refname)
+static int match_pattern(const struct ref_filter *filter,
+			 const char **patterns,
+			 const char *refname)
 {
-	const char **patterns = filter->name_patterns;
 	unsigned flags = 0;
 
 	if (filter->ignore_case)
@@ -2132,9 +2133,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)
 {
-	const char **pattern = filter->name_patterns;
 	int namelen = strlen(refname);
 	unsigned flags = WM_PATHNAME;
 
@@ -2163,8 +2165,8 @@  static int filter_pattern_match(struct ref_filter *filter, const char *refname)
 	if (!*filter->name_patterns)
 		return 1; /* No pattern always matches */
 	if (filter->match_as_path)
-		return match_name_as_path(filter, refname);
-	return match_pattern(filter, refname);
+		return match_name_as_path(filter, filter->name_patterns, refname);
+	return match_pattern(filter, filter->name_patterns, refname);
 }
 
 /*