Florian Koeberle <florianskarten@web.de> wrote:
quoted text
> @@ -1242,4 +1247,89 @@ public class Repository {
>  		}
>  	}
>  
> +	/**
> +	 * Find the git repository for the current working directory.
> +	 * 
> +	 * @return a {@link Repository}.
> +	 * @throws IOException
> +	 *             if the system property user.dir isn't set or if it is
> +	 *             invalid.
> +	 */
> +	public static WorkTree findWorkTree() throws IOException {
> +		final String workingDirectoryPath = System.getProperty("user.dir");
> +		if (workingDirectoryPath == null) {
> +			throw new IOException("unable to get working directory");
> +		}
> +		final File workingDirectoryFile = new File(workingDirectoryPath);

Isn't `final File workingDirectoryFile = new File(".")` easier to write?

And if we are going to throw a checked exception because we cannot
find a repository in ".", maybe that should be a new checked
exception, like NoGitRepositoryFoundException?

quoted text
> +	/**
> +	 * Checks if a path is a valid git repository. Works similar like the method
> +	 * is_git_directory from the original setup.c file.

References to C Git aren't common in jgit source code.  We aren't
a reference implementation, but that hasn't stopped people from
finding out code a sane description of "how things work".  I'd rather
not describe our code in terms of C Git's code; especially what
happens if C Git refactors one day, this reference might no longer
make sense.  Anyone coming back here to improve jgit or use this
method will be confused about what we are doing.

quoted text
> +	 * 
> +	 * @param directory
> +	 *            the path which should be checked.
> +	 * @return true if the path is a valid git repository.
> +	 */
> +	private static boolean isRepository(File directory) {
> +		if (!directory.isDirectory()) {
> +			return false;
> +		}

We usually omit { and } on simple conditions like this.  Its a coding
pattern we stole from C Git, which stole it from the Linux kernel.

quoted text
> +	private static WorkTree findWorkTree(File directory) throws IOException {
> +		File currentDirectory = directory.getAbsoluteFile();
> +		while (true) {
> +			final File commonGitDirectory = new File(directory,
> +					REPOSITORY_DIRECTORY_NAME);
> +			if (isRepository(commonGitDirectory)) {
> +				return new WorkTree(currentDirectory, new Repository(
> +						commonGitDirectory));
> +			}
> +
> +			if (isRepository(currentDirectory)) {
> +				return new WorkTree(null, new Repository(currentDirectory));
> +			}
> +			currentDirectory = currentDirectory.getParentFile();
> +			if (currentDirectory == null) {
> +				throw new IOException("Can't find git repository");
> +			}
> +		}

Would this perhaps be a shorter, easier to follow variant of the
same algorithm?

	directory = directory.getAbsoluteFile();
	File workdir = directory;
	while (workdir != null) {
		final File gitdir = new File(workdir, REPOSITORY_DIRECTORY_NAME);
		if (isRepository(gitdir))
			return new WorkTree(workdir, new Repository(gitdir));
		if (isRepository(workdir))
			return new WorkTree(null, new Repository(workdir));
		workdir = workdir.getParentFile();
	}
	throw new NoGitRepositoryFoundException("No repository for " + directory);

I often do write infinite loops with "for (;;) {", but usually its
because the loop condition is based on an iterator-style method
returning null at the end and I want to store the result in a local
variable scoped to only the body of the loop:

	for (;;) {
		final RevCommit next = walk.next();
		if (next == null)
			break;

		// use next somehow
	}

Otherwise I try to avoid infinite loops in my final version
of something.  Though on a recent job interview the interviewer
noticed I tend to start writing code with an infinite loop, then
go back and correct the loop condition before saying "done".  ;-)

-- 
Shawn.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html