Move rename detection, path following into DiffFormatter

Applications just want a quick way to configure our diff
implementation, and then just want to use it without a lot of fuss.

Move all of the rename detection logic and path following logic
out of our pgm package and into DiffFormatter itself, making it
much easier for a GUI to take advantage of the features without
duplicating a lot of code.

Change-Id: I4b54e987bb6dc804fb270cbc495fe4cae26c7b0e
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

3 files changed, 61 insertions, 108 deletions

diff --git a/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/CLIText.properties b/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/CLIText.properties
index dc738d3856..e7dce1bf55 100644
--- a/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/CLIText.properties
+++ b/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/CLIText.properties
@@ -168,6 +168,7 @@ usage_listCreateOrDeleteBranches=List, create, or delete branches
 usage_logAllPretty=format:%H %ct %P' output=log --all '--pretty=format:%H %ct %P' output
 usage_moveRenameABranch=move/rename a branch
 usage_nameStatus=show only name and status of files
+usage_noRenames=disable rename detection
 usage_outputFile=Output file
 usage_path=path
 usage_performFsckStyleChecksOnReceive=perform fsck style checks on receive
diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Diff.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Diff.java
index 2be5722040..b6650a4ea1 100644
--- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Diff.java
+++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Diff.java
@@ -46,9 +46,7 @@
 package org.eclipse.jgit.pgm;
 
 import java.io.BufferedOutputStream;
-import java.io.IOException;
 import java.io.PrintWriter;
-import java.util.ArrayList;
 import java.util.List;
 
 import org.eclipse.jgit.diff.DiffEntry;
@@ -62,8 +60,6 @@ import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.TextProgressMonitor;
 import org.eclipse.jgit.pgm.opt.PathTreeFilterHandler;
 import org.eclipse.jgit.treewalk.AbstractTreeIterator;
-import org.eclipse.jgit.treewalk.TreeWalk;
-import org.eclipse.jgit.treewalk.filter.AndTreeFilter;
 import org.eclipse.jgit.treewalk.filter.TreeFilter;
 import org.kohsuke.args4j.Argument;
 import org.kohsuke.args4j.Option;
@@ -74,12 +70,10 @@ class Diff extends TextBuiltin {
 			new BufferedOutputStream(System.out));
 
 	@Argument(index = 0, metaVar = "metaVar_treeish", required = true)
-	void tree_0(final AbstractTreeIterator c) {
-		trees.add(c);
-	}
+	private AbstractTreeIterator oldTree;
 
 	@Argument(index = 1, metaVar = "metaVar_treeish", required = true)
-	private final List<AbstractTreeIterator> trees = new ArrayList<AbstractTreeIterator>();
+	private AbstractTreeIterator newTree;
 
 	@Option(name = "--", metaVar = "metaVar_paths", multiValued = true, handler = PathTreeFilterHandler.class)
 	private TreeFilter pathFilter = TreeFilter.ALL;
@@ -89,7 +83,12 @@ class Diff extends TextBuiltin {
 	boolean showPatch;
 
 	@Option(name = "-M", usage = "usage_detectRenames")
-	private boolean detectRenames;
+	private Boolean detectRenames;
+
+	@Option(name = "--no-renames", usage = "usage_noRenames")
+	void noRenames(@SuppressWarnings("unused") boolean on) {
+		detectRenames = Boolean.FALSE;
+	}
 
 	@Option(name = "-l", usage = "usage_renameLimit")
 	private Integer renameLimit;
@@ -136,16 +135,27 @@ class Diff extends TextBuiltin {
 
 	@Override
 	protected void run() throws Exception {
-		List<DiffEntry> files = scan();
+		diffFmt.setRepository(db);
+		try {
+			diffFmt.setProgressMonitor(new TextProgressMonitor());
+			diffFmt.setPathFilter(pathFilter);
+			if (detectRenames != null)
+				diffFmt.setDetectRenames(detectRenames.booleanValue());
+			if (renameLimit != null && diffFmt.isDetectRenames()) {
+				RenameDetector rd = diffFmt.getRenameDetector();
+				rd.setRenameLimit(renameLimit.intValue());
+			}
 
-		if (showNameAndStatusOnly) {
-			nameStatus(out, files);
-			out.flush();
+			if (showNameAndStatusOnly) {
+				nameStatus(out, diffFmt.scan(oldTree, newTree));
+				out.flush();
 
-		} else {
-			diffFmt.setRepository(db);
-			diffFmt.format(files);
-			diffFmt.flush();
+			} else {
+				diffFmt.format(oldTree, newTree);
+				diffFmt.flush();
+			}
+		} finally {
+			diffFmt.release();
 		}
 	}
 
@@ -174,23 +184,4 @@ class Diff extends TextBuiltin {
 			}
 		}
 	}
-
-	private List<DiffEntry> scan() throws IOException {
-		final TreeWalk walk = new TreeWalk(db);
-		walk.reset();
-		walk.setRecursive(true);
-		for (final AbstractTreeIterator i : trees)
-			walk.addTree(i);
-		walk.setFilter(AndTreeFilter.create(TreeFilter.ANY_DIFF, pathFilter));
-
-		List<DiffEntry> files = DiffEntry.scan(walk);
-		if (detectRenames) {
-			RenameDetector rd = new RenameDetector(db);
-			if (renameLimit != null)
-				rd.setRenameLimit(renameLimit.intValue());
-			rd.addAll(files);
-			files = rd.compute(new TextProgressMonitor());
-		}
-		return files;
-	}
 }
diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Log.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Log.java
index d0ae22a216..2b29f73385 100644
--- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Log.java
+++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Log.java
@@ -51,32 +51,25 @@ import java.text.DateFormat;
 import java.text.MessageFormat;
 import java.text.SimpleDateFormat;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.Iterator;
-import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
 import java.util.TimeZone;
 
-import org.eclipse.jgit.diff.DiffEntry;
 import org.eclipse.jgit.diff.DiffFormatter;
 import org.eclipse.jgit.diff.RawTextIgnoreAllWhitespace;
 import org.eclipse.jgit.diff.RawTextIgnoreLeadingWhitespace;
 import org.eclipse.jgit.diff.RawTextIgnoreTrailingWhitespace;
 import org.eclipse.jgit.diff.RawTextIgnoreWhitespaceChange;
 import org.eclipse.jgit.diff.RenameDetector;
-import org.eclipse.jgit.diff.DiffEntry.ChangeType;
 import org.eclipse.jgit.lib.AnyObjectId;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.revwalk.FollowFilter;
 import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevTree;
 import org.eclipse.jgit.revwalk.RevWalk;
-import org.eclipse.jgit.treewalk.TreeWalk;
-import org.eclipse.jgit.treewalk.filter.AndTreeFilter;
-import org.eclipse.jgit.treewalk.filter.TreeFilter;
 import org.kohsuke.args4j.Option;
 
 @Command(common = true, usage = "usage_viewCommitHistory")
@@ -98,7 +91,12 @@ class Log extends RevWalkTextBuiltin {
 	boolean showPatch;
 
 	@Option(name = "-M", usage = "usage_detectRenames")
-	private boolean detectRenames;
+	private Boolean detectRenames;
+
+	@Option(name = "--no-renames", usage = "usage_noRenames")
+	void noRenames(@SuppressWarnings("unused") boolean on) {
+		detectRenames = Boolean.FALSE;
+	}
 
 	@Option(name = "-l", usage = "usage_renameLimit")
 	private Integer renameLimit;
@@ -156,6 +154,24 @@ class Log extends RevWalkTextBuiltin {
 	}
 
 	@Override
+	protected void run() throws Exception {
+		diffFmt.setRepository(db);
+		try {
+			diffFmt.setPathFilter(pathFilter);
+			if (detectRenames != null)
+				diffFmt.setDetectRenames(detectRenames.booleanValue());
+			if (renameLimit != null && diffFmt.isDetectRenames()) {
+				RenameDetector rd = diffFmt.getRenameDetector();
+				rd.setRenameLimit(renameLimit.intValue());
+			}
+
+			super.run();
+		} finally {
+			diffFmt.release();
+		}
+	}
+
+	@Override
 	protected void show(final RevCommit c) throws Exception {
 		out.print(CLIText.get().commitLabel);
 		out.print(" ");
@@ -196,71 +212,16 @@ class Log extends RevWalkTextBuiltin {
 	}
 
 	private void showDiff(RevCommit c) throws IOException {
-		final TreeWalk tw = new TreeWalk(db);
-		tw.setRecursive(true);
-		tw.reset();
-		tw.addTree(c.getParent(0).getTree());
-		tw.addTree(c.getTree());
-		tw.setFilter(AndTreeFilter.create(pathFilter, TreeFilter.ANY_DIFF));
-
-		List<DiffEntry> files = DiffEntry.scan(tw);
-		if (pathFilter instanceof FollowFilter && isAdd(files)) {
-			// The file we are following was added here, find where it
-			// came from so we can properly show the rename or copy,
-			// then continue digging backwards.
-			//
-			tw.reset();
-			tw.addTree(c.getParent(0).getTree());
-			tw.addTree(c.getTree());
-			tw.setFilter(TreeFilter.ANY_DIFF);
-			files = updateFollowFilter(detectRenames(DiffEntry.scan(tw)));
-
-		} else if (detectRenames)
-			files = detectRenames(files);
-
-		if (showNameAndStatusOnly) {
-			Diff.nameStatus(out, files);
-
-		} else {
-			diffFmt.setRepository(db);
-			diffFmt.format(files);
+		final RevTree a = c.getParent(0).getTree();
+		final RevTree b = c.getTree();
+
+		if (showNameAndStatusOnly)
+			Diff.nameStatus(out, diffFmt.scan(a, b));
+		else {
+			diffFmt.format(a, b);
 			diffFmt.flush();
 		}
 		out.println();
-	}
-
-	private List<DiffEntry> detectRenames(List<DiffEntry> files)
-			throws IOException {
-		RenameDetector rd = new RenameDetector(db);
-		if (renameLimit != null)
-			rd.setRenameLimit(renameLimit.intValue());
-		rd.addAll(files);
-		return rd.compute();
-	}
-
-	private boolean isAdd(List<DiffEntry> files) {
-		String oldPath = ((FollowFilter) pathFilter).getPath();
-		for (DiffEntry ent : files) {
-			if (ent.getChangeType() == ChangeType.ADD
-					&& ent.getNewPath().equals(oldPath))
-				return true;
-		}
-		return false;
-	}
-
-	private List<DiffEntry> updateFollowFilter(List<DiffEntry> files) {
-		String oldPath = ((FollowFilter) pathFilter).getPath();
-		for (DiffEntry ent : files) {
-			if (isRename(ent) && ent.getNewPath().equals(oldPath)) {
-				pathFilter = FollowFilter.create(ent.getOldPath());
-				return Collections.singletonList(ent);
-			}
-		}
-		return Collections.emptyList();
-	}
-
-	private static boolean isRename(DiffEntry ent) {
-		return ent.getChangeType() == ChangeType.RENAME
-				|| ent.getChangeType() == ChangeType.COPY;
+		out.flush();
 	}
 }