Redo DiffFormatter API to be easier to use

Passing around the OutputStream and the Repository is crazy.  Instead
put the stream in the constructor, since this formatter exists only to
output to the stream, and put the repository as a member variable that
can be optionally set.

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

org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Diff.java

@@ -104,7 +104,8 @@ class Diff extends TextBuiltin {
 		fmt.setContext(lines);
 	}
 
-	private DiffFormatter fmt = new DiffFormatter() {
+	private DiffFormatter fmt = new DiffFormatter( //
+			new BufferedOutputStream(System.out)) {
 		@Override
 		protected RawText newRawText(byte[] raw) {
 			if (ignoreWsAll)
@@ -129,9 +130,9 @@ class Diff extends TextBuiltin {
 			out.flush();
 
 		} else {
-			BufferedOutputStream o = new BufferedOutputStream(System.out);
-			fmt.format(o, db, files);
-			o.flush();
+			fmt.setRepository(db);
+			fmt.format(files);
+			fmt.flush();
 		}
 	}
 

org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Log.java

@@ -97,7 +97,8 @@ class Log extends RevWalkTextBuiltin {
 		diffFmt.setContext(lines);
 	}
 
-	private DiffFormatter diffFmt = new DiffFormatter();
+	private DiffFormatter diffFmt = new DiffFormatter( //
+			new BufferedOutputStream(System.out));
 
 	Log() {
 		fmt = new SimpleDateFormat("EEE MMM dd HH:mm:ss yyyy ZZZZZ", Locale.US);
@@ -170,10 +171,9 @@ class Log extends RevWalkTextBuiltin {
 			Diff.nameStatus(out, files);
 
 		} else {
-			out.flush();
-			BufferedOutputStream o = new BufferedOutputStream(System.out);
-			diffFmt.format(o, db, files);
-			o.flush();
+			diffFmt.setRepository(db);
+			diffFmt.format(files);
+			diffFmt.flush();
 		}
 	}
 }

org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterReflowTest.java

@@ -68,7 +68,7 @@ public class DiffFormatterReflowTest extends TestCase {
 	protected void setUp() throws Exception {
 		super.setUp();
 		out = new ByteArrayOutputStream();
-		fmt = new DiffFormatter();
+		fmt = new DiffFormatter(out);
 	}
 
 	public void testNegativeContextFails() throws IOException {
@@ -143,7 +143,7 @@ public class DiffFormatterReflowTest extends TestCase {
 	}
 
 	private void assertFormatted(final String name) throws IOException {
-		fmt.format(out, file, a, b);
+		fmt.format(file, a, b);
 		final String exp = RawParseUtils.decode(readFile(name));
 		assertEquals(exp, RawParseUtils.decode(out.toByteArray()));
 	}

org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties

@@ -301,6 +301,7 @@ renamesAlreadyFound=Renames have already been found.
 renamesFindingByContent=Finding renames by content similarity
 renamesFindingExact=Finding exact renames
 repositoryAlreadyExists=Repository already exists: {0}
+repositoryIsRequired=Repository is required.
 repositoryNotFound=repository not found: {0}
 requiredHashFunctionNotAvailable=Required hash function {0} not available.
 resolvingDeltas=Resolving deltas

org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java

@@ -360,6 +360,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String renamesFindingByContent;
 	/***/ public String renamesFindingExact;
 	/***/ public String repositoryAlreadyExists;
+	/***/ public String repositoryIsRequired;
 	/***/ public String repositoryNotFound;
 	/***/ public String requiredHashFunctionNotAvailable;
 	/***/ public String resolvingDeltas;

org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java

@@ -66,13 +66,40 @@ import org.eclipse.jgit.util.QuotedString;
 public class DiffFormatter {
 	private static final byte[] noNewLine = encodeASCII("\\ No newline at end of file\n");
 
+	private final OutputStream out;
+
+	private Repository db;
+
 	private int context;
 
-	/** Create a new formatter with a default level of context. */
-	public DiffFormatter() {
+	/**
+	 * Create a new formatter with a default level of context.
+	 *
+	 * @param out
+	 *            the stream the formatter will write line data to. This stream
+	 *            should have buffering arranged by the caller, as many small
+	 *            writes are performed to it.
+	 */
+	public DiffFormatter(OutputStream out) {
+		this.out = out;
 		setContext(3);
 	}
 
+	/** @return the stream we are outputting data to. */
+	protected OutputStream getOutputStream() {
+		return out;
+	}
+
+	/**
+	 * Set the repository the formatter can load object contents from.
+	 *
+	 * @param repository
+	 *            source repository holding referenced objects.
+	 */
+	public void setRepository(Repository repository) {
+		db = repository;
+	}
+
 	/**
 	 * Change the number of lines of context to display.
 	 *
@@ -83,45 +110,61 @@ public class DiffFormatter {
 	 */
 	public void setContext(final int lineCount) {
 		if (lineCount < 0)
-			throw new IllegalArgumentException(JGitText.get().contextMustBeNonNegative);
+			throw new IllegalArgumentException(
+					JGitText.get().contextMustBeNonNegative);
 		context = lineCount;
 	}
 
+	/**
+	 * Flush the underlying output stream of this formatter.
+	 *
+	 * @throws IOException
+	 *             the stream's own flush method threw an exception.
+	 */
+	public void flush() throws IOException {
+		out.flush();
+	}
+
 	/**
 	 * Format a patch script from a list of difference entries.
 	 *
-	 * @param out
-	 *            stream to write the patch script out to.
-	 * @param src
-	 *            repository the file contents can be read from.
 	 * @param entries
 	 *            entries describing the affected files.
 	 * @throws IOException
 	 *             a file's content cannot be read, or the output stream cannot
 	 *             be written to.
 	 */
-	public void format(final OutputStream out, Repository src,
-			List<? extends DiffEntry> entries) throws IOException {
-		for(DiffEntry ent : entries) {
-			if (ent instanceof FileHeader) {
-				format(
-						out,
-						(FileHeader) ent, //
-						newRawText(open(src, ent.getOldMode(), ent.getOldId())),
-						newRawText(open(src, ent.getNewMode(), ent.getNewId())));
-			} else {
-				format(out, src, ent);
-			}
+	public void format(List<? extends DiffEntry> entries) throws IOException {
+		for (DiffEntry ent : entries)
+			format(ent);
+	}
+
+	/**
+	 * Format a patch script for one file entry.
+	 *
+	 * @param entry
+	 *            the entry to be formatted.
+	 * @throws IOException
+	 *             a file's content cannot be read, or the output stream cannot
+	 *             be written to.
+	 */
+	public void format(DiffEntry entry) throws IOException {
+		if (entry instanceof FileHeader) {
+			format(
+					(FileHeader) entry, //
+					newRawText(open(entry.getOldMode(), entry.getOldId())),
+					newRawText(open(entry.getNewMode(), entry.getNewId())));
+		} else {
+			formatAndDiff(entry);
 		}
 	}
 
-	private void format(OutputStream out, Repository src, DiffEntry ent)
-			throws IOException {
+	private void formatAndDiff(DiffEntry ent) throws IOException {
 		String oldName = quotePath("a/" + ent.getOldName());
 		String newName = quotePath("b/" + ent.getNewName());
 		out.write(encode("diff --git " + oldName + " " + newName + "\n"));
 
-		switch(ent.getChangeType()) {
+		switch (ent.getChangeType()) {
 		case ADD:
 			out.write(encodeASCII("new file mode "));
 			ent.getNewMode().copyTo(out);
@@ -135,7 +178,7 @@ public class DiffFormatter {
 			break;
 
 		case RENAME:
-			out.write(encode("similarity index " + ent.getScore() + "%"));
+			out.write(encodeASCII("similarity index " + ent.getScore() + "%"));
 			out.write('\n');
 
 			out.write(encode("rename from " + quotePath(ent.getOldName())));
@@ -146,7 +189,7 @@ public class DiffFormatter {
 			break;
 
 		case COPY:
-			out.write(encode("similarity index " + ent.getScore() + "%"));
+			out.write(encodeASCII("similarity index " + ent.getScore() + "%"));
 			out.write('\n');
 
 			out.write(encode("copy from " + quotePath(ent.getOldName())));
@@ -178,9 +221,9 @@ public class DiffFormatter {
 		}
 
 		out.write(encodeASCII("index " //
-				+ format(src, ent.getOldId()) //
+				+ format(ent.getOldId()) //
 				+ ".." //
-				+ format(src, ent.getNewId())));
+				+ format(ent.getNewId())));
 		if (ent.getOldMode().equals(ent.getNewMode())) {
 			out.write(' ');
 			ent.getNewMode().copyTo(out);
@@ -189,8 +232,8 @@ public class DiffFormatter {
 		out.write(encode("--- " + oldName + '\n'));
 		out.write(encode("+++ " + newName + '\n'));
 
-		byte[] aRaw = open(src, ent.getOldMode(), ent.getOldId());
-		byte[] bRaw = open(src, ent.getNewMode(), ent.getNewId());
+		byte[] aRaw = open(ent.getOldMode(), ent.getOldId());
+		byte[] bRaw = open(ent.getNewMode(), ent.getNewId());
 
 		if (RawText.isBinary(aRaw) || RawText.isBinary(bRaw)) {
 			out.write(encodeASCII("Binary files differ\n"));
@@ -198,7 +241,7 @@ public class DiffFormatter {
 		} else {
 			RawText a = newRawText(aRaw);
 			RawText b = newRawText(bRaw);
-			formatEdits(out, a, b, new MyersDiff(a, b).getEdits());
+			formatEdits(a, b, new MyersDiff(a, b).getEdits());
 		}
 	}
 
@@ -213,8 +256,8 @@ public class DiffFormatter {
 		return new RawText(content);
 	}
 
-	private String format(Repository db, AbbreviatedObjectId oldId) {
-		if (oldId.isComplete())
+	private String format(AbbreviatedObjectId oldId) {
+		if (oldId.isComplete() && db != null)
 			oldId = oldId.toObjectId().abbreviate(db, 8);
 		return oldId.name();
 	}
@@ -224,7 +267,7 @@ public class DiffFormatter {
 		return ('"' + name + '"').equals(q) ? name : q;
 	}
 
-	private byte[] open(Repository src, FileMode mode, AbbreviatedObjectId id)
+	private byte[] open(FileMode mode, AbbreviatedObjectId id)
 			throws IOException {
 		if (mode == FileMode.MISSING)
 			return new byte[] {};
@@ -232,8 +275,10 @@ public class DiffFormatter {
 		if (mode.getObjectType() != Constants.OBJ_BLOB)
 			return new byte[] {};
 
+		if (db == null)
+			throw new IllegalStateException(JGitText.get().repositoryIsRequired);
 		if (id.isComplete()) {
-			ObjectLoader ldr = src.openObject(id.toObjectId());
+			ObjectLoader ldr = db.openObject(id.toObjectId());
 			return ldr.getCachedBytes();
 		}
 
@@ -247,8 +292,6 @@ public class DiffFormatter {
 	 * to increase or reduce the number of lines of context within the script.
 	 * All header lines are reused as-is from the supplied FileHeader.
 	 *
-	 * @param out
-	 *            stream to write the patch script out to.
 	 * @param head
 	 *            existing file header containing the header lines to copy.
 	 * @param a
@@ -260,8 +303,8 @@ public class DiffFormatter {
 	 * @throws IOException
 	 *             writing to the supplied stream failed.
 	 */
-	public void format(final OutputStream out, final FileHeader head,
-			final RawText a, final RawText b) throws IOException {
+	public void format(final FileHeader head, final RawText a, final RawText b)
+			throws IOException {
 		// Reuse the existing FileHeader as-is by blindly copying its
 		// header lines, but avoiding its hunks. Instead we recreate
 		// the hunks from the text instances we have been supplied.
@@ -272,19 +315,22 @@ public class DiffFormatter {
 			end = head.getHunks().get(0).getStartOffset();
 		out.write(head.getBuffer(), start, end - start);
 
-		formatEdits(out, a, b, head.toEditList());
+		formatEdits(a, b, head.toEditList());
 	}
 
 	/**
 	 * Formats a list of edits in unified diff format
-	 * @param out where the unified diff is written to
-	 * @param a the text A which was compared
-	 * @param b the text B which was compared
-	 * @param edits some differences which have been calculated between A and B
+	 *
+	 * @param a
+	 *            the text A which was compared
+	 * @param b
+	 *            the text B which was compared
+	 * @param edits
+	 *            some differences which have been calculated between A and B
 	 * @throws IOException
 	 */
-	public void formatEdits(final OutputStream out, final RawText a,
-			final RawText b, final EditList edits) throws IOException {
+	public void formatEdits(final RawText a, final RawText b,
+			final EditList edits) throws IOException {
 		for (int curIdx = 0; curIdx < edits.size();) {
 			Edit curEdit = edits.get(curIdx);
 			final int endIdx = findCombinedEnd(edits, curIdx);
@@ -295,18 +341,24 @@ public class DiffFormatter {
 			final int aEnd = Math.min(a.size(), endEdit.getEndA() + context);
 			final int bEnd = Math.min(b.size(), endEdit.getEndB() + context);
 
-			writeHunkHeader(out, aCur, aEnd, bCur, bEnd);
+			writeHunkHeader(aCur, aEnd, bCur, bEnd);
 
 			while (aCur < aEnd || bCur < bEnd) {
 				if (aCur < curEdit.getBeginA() || endIdx + 1 < curIdx) {
-					writeContextLine(out, a, aCur, isEndOfLineMissing(a, aCur));
+					writeContextLine(a, aCur);
+					if (isEndOfLineMissing(a, aCur))
+						out.write(noNewLine);
 					aCur++;
 					bCur++;
 				} else if (aCur < curEdit.getEndA()) {
-					writeRemovedLine(out, a, aCur, isEndOfLineMissing(a, aCur));
+					writeRemovedLine(a, aCur);
+					if (isEndOfLineMissing(a, aCur))
+						out.write(noNewLine);
 					aCur++;
 				} else if (bCur < curEdit.getEndB()) {
-					writeAddedLine(out, b, bCur, isEndOfLineMissing(b, bCur));
+					writeAddedLine(b, bCur);
+					if (isEndOfLineMissing(b, bCur))
+						out.write(noNewLine);
 					bCur++;
 				}
 
@@ -317,21 +369,17 @@ public class DiffFormatter {
 	}
 
 	/**
-	 * Output a line of diff context
+	 * Output a line of context (unmodified line).
 	 *
-	 * @param out
-	 *            OutputStream
 	 * @param text
 	 *            RawText for accessing raw data
 	 * @param line
 	 *            the line number within text
-	 * @param endOfLineMissing
-	 *            true if we should add the GNU end of line missing warning
 	 * @throws IOException
 	 */
-	protected void writeContextLine(final OutputStream out, final RawText text,
-			final int line, boolean endOfLineMissing) throws IOException {
-		writeLine(out, ' ', text, line, endOfLineMissing);
+	protected void writeContextLine(final RawText text, final int line)
+			throws IOException {
+		writeLine(' ', text, line);
 	}
 
 	private boolean isEndOfLineMissing(final RawText text, final int line) {
@@ -339,46 +387,36 @@ public class DiffFormatter {
 	}
 
 	/**
-	 * Output an added line
+	 * Output an added line.
 	 *
-	 * @param out
-	 *            OutputStream
 	 * @param text
 	 *            RawText for accessing raw data
 	 * @param line
 	 *            the line number within text
-	 * @param endOfLineMissing
-	 *            true if we should add the gnu end of line missing warning
 	 * @throws IOException
 	 */
-	protected void writeAddedLine(final OutputStream out, final RawText text, final int line, boolean endOfLineMissing)
+	protected void writeAddedLine(final RawText text, final int line)
 			throws IOException {
-		writeLine(out, '+', text, line, endOfLineMissing);
+		writeLine('+', text, line);
 	}
 
 	/**
 	 * Output a removed line
 	 *
-	 * @param out
-	 *            OutputStream
 	 * @param text
 	 *            RawText for accessing raw data
 	 * @param line
 	 *            the line number within text
-	 * @param endOfLineMissing
-	 *            true if we should add the gnu end of line missing warning
 	 * @throws IOException
 	 */
-	protected void writeRemovedLine(final OutputStream out, final RawText text,
-			final int line, boolean endOfLineMissing) throws IOException {
-		writeLine(out, '-', text, line, endOfLineMissing);
+	protected void writeRemovedLine(final RawText text, final int line)
+			throws IOException {
+		writeLine('-', text, line);
 	}
 
 	/**
 	 * Output a hunk header
 	 *
-	 * @param out
-	 *            OutputStream
 	 * @param aStartLine
 	 *            within first source
 	 * @param aEndLine
@@ -389,20 +427,20 @@ public class DiffFormatter {
 	 *            within second source
 	 * @throws IOException
 	 */
-	protected void writeHunkHeader(final OutputStream out, int aStartLine, int aEndLine,
+	protected void writeHunkHeader(int aStartLine, int aEndLine,
 			int bStartLine, int bEndLine) throws IOException {
 		out.write('@');
 		out.write('@');
-		writeRange(out, '-', aStartLine + 1, aEndLine - aStartLine);
-		writeRange(out, '+', bStartLine + 1, bEndLine - bStartLine);
+		writeRange('-', aStartLine + 1, aEndLine - aStartLine);
+		writeRange('+', bStartLine + 1, bEndLine - bStartLine);
 		out.write(' ');
 		out.write('@');
 		out.write('@');
 		out.write('\n');
 	}
 
-	private static void writeRange(final OutputStream out, final char prefix,
-			final int begin, final int cnt) throws IOException {
+	private void writeRange(final char prefix, final int begin, final int cnt)
+			throws IOException {
 		out.write(' ');
 		out.write(prefix);
 		switch (cnt) {
@@ -431,18 +469,23 @@ public class DiffFormatter {
 		}
 	}
 
-	private static void writeLine(final OutputStream out, final char prefix,
-			final RawText text, final int cur, boolean noNewLineIndicator) throws IOException {
+	/**
+	 * Write a standard patch script line.
+	 *
+	 * @param prefix
+	 *            prefix before the line, typically '-', '+', ' '.
+	 * @param text
+	 *            the text object to obtain the line from.
+	 * @param cur
+	 *            line number to output.
+	 * @throws IOException
+	 *             the stream threw an exception while writing to it.
+	 */
+	protected void writeLine(final char prefix, final RawText text,
+			final int cur) throws IOException {
 		out.write(prefix);
 		text.writeLine(out, cur);
 		out.write('\n');
-		if (noNewLineIndicator)
-			writeNoNewLine(out);
-	}
-
-	private static void writeNoNewLine(final OutputStream out)
-			throws IOException {
-		out.write(noNewLine);
 	}
 
 	private int findCombinedEnd(final List<Edit> edits, final int i) {