commit 2298cefe7286140ae3fc92f64274f50be1785d40
author Dave Borowitz <dborowitz@google.com> Tue Sep 23 10:29:46 2014 -0700
committer Dave Borowitz <dborowitz@google.com> Tue Sep 23 12:43:17 2014 -0700
tree 10bbcb1483caa1749f5c15c30ed0279cd002447c
parent 8d11fb7c464a046a0b85e58667d36dcf4241bbcf

Set proper old/new revisions on diff results

This happened to work on the commit detail page, but when using a
custom pretty log template to include the diff list on the log page,
it took the old revision from the topmost view, which may have been of
the form oldsha..newsha. This is inappropriate for a single log entry,
which should always contain the diff against just its parent(s).

Fix this by passing down a new revision as well as an old revision in
DiffList and actually using these where necessary.

Issue: 66
Change-Id: Ie87ea857f2be342ead395d71371fb29912aebb26

gitiles-servlet/src/main/java/com/google/gitiles/CommitData.java

diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/CommitData.java b/gitiles-servlet/src/main/java/com/google/gitiles/CommitData.java
index 76f5273..74d0422 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/CommitData.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/CommitData.java
@@ -80,6 +80,7 @@
   }
 
   static class DiffList {
+    Revision revision;
     Revision oldRevision;
     List<DiffEntry> entries;
   }
@@ -213,6 +214,10 @@
     private DiffList computeDiffEntries(Repository repo, GitilesView view, RevCommit commit)
         throws IOException {
       DiffList result = new DiffList();
+      result.revision = view.getRevision().matches(commit)
+          ? view.getRevision()
+          : Revision.peeled(commit.name(), commit);
+
       AbstractTreeIterator oldTree;
       switch (commit.getParentCount()) {
         case 0:
@@ -220,8 +225,7 @@
           oldTree = new EmptyTreeIterator();
           break;
         case 1:
-          result.oldRevision =
-              Revision.peeled(view.getRevision().getName() + "^", commit.getParent(0));
+          result.oldRevision = Revision.peeled(result.revision.getName() + "^", commit.getParent(0));
           oldTree = getTreeIterator(commit.getParent(0));
           break;
         default:

gitiles-servlet/src/main/java/com/google/gitiles/CommitSoyData.java

diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/CommitSoyData.java b/gitiles-servlet/src/main/java/com/google/gitiles/CommitSoyData.java
index bba35f7..733da0a 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/CommitSoyData.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/CommitSoyData.java
@@ -196,7 +196,10 @@
     if (dl.oldRevision == null) {
       return NullData.INSTANCE;
     }
-    GitilesView.Builder diffUrl = GitilesView.diff().copyFrom(view)
+    GitilesView.Builder diffUrl = GitilesView.diff()
+        .copyFrom(view)
+        .setOldRevision(dl.oldRevision)
+        .setRevision(dl.revision)
         .setPathPart("");
 
     List<Object> result = Lists.newArrayListWithCapacity(dl.entries.size());
@@ -207,6 +210,7 @@
         entry.put("path", e.getNewPath());
         entry.put("url", GitilesView.path()
             .copyFrom(view)
+            .setRevision(dl.revision)
             .setPathPart(e.getNewPath())
             .toUrl());
       } else {

gitiles-servlet/src/main/java/com/google/gitiles/Revision.java

diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/Revision.java b/gitiles-servlet/src/main/java/com/google/gitiles/Revision.java
index a93d4b5..54c72fa 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/Revision.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/Revision.java
@@ -109,9 +109,12 @@
     return peeledType;
   }
 
+  public boolean matches(ObjectId other) {
+    return id.equals(other) || nameEqualsAbbreviated(other);
+  }
+
   public boolean nameIsId() {
-    return AbbreviatedObjectId.isId(name)
-        && (AbbreviatedObjectId.fromString(name).prefixCompare(id) == 0);
+    return nameEqualsAbbreviated(id);
   }
 
   @Override
@@ -143,4 +146,10 @@
         .add("peeledType", type > 0 ? Constants.typeString(peeledType) : null)
         .toString();
   }
+
+  private boolean nameEqualsAbbreviated(ObjectId other) {
+    return AbbreviatedObjectId.isId(name)
+        ? AbbreviatedObjectId.fromString(name).prefixCompare(other) == 0
+        : false;
+  }
 }