From df8adefe86934cbd25982527780b5eed9a1feae1 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Tue, 31 Aug 2010 19:28:15 -0700 Subject: [PATCH] Correct diff header formatting When adding or deleting a file, we shouldn't ever prefix /dev/null with the a/ or b/ prefixes. Doing so is a mistake and confuses a patch parser which handles /dev/null magically, while a/dev/null is a file called null in the dev directory of the project. Also when adding or deleting the "diff --git" line has the "real" path on both sides, so we should see the following when adding the file called foo: diff --git a/foo b/foo --- /dev/null +++ b/foo The --- and +++ lines do not appear in a pure rename or copy delta, C Git diff seems to omit these, so we now omit them as well. We also omit the index line when the ObjectIds are exactly equal. Change-Id: Ic46892dea935ee8bdee29088aab96307d7ec6d3d Signed-off-by: Shawn O. Pearce --- .../eclipse/jgit/diff/DiffFormatterTest.java | 70 +++++ .../org/eclipse/jgit/diff/DiffFormatter.java | 249 ++++++++++++------ 2 files changed, 235 insertions(+), 84 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterTest.java index 92d4fa114..d7a10e4b1 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterTest.java @@ -77,6 +77,76 @@ public void setUp() throws Exception { df.setAbbreviationLength(8); } + public void testCreateFileHeader_Add() throws Exception { + ObjectId adId = blob("a\nd\n"); + DiffEntry ent = DiffEntry.add("FOO", adId); + FileHeader fh = df.createFileHeader(ent); + + String diffHeader = "diff --git a/FOO b/FOO\n" // + + "new file mode " + REGULAR_FILE + "\n" + + "index " + + ObjectId.zeroId().abbreviate(8).name() + + ".." + + adId.abbreviate(8).name() + "\n" // + + "--- /dev/null\n"// + + "+++ b/FOO\n"; + assertEquals(diffHeader, RawParseUtils.decode(fh.getBuffer())); + + assertEquals(0, fh.getStartOffset()); + assertEquals(fh.getBuffer().length, fh.getEndOffset()); + assertEquals(FileHeader.PatchType.UNIFIED, fh.getPatchType()); + + assertEquals(1, fh.getHunks().size()); + + HunkHeader hh = fh.getHunks().get(0); + assertEquals(1, hh.toEditList().size()); + + EditList el = hh.toEditList(); + assertEquals(1, el.size()); + + Edit e = el.get(0); + assertEquals(0, e.getBeginA()); + assertEquals(0, e.getEndA()); + assertEquals(0, e.getBeginB()); + assertEquals(2, e.getEndB()); + assertEquals(Edit.Type.INSERT, e.getType()); + } + + public void testCreateFileHeader_Delete() throws Exception { + ObjectId adId = blob("a\nd\n"); + DiffEntry ent = DiffEntry.delete("FOO", adId); + FileHeader fh = df.createFileHeader(ent); + + String diffHeader = "diff --git a/FOO b/FOO\n" // + + "deleted file mode " + REGULAR_FILE + "\n" + + "index " + + adId.abbreviate(8).name() + + ".." + + ObjectId.zeroId().abbreviate(8).name() + "\n" // + + "--- a/FOO\n"// + + "+++ /dev/null\n"; + assertEquals(diffHeader, RawParseUtils.decode(fh.getBuffer())); + + assertEquals(0, fh.getStartOffset()); + assertEquals(fh.getBuffer().length, fh.getEndOffset()); + assertEquals(FileHeader.PatchType.UNIFIED, fh.getPatchType()); + + assertEquals(1, fh.getHunks().size()); + + HunkHeader hh = fh.getHunks().get(0); + assertEquals(1, hh.toEditList().size()); + + EditList el = hh.toEditList(); + assertEquals(1, el.size()); + + Edit e = el.get(0); + assertEquals(0, e.getBeginA()); + assertEquals(2, e.getEndA()); + assertEquals(0, e.getBeginB()); + assertEquals(0, e.getEndB()); + assertEquals(Edit.Type.DELETE, e.getType()); + } + public void testCreateFileHeader_Modify() throws Exception { ObjectId adId = blob("a\nd\n"); ObjectId abcdId = blob("a\nb\nc\nd\n"); 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 d8429a4cd..52c957390 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java @@ -44,6 +44,10 @@ package org.eclipse.jgit.diff; +import static org.eclipse.jgit.diff.DiffEntry.ChangeType.ADD; +import static org.eclipse.jgit.diff.DiffEntry.ChangeType.DELETE; +import static org.eclipse.jgit.diff.DiffEntry.ChangeType.MODIFY; +import static org.eclipse.jgit.diff.DiffEntry.ChangeType.RENAME; import static org.eclipse.jgit.lib.Constants.encode; import static org.eclipse.jgit.lib.Constants.encodeASCII; import static org.eclipse.jgit.lib.FileMode.GITLINK; @@ -55,6 +59,7 @@ import java.util.List; import org.eclipse.jgit.JGitText; +import org.eclipse.jgit.diff.DiffEntry.ChangeType; import org.eclipse.jgit.errors.AmbiguousObjectException; import org.eclipse.jgit.errors.CorruptObjectException; import org.eclipse.jgit.errors.MissingObjectException; @@ -90,6 +95,10 @@ public class DiffFormatter { private int bigFileThreshold = 50 * 1024 * 1024; + private String oldPrefix = "a/"; + + private String newPrefix = "b/"; + /** * Create a new formatter with a default level of context. * @@ -178,6 +187,32 @@ public void setBigFileThreshold(int bigFileThreshold) { this.bigFileThreshold = bigFileThreshold; } + /** + * Set the prefix applied in front of old file paths. + * + * @param prefix + * the prefix in front of old paths. Typically this is the + * standard string {@code "a/"}, but may be any prefix desired by + * the caller. Must not be null. Use the empty string to have no + * prefix at all. + */ + public void setOldPrefix(String prefix) { + oldPrefix = prefix; + } + + /** + * Set the prefix applied in front of new file paths. + * + * @param prefix + * the prefix in front of new paths. Typically this is the + * standard string {@code "b/"}, but may be any prefix desired by + * the caller. Must not be null. Use the empty string to have no + * prefix at all. + */ + public void setNewPrefix(String prefix) { + newPrefix = prefix; + } + /** * Flush the underlying output stream of this formatter. * @@ -228,89 +263,6 @@ private void writeGitLinkDiffText(OutputStream o, DiffEntry ent) } } - private void writeDiffHeader(OutputStream o, DiffEntry ent) - throws IOException { - String oldName = quotePath("a/" + ent.getOldPath()); - String newName = quotePath("b/" + ent.getNewPath()); - o.write(encode("diff --git " + oldName + " " + newName + "\n")); - - switch (ent.getChangeType()) { - case ADD: - o.write(encodeASCII("new file mode ")); - ent.getNewMode().copyTo(o); - o.write('\n'); - break; - - case DELETE: - o.write(encodeASCII("deleted file mode ")); - ent.getOldMode().copyTo(o); - o.write('\n'); - break; - - case RENAME: - o.write(encodeASCII("similarity index " + ent.getScore() + "%")); - o.write('\n'); - - o.write(encode("rename from " + quotePath(ent.getOldPath()))); - o.write('\n'); - - o.write(encode("rename to " + quotePath(ent.getNewPath()))); - o.write('\n'); - break; - - case COPY: - o.write(encodeASCII("similarity index " + ent.getScore() + "%")); - o.write('\n'); - - o.write(encode("copy from " + quotePath(ent.getOldPath()))); - o.write('\n'); - - o.write(encode("copy to " + quotePath(ent.getNewPath()))); - o.write('\n'); - - if (!ent.getOldMode().equals(ent.getNewMode())) { - o.write(encodeASCII("new file mode ")); - ent.getNewMode().copyTo(o); - o.write('\n'); - } - break; - case MODIFY: - int score = ent.getScore(); - if (0 < score && score <= 100) { - o.write(encodeASCII("dissimilarity index " + (100 - score) - + "%")); - o.write('\n'); - } - break; - } - - switch (ent.getChangeType()) { - case RENAME: - case MODIFY: - if (!ent.getOldMode().equals(ent.getNewMode())) { - o.write(encodeASCII("old mode ")); - ent.getOldMode().copyTo(o); - o.write('\n'); - - o.write(encodeASCII("new mode ")); - ent.getNewMode().copyTo(o); - o.write('\n'); - } - } - - o.write(encodeASCII("index " // - + format(ent.getOldId()) // - + ".." // - + format(ent.getNewId()))); - if (ent.getOldMode().equals(ent.getNewMode())) { - o.write(' '); - ent.getNewMode().copyTo(o); - } - o.write('\n'); - o.write(encode("--- " + oldName + '\n')); - o.write(encode("+++ " + newName + '\n')); - } - private String format(AbbreviatedObjectId id) { if (id.isComplete() && db != null) { ObjectReader reader = db.newObjectReader(); @@ -596,12 +548,14 @@ private FormatResult createFormatResult(DiffEntry ent) throws IOException, final EditList editList; final FileHeader.PatchType type; - writeDiffHeader(buf, ent); + formatHeader(buf, ent); if (ent.getOldMode() == GITLINK || ent.getNewMode() == GITLINK) { + formatOldNewPaths(buf, ent); writeGitLinkDiffText(buf, ent); editList = new EditList(); type = PatchType.UNIFIED; + } else { if (db == null) throw new IllegalStateException( @@ -616,14 +570,28 @@ private FormatResult createFormatResult(DiffEntry ent) throws IOException, } if (RawText.isBinary(aRaw) || RawText.isBinary(bRaw)) { + formatOldNewPaths(buf, ent); buf.write(encodeASCII("Binary files differ\n")); editList = new EditList(); type = PatchType.BINARY; + } else { res.a = rawTextFactory.create(aRaw); res.b = rawTextFactory.create(bRaw); editList = new MyersDiff(res.a, res.b).getEdits(); type = PatchType.UNIFIED; + + switch (ent.getChangeType()) { + case RENAME: + case COPY: + if (!editList.isEmpty()) + formatOldNewPaths(buf, ent); + break; + + default: + formatOldNewPaths(buf, ent); + break; + } } } @@ -631,6 +599,119 @@ private FormatResult createFormatResult(DiffEntry ent) throws IOException, return res; } + private void formatHeader(ByteArrayOutputStream o, DiffEntry ent) + throws IOException { + final ChangeType type = ent.getChangeType(); + final String oldp = ent.getOldPath(); + final String newp = ent.getNewPath(); + final FileMode oldMode = ent.getOldMode(); + final FileMode newMode = ent.getNewMode(); + + o.write(encodeASCII("diff --git ")); + o.write(encode(quotePath(oldPrefix + (type == ADD ? newp : oldp)))); + o.write(' '); + o.write(encode(quotePath(newPrefix + (type == DELETE ? oldp : newp)))); + o.write('\n'); + + switch (type) { + case ADD: + o.write(encodeASCII("new file mode ")); + newMode.copyTo(o); + o.write('\n'); + break; + + case DELETE: + o.write(encodeASCII("deleted file mode ")); + oldMode.copyTo(o); + o.write('\n'); + break; + + case RENAME: + o.write(encodeASCII("similarity index " + ent.getScore() + "%")); + o.write('\n'); + + o.write(encode("rename from " + quotePath(oldp))); + o.write('\n'); + + o.write(encode("rename to " + quotePath(newp))); + o.write('\n'); + break; + + case COPY: + o.write(encodeASCII("similarity index " + ent.getScore() + "%")); + o.write('\n'); + + o.write(encode("copy from " + quotePath(oldp))); + o.write('\n'); + + o.write(encode("copy to " + quotePath(newp))); + o.write('\n'); + + if (!oldMode.equals(newMode)) { + o.write(encodeASCII("new file mode ")); + newMode.copyTo(o); + o.write('\n'); + } + break; + + case MODIFY: + if (0 < ent.getScore()) { + o.write(encodeASCII("dissimilarity index " + + (100 - ent.getScore()) + "%")); + o.write('\n'); + } + break; + } + + if ((type == MODIFY || type == RENAME) && !oldMode.equals(newMode)) { + o.write(encodeASCII("old mode ")); + oldMode.copyTo(o); + o.write('\n'); + + o.write(encodeASCII("new mode ")); + newMode.copyTo(o); + o.write('\n'); + } + + if (!ent.getOldId().equals(ent.getNewId())) { + o.write(encodeASCII("index " // + + format(ent.getOldId()) // + + ".." // + + format(ent.getNewId()))); + if (oldMode.equals(newMode)) { + o.write(' '); + newMode.copyTo(o); + } + o.write('\n'); + } + } + + private void formatOldNewPaths(ByteArrayOutputStream o, DiffEntry ent) + throws IOException { + final String oldp; + final String newp; + + switch (ent.getChangeType()) { + case ADD: + oldp = DiffEntry.DEV_NULL; + newp = quotePath(newPrefix + ent.getNewPath()); + break; + + case DELETE: + oldp = quotePath(oldPrefix + ent.getOldPath()); + newp = DiffEntry.DEV_NULL; + break; + + default: + oldp = quotePath(oldPrefix + ent.getOldPath()); + newp = quotePath(newPrefix + ent.getNewPath()); + break; + } + + o.write(encode("--- " + oldp + "\n")); + o.write(encode("+++ " + newp + "\n")); + } + private int findCombinedEnd(final List edits, final int i) { int end = i + 1; while (end < edits.size()