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

From: Shawn O. Pearce <spearce@spearce.org>
Date: May 13, 2008, 00:38 UTC
Message-ID: <20080513003832.GD29038@spearce.org>
In-Reply-To: <1210623222-24908-12-git-send-email-florianskarten@web.de>

Florian Koeberle <florianskarten@web.de> wrote:

Show 12 quoted lines

> +/**
> + * 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.

Show 25 quoted lines

> +	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.