Subject: Re: [JGIT PATCH v2 11/24] Added the class FNMatchPattern.

From: "Shawn O. Pearce" <[email-masked]>
Date: Mon, 12 May 2008 20:38:32 -0400

Florian Koeberle <[email-masked]> wrote:
> +/**
> + * This class represents a pattern which should work like the fnmatch method.
> + * <code>new FNMatchPattern(exp).matches(input)</code> should do the same like
> + * <code>fnmatch(exp, input, 0) == 0</code>
> + * 
> + * As this isn't a one to one code port, but written based on the documentation
> + * of fnmatch it can be that the behavior of this class differ in some corner
> + * cases from the behavior of the fnmatch function.
> + */
> +public class FNMatchPattern {
> +
> +	private final Pattern regexPattern;

For what it is worth, I got a performance improvement by declaring
that such classes like FNMatchPattern are _not_ threadsafe and
storing a Matcher rather than a Pattern.  Then on each test you
can just reset the Matcher and evaluate it again.

This was worthwhile enough that I went back into RevFilter and
added a clone() method so you can safely clone a RevFilter graph
to create a new set of instances for another thread.

Consider using a Matcher here.  Ignore rule matching with a lot
of patterns will bottleneck things like working directory status
operations.

> +	private static String toRegexString(String fnmatchPattern) {
> +		final StringBuilder regexStringBuilder = new StringBuilder();
> +		char perviosCharacter = 0;
> +		for (int i = 0; i < fnmatchPattern.length(); i++) {
> +			final char c = fnmatchPattern.charAt(i);
> +			switch (c) {
> +			case '^':
> +				if (perviosCharacter == '[') {
> +					regexStringBuilder.append('!');
> +				} else {
> +					regexStringBuilder.append("\\x5E");
> +				}
> +				break;
> +			case '.':
> +				regexStringBuilder.append("\\x2E");
> +				break;
> +			case '*':
> +				regexStringBuilder.append(".*");
> +				break;
> +			default:
> +				regexStringBuilder.append(c);
> +			}
> +			perviosCharacter = c;
> +		}
> +		return regexStringBuilder.toString();

Huh.  So the fnmatchPattern of "foo?" will match the name "fo"
in this implementation, but it does not in my C library's fnmatch
function:

	$ cat fnmatch.c 
	#include <fnmatch.h>
	#include <stdio.h>

	int main(int argc, char *argv[]) {
		const char *pattern = argv[1];
		const char *name = argv[2];
		printf("%s on %s = %i\n", pattern, name, fnmatch(pattern, name, 0));
		return 0;
	}
	$ ./fnmatch 'foo?' 'fo'
	foo? on fo = 1

There are plenty more cases like that as too many of the regex
operators are leaking through. All of the regex operators need to
be treated as literals in the regex pattern.

-- 
Shawn.