Thank you for the feedback. Here is a new version using apr_fnmatch. Based on my tests, it seems to handle escapes ('\') ok and has '[' matching on top of the previous one. I did not see a case which would require me to handle escapes aditionally from apr_fnmatch .. did you
have any specific functionality in mind that requires this?
Andrei
On Mon, 2006-10-02 at 09:00 -0400, C. Michael Pilato wrote:
> 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.
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Oct 3 03:57:10 2006