Re: [PATCH v4 06/10] ref-filter: add option to match literal pattern
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Jul 26, 2015, 05:15 UTC
- Message-ID
- <CAPig+cTAYAxg4aWpaPrHe6Hfz3wCEiypoXZxS_Lir4yNOFzSNA@mail.gmail.com>
- In-Reply-To
- <1437764685-8633-7-git-send-email-Karthik.188@gmail.com>
On Fri, Jul 24, 2015 at 3:04 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
Show 34 quoted lines
> Since 'ref-filter' only has an option to match path names add an
> option for plain fnmatch pattern-matching.
>
> This is to support the pattern matching options which are used in `git
> tag -l` and `git branch -l` where we can match patterns like `git tag
> -l foo*` which would match all tags which has a "foo*" pattern.
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
> diff --git a/ref-filter.c b/ref-filter.c
> index 8f2148f..0a64b84 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -951,6 +951,31 @@ static int commit_contains(struct ref_filter *filter, struct commit *commit)
>
> /*
> * Return 1 if the refname matches one of the patterns, otherwise 0.
> + * A pattern can be a literal prefix (e.g. a refname "refs/heads/master"
> + * matches a pattern "refs/heads/mas") or a wildcard (e.g. the same ref
> + * matches "refs/heads/mas*", too).
> + */
> +static int match_pattern(const char **patterns, const char *refname)
> +{
> + for (; *patterns; patterns++) {
> + /*
> + * When no '--format' option is given we need to skip the prefix
> + * for matching refs of tags and branches.
> + */
> + if (!starts_with(*patterns, "refs/tags/"))
> + skip_prefix(refname, "refs/tags/", &refname);
> + if (!starts_with(*patterns, "refs/heads/"))
> + skip_prefix(refname, "refs/heads/", &refname);
> + if (!starts_with(*patterns, "refs/remotes/"))
> + skip_prefix(refname, "refs/remotes/", &refname);Note the inefficiency here: You're performing an "expensive" starts_with() on each pattern for each refname even though the patterns will never change. You could instead compute starts_with() for each pattern just once, in a preprocessing step, and remember each result as a boolean, and then use the computed booleans here in place of starts_with(). For a few refnames, this may not make a difference, but for a project with a huge number, it could. Whether such an optimization is worth the complexity (at this time or ever) is something that can be answered by taking measurements.
Also, the repetition in the code is not all that pretty. You could instead place "refs/tags/", "refs/heads/", and "refs/remotes/" in a table and then loop over them rather than repeating the code for each one, though whether that would be an improvement is likely a judgment call (so not something I'd insist upon).
Show 5 quoted lines
> + if (!wildmatch(*patterns, refname, 0, NULL)) > + return 1; > + } > + return 0; > +}