Andrei wrote:
> This patch adds wildcard matching to the regular "prefix" matching for
> svndumpfilter. It is my first patch and feedback would be appreciated.
>
> There is one place where I am unsure about: the wildcard matching uses
> "NXOR", so I am not sure if it was designed to only work with path
> prefixes (it seems to go well if I choose "*.txt" as a pattern for
> example).
Andrei, thanks for the generally good patch. I have a few comments
which you can find inline below.
> Index: subversion/svndumpfilter/main.c
> ===================================================================
> --- subversion/svndumpfilter/main.c (revision 21719)
> +++ subversion/svndumpfilter/main.c (working copy)
> @@ -97,7 +97,118 @@
> svn_stringbuf_appendbytes(*strbuf, "\n", 1);
> }
>
> +/* Match a string against a wildcard pattern.
> + * @param str - [in] the string to check
> + * @param pat - [in] the pattern. '*' and '?' wildcards are accepted,
> + * with '\\' as an escape character
> + * @return TRUE if pattern match is detected
> + */
> +static svn_boolean_t
> +wildcard_match(const char *str, const char *pat)
> +{
...
Could not this whole function be written using apr_fnmatch(), the same
matching interface we use elsewhere in Subversion? You could do a
quicky pass to handle the backslash escape character (a UI quirk I'm not
totally sold on), and then hand the result off to apr_fnmatch(). Sure
would simplify the code, and keep this behavior consistent with that of
other areas of Subversion.
> @@ -864,6 +991,23 @@
> baton->renumber_history = apr_hash_make(pool);
> baton->last_live_revision = SVN_INVALID_REVNUM;
>
> + if (opt_state->match_type == NULL) { /* just use the default */
> + baton->match_function = ary_prefix_match;
> + } else if (strcmp(opt_state->match_type, MATCH_TYPE_PREFIX) == 0) {
> + baton->match_function = ary_prefix_match;
> + } else if (strcmp(opt_state->match_type, MATCH_TYPE_WILDCARD) == 0) {
> + svn_error_t *pComplexityCheck;
> +
> + pComplexityCheck = check_wildcard_complexity(baton->prefixes);
> + if (pComplexityCheck)
> + return pComplexityCheck;
We don't do camel-case. But if you use apr_fnmatch(), you needn't check
complexity anyway.
--
C. Michael Pilato <cmpilato@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Received on Mon Oct 2 15:00:37 2006