From a28ba3f2c7e0fa96f778c62813734807c99c61c3 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Thu, 17 Sep 2015 23:05:07 -0700 Subject: [PATCH] Show submodule difference as a hunk Current DiffFormat behavior regarding submodules (aka git links) is incorrect. The "Subproject commit " appears as part of the diff header, rather than as its own hunk. --> From JGit implementation diff --git a/plugins/cookbook-plugin b/plugins/cookbook-plugin index b9d3ca8..ec6ed89 160000 --- a/plugins/cookbook-plugin +++ b/plugins/cookbook-plugin -Subproject commit b9d3ca8a65030071e28be19296ba867ab424fbbf +Subproject commit ec6ed89c47ba7223f82d9cb512926a6c5081343e --> From C Git 2.5.2 diff --git a/plugins/cookbook-plugin b/plugins/cookbook-plugin index b9d3ca8..ec6ed89 160000 --- a/plugins/cookbook-plugin +++ b/plugins/cookbook-plugin @@ -1 +1 @@ -Subproject commit b9d3ca8a65030071e28be19296ba867ab424fbbf +Subproject commit ec6ed89c47ba7223f82d9cb512926a6c5081343e The current way of processing submodules results in no hunk header and includes the contents of the hunk as part of the headers. To fix this, we can't just have our writeGitLinkDiffText output the hunk header. We have to change the flow so that the raw text gets parsed as a diff. The easiest way to do this is to fake the RawText in the FormatResult when we have a GITLINK. It should be noted that it seems possible for there to be a difference between a GITLINK and a non-GITLINK, but I don't think this can happen in practice, so I don't think we need to worry too much about it. This patch also fixes up the test for GitLink headers, as the test was for the old behavior. My setup has 3 other failing tests which may or may not be the result of environmental changes. However, the same tests fail without this commit, so I do not believe they are related. Bug: 477759 Change-Id: If13f7b406904fad814416c93ed09ea47ef183337 Signed-off-by: Jacob Keller --- .../eclipse/jgit/diff/DiffFormatterTest.java | 5 ++- .../org/eclipse/jgit/diff/DiffFormatter.java | 33 ++++++++----------- 2 files changed, 16 insertions(+), 22 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 02e485d41..24b0f8141 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 @@ -241,8 +241,7 @@ public class DiffFormatterTest extends RepositoryTestCase { ObjectId bId = blob("b\n"); String diffHeader = makeDiffHeaderModeChange(PATH_A, PATH_A, aId, bId, - GITLINK, REGULAR_FILE) - + "-Subproject commit " + aId.name() + "\n"; + GITLINK, REGULAR_FILE); DiffEntry ad = DiffEntry.delete(PATH_A, aId); ad.oldMode = FileMode.GITLINK; @@ -257,7 +256,7 @@ public class DiffFormatterTest extends RepositoryTestCase { assertEquals(1, fh.getHunks().size()); HunkHeader hh = fh.getHunks().get(0); - assertEquals(0, hh.toEditList().size()); + assertEquals(1, hh.toEditList().size()); } @Test 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 bcc30c35b..15abcda56 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java @@ -665,16 +665,10 @@ public class DiffFormatter implements AutoCloseable { format(res.header, res.a, res.b); } - private static void writeGitLinkDiffText(OutputStream o, DiffEntry ent) - throws IOException { - if (ent.getOldMode() == GITLINK) { - o.write(encodeASCII("-Subproject commit " + ent.getOldId().name() //$NON-NLS-1$ - + "\n")); //$NON-NLS-1$ - } - if (ent.getNewMode() == GITLINK) { - o.write(encodeASCII("+Subproject commit " + ent.getNewId().name() //$NON-NLS-1$ - + "\n")); //$NON-NLS-1$ - } + private static byte[] writeGitLinkText(AbbreviatedObjectId id) + throws IOException { + return encodeASCII("Subproject commit " + id.name() //$NON-NLS-1$ + + "\n"); } private String format(AbbreviatedObjectId id) { @@ -938,13 +932,7 @@ public class DiffFormatter implements AutoCloseable { formatHeader(buf, ent); - if (ent.getOldMode() == GITLINK || ent.getNewMode() == GITLINK) { - formatOldNewPaths(buf, ent); - writeGitLinkDiffText(buf, ent); - editList = new EditList(); - type = PatchType.UNIFIED; - - } else if (ent.getOldId() == null || ent.getNewId() == null) { + if (ent.getOldId() == null || ent.getNewId() == null) { // Content not changed (e.g. only mode, pure rename) editList = new EditList(); type = PatchType.UNIFIED; @@ -952,8 +940,15 @@ public class DiffFormatter implements AutoCloseable { } else { assertHaveRepository(); - byte[] aRaw = open(OLD, ent); - byte[] bRaw = open(NEW, ent); + byte[] aRaw, bRaw; + + if (ent.getOldMode() == GITLINK || ent.getNewMode() == GITLINK) { + aRaw = writeGitLinkText(ent.getOldId()); + bRaw = writeGitLinkText(ent.getNewId()); + } else { + aRaw = open(OLD, ent); + bRaw = open(NEW, ent); + } if (aRaw == BINARY || bRaw == BINARY // || RawText.isBinary(aRaw) || RawText.isBinary(bRaw)) {