Florian Koeberle <florianskarten@xxxxxx> wrote:
> +/**
> + * This class can be used to create lists of {@link Rule} objects from lines of
> + * .gitignore like files.
> + *
> + */
> +class IgnoreRuleListFactory {
> +
> + List<Rule> createIgnoreRuleList(Iterable<String> lineIterable) {
> + LinkedList<Rule> rules = new LinkedList<Rule>();
> + for (String line : lineIterable) {
> + final String trimmedLine = line.trim();
> + if (trimmedLine.startsWith("#")) {
> + continue;
> + }
> + if (trimmedLine.length() == 0) {
> + continue;
> + }
> + rules.add(0, createRule(trimmedLine));
> + }
> + return rules;
> + }
> +
> + List<Rule> createIgnoreRuleList(List<File> files)
> + throws FileNotFoundException {
> + final List<String> lines = new ArrayList<String>();
> + for (File file : files) {
> + Scanner scanner = new Scanner(file);
> + try {
> + while (scanner.hasNextLine()) {
> + lines.add(scanner.nextLine());
> + }
> + } finally {
> + scanner.close();
> + }
> + }
> + return createIgnoreRuleList(lines);
> + }
Why go through all this work to buffer the lines we don't care about
(starting with # or are blank) when we could just discard them in the
inside of createIgnoreRuleList and then create the rule right away?
> + private Rule createRule(String trimmedLine) {
> + final boolean exclude;
> + String patternString;
> + if (trimmedLine.startsWith("!")) {
> + exclude = false;
> + patternString = trimmedLine.substring(1);
> + } else {
> + exclude = true;
> + patternString = trimmedLine;
> + }
I suspect this code would be easier to follow if you just accepted
changing the method parameter, such as:
private Rule createRule(String pattern) {
boolean exclude = true;
if (pattern.startsWith("!)) {
pattern = pattern.substring(1);
exclude = false;
}
> + final boolean matchDirectoriesOnly;
> + if (patternString.endsWith("/")) {
> + matchDirectoriesOnly = true;
> + patternString = patternString.substring(0,
> + patternString.length() - 1);
> + } else {
> + matchDirectoriesOnly = false;
> + }
> +
> + final FilePattern pattern;
> + if (patternString.contains("/")) {
> + if (patternString.startsWith("/")) {
> + patternString = patternString.substring(1);
> + }
"foo/bar" will always end up in this code-path and will not match
in all levels of the tree if I follow your code correctly.
An ignore rule in the top level of "foo/bar" should ignore any entry
named "bar" within a directory "foo" at any level of the tree, even
if it is 35 directories down from the root. Isn't ComplexFilePattern
about the absolute (starts with "/") cases only?
> + final StringTokenizer stringTokenizer = new StringTokenizer(
> + patternString, "/");
> + final List<String> patternList = new ArrayList<String>();
> + while (stringTokenizer.hasMoreTokens()) {
> + final String token = stringTokenizer.nextToken();
> + patternList.add(token);
> + }
StringTokenizer is more-or-less replaced by String.split("/"), with
the split method being the more preferred method of doing this.
--
Shawn.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
