]> source.dussan.org Git - jgit.git/commitdiff
Correct diff header formatting 85/1485/1
authorShawn O. Pearce <spearce@spearce.org>
Wed, 1 Sep 2010 02:28:15 +0000 (19:28 -0700)
committerShawn O. Pearce <spearce@spearce.org>
Wed, 1 Sep 2010 17:19:43 +0000 (10:19 -0700)
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 <spearce@spearce.org>
org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterTest.java
org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java

index 92d4fa114f984d36e1a4e1fa5eafe5e3fbca1ad3..d7a10e4b15a4032692f26eaecb6565798bb55e46 100644 (file)
@@ -77,6 +77,76 @@ public class DiffFormatterTest extends RepositoryTestCase {
                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");
index d8429a4cd8f1ba862428ff5adf3be544520f4cc5..52c9573905aa610e48d7e6a3ff42f09adf64a2db 100644 (file)
 
 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.Collection;
 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 class DiffFormatter {
                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 @@ public class DiffFormatter {
                }
        }
 
-       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 @@ public class DiffFormatter {
                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 @@ public class DiffFormatter {
                        }
 
                        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 @@ public class DiffFormatter {
                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<Edit> edits, final int i) {
                int end = i + 1;
                while (end < edits.size()