From 5be90be996551302d59a07ea49af5e5b9fed1cb3 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Sat, 3 Jul 2010 16:58:37 -0700 Subject: [PATCH] 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 --- .../src/org/eclipse/jgit/pgm/Diff.java | 9 +- .../src/org/eclipse/jgit/pgm/Log.java | 10 +- .../jgit/diff/DiffFormatterReflowTest.java | 4 +- .../org/eclipse/jgit/JGitText.properties | 1 + .../src/org/eclipse/jgit/JGitText.java | 1 + .../org/eclipse/jgit/diff/DiffFormatter.java | 215 +++++++++++------- 6 files changed, 143 insertions(+), 97 deletions(-) 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 a3fca638b..32499618d 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 @@ -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(); } } 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 511da4498..aa4e8ae3c 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 @@ -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(); } } } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterReflowTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterReflowTest.java index d9e50d20a..d9c95d765 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterReflowTest.java +++ b/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())); } diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties index a8b9a33a4..afe7a8c05 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties +++ b/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 diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java index ed5192cf5..1f21b4a02 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java +++ b/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; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java b/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java index b08d1cb50..00cfe8b08 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java +++ b/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 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 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 edits, final int i) {