public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: "Shawn O. Pearce" <spearce@spearce•org>
To: Florian Koeberle <florianskarten@web•de>
Cc: git@vger•kernel.org
Subject: Re: [JGIT PATCH v2 14/24] Added the class IgnoreRuleListFactory.
Date: Mon, 12 May 2008 21:08:44 -0400	[thread overview]
Message-ID: <20080513010844.GE29038@spearce.org> (raw)
In-Reply-To: <1210623222-24908-15-git-send-email-florianskarten@web.de>

Florian Koeberle <florianskarten@web•de> 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.

  reply	other threads:[~2008-05-13  1:09 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-12 20:13 [JGIT PATCH v2 0/24] Implementation of a file tree iteration using ignore rules Florian Koeberle
2008-05-12 20:13 ` [JGIT PATCH v2 01/24] Start of an implementation of a git like command line tool Florian Koeberle
2008-05-12 20:13 ` [JGIT PATCH v2 02/24] Formatted Repository class Florian Koeberle
2008-05-12 20:13 ` [JGIT PATCH v2 03/24] Formatted Constats class Florian Koeberle
2008-05-12 20:13 ` [JGIT PATCH v2 04/24] Added path related constats to " Florian Koeberle
2008-05-12 23:54   ` Shawn O. Pearce
2008-05-23 15:46     ` Florian Köberle
2008-05-12 20:13 ` [JGIT PATCH v2 05/24] Added WorkTree class which can be constructed over Repository Florian Koeberle
2008-05-13  0:04   ` Shawn O. Pearce
2008-05-13 21:13     ` Robin Rosenberg
2008-05-14  0:35       ` Shawn O. Pearce
2008-05-12 20:13 ` [JGIT PATCH v2 06/24] Added a "init" command to the git like command line tool Florian Koeberle
2008-05-12 20:13 ` [JGIT PATCH v2 07/24] Added findWorkTree method to Repository class Florian Koeberle
2008-05-13  0:24   ` Shawn O. Pearce
2008-05-23 18:12     ` Florian Köberle
2008-05-23 18:31       ` Miklos Vajna
2008-05-23 20:39         ` Shawn O. Pearce
2008-05-23 21:28       ` Robin Rosenberg
2008-05-12 20:13 ` [JGIT PATCH v2 08/24] Added the interface FilePattern Florian Koeberle
2008-05-12 20:13 ` [JGIT PATCH v2 09/24] Added the class Rule Florian Koeberle
2008-05-12 20:13 ` [JGIT PATCH v2 10/24] Added the iterface Rules Florian Koeberle
2008-05-12 20:13 ` [JGIT PATCH v2 11/24] Added the class FNMatchPattern Florian Koeberle
2008-05-13  0:38   ` Shawn O. Pearce
2008-05-12 20:13 ` [JGIT PATCH v2 12/24] Added the class GlobalFilePattern Florian Koeberle
2008-05-12 20:13 ` [JGIT PATCH v2 13/24] Added the class ComplexFilePattern Florian Koeberle
2008-05-12 20:13 ` [JGIT PATCH v2 14/24] Added the class IgnoreRuleListFactory Florian Koeberle
2008-05-13  1:08   ` Shawn O. Pearce [this message]
2008-05-13 10:19     ` Florian Köberle
2008-05-14  1:06       ` Shawn O. Pearce
2008-05-12 20:13 ` [JGIT PATCH v2 15/24] Added a Rules interface implementation and a factory for it Florian Koeberle
2008-05-12 20:13 ` [JGIT PATCH v2 16/24] Added test class OverallIgnoreRulestest Florian Koeberle
2008-05-12 20:13 ` [JGIT PATCH v2 17/24] Added the class TreeFilePattern Florian Koeberle
2008-05-13  1:22   ` Shawn O. Pearce
2008-05-12 20:13 ` [JGIT PATCH v2 18/24] Added InvalidPatternException and PathNotInProjectDirectoryException Florian Koeberle
2008-05-12 20:13 ` [JGIT PATCH v2 19/24] Added the class AddRuleListFactory Florian Koeberle
2008-05-13  1:29   ` Shawn O. Pearce
2008-05-13 11:24     ` Florian Köberle
2008-05-13 20:55       ` Robin Rosenberg
2008-05-14  1:49         ` Shawn O. Pearce
2008-05-12 20:13 ` [JGIT PATCH v2 20/24] Added class AddRulesFactory Florian Koeberle
2008-05-12 20:13 ` [JGIT PATCH v2 21/24] Added the class LightFileTreeIterator and a test for it Florian Koeberle
2008-05-14 14:27   ` Paolo Bonzini
2008-05-12 20:13 ` [JGIT PATCH v2 22/24] Added class LightFileTreeIterable Florian Koeberle
2008-05-12 20:13 ` [JGIT PATCH v2 23/24] Added the test class AddCommandIterationTest Florian Koeberle
2008-05-12 20:13 ` [JGIT PATCH v2 24/24] Added a "add" command to the git like command line tool Florian Koeberle
2008-05-12 20:43 ` [JGIT PATCH v2 0/24] Implementation of a file tree iteration using ignore rules Miklos Vajna

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080513010844.GE29038@spearce.org \
    --to=spearce@spearce$(echo .)org \
    --cc=florianskarten@web$(echo .)de \
    --cc=git@vger$(echo .)kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox