Add blame link against parent tree in single-file diff view This helps enable the workflow outlined in Issue 5, with one click for each step: -View blame of a file. -Check full diff of a revision referred to by one line. -See that was not the diff we were looking for, check blame at the parent revision. Change-Id: Ib11706aaa5bab1a7ffe81f34568a72268e6393ea
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 3048aa8..76f5273 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/CommitData.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/CommitData.java
@@ -66,6 +66,7 @@ LOG_URL, MESSAGE, PARENTS, + PARENT_BLAME_URL, SHA, SHORT_MESSAGE, TAGS,
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 3e77d8d..8127d6d 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/CommitSoyData.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/CommitSoyData.java
@@ -52,6 +52,9 @@ Field.COMMITTER, Field.SHA, Field.TREE, Field.TREE_URL, Field.PARENTS, Field.MESSAGE, Field.LOG_URL, Field.ARCHIVE_URL, Field.ARCHIVE_TYPE); + private static final ImmutableSet<Field> NESTED_FIELDS = Sets.immutableEnumSet( + Field.PARENT_BLAME_URL); + private Linkifier linkifier; private RevWalk walk; private ArchiveFormat archiveFormat; @@ -111,7 +114,7 @@ data.put("treeUrl", cd.treeUrl); } if (cd.parents != null) { - data.put("parents", toSoyData(view, cd.parents)); + data.put("parents", toSoyData(view, fs, cd.parents)); } if (cd.shortMessage != null) { data.put("shortMessage", cd.shortMessage); @@ -132,7 +135,8 @@ if (cd.diffEntries != null) { data.put("diffTree", toSoyData(view, cd.diffEntries)); } - checkState(fs.size() == data.size(), "bad commit data fields: %s != %s", fs, data.keySet()); + checkState(Sets.difference(fs, NESTED_FIELDS).size() == data.size(), + "bad commit data fields: %s != %s", fs, data.keySet()); return ImmutableMap.copyOf(data); } @@ -151,7 +155,8 @@ "relativeTime", RelativeDateFormatter.format(ident.getWhen())); } - private List<Map<String, String>> toSoyData(GitilesView view, List<RevCommit> parents) { + private List<Map<String, String>> toSoyData(GitilesView view, Set<Field> fs, + List<RevCommit> parents) { List<Map<String, String>> result = Lists.newArrayListWithCapacity(parents.size()); int i = 1; // TODO(dborowitz): Render something slightly different when we're actively @@ -167,13 +172,21 @@ } else { parentName = view.getRevision().getName() + "^" + (i++); } - result.add(ImmutableMap.of( - "sha", name, - "url", GitilesView.revision() - .copyFrom(view) - .setRevision(parentName, parent) - .toUrl(), - "diffUrl", diff.setOldRevision(parentName, parent).toUrl())); + Map<String, String> e = Maps.newHashMapWithExpectedSize(4); + e.put("sha", name); + e.put("url", GitilesView.revision() + .copyFrom(view) + .setRevision(parentName, parent) + .toUrl()); + e.put("diffUrl", diff.setOldRevision(parentName, parent).toUrl()); + if (fs.contains(Field.PARENT_BLAME_URL)) { + // Assumes caller has ensured path is a file. + e.put("blameUrl", GitilesView.blame() + .copyFrom(view) + .setRevision(Revision.peeled(parentName, parent)) + .toUrl()); + } + result.add(e); } return result; }
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/DiffServlet.java b/gitiles-servlet/src/main/java/com/google/gitiles/DiffServlet.java index bf90bf7..ebe89e2 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/DiffServlet.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/DiffServlet.java
@@ -18,11 +18,13 @@ import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; import com.google.common.base.Charsets; +import com.google.gitiles.CommitData.Field; import org.eclipse.jgit.diff.DiffFormatter; import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.http.server.ServletUtils; +import org.eclipse.jgit.lib.FileMode; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; @@ -30,6 +32,7 @@ import org.eclipse.jgit.treewalk.AbstractTreeIterator; import org.eclipse.jgit.treewalk.CanonicalTreeParser; import org.eclipse.jgit.treewalk.EmptyTreeIterator; +import org.eclipse.jgit.treewalk.TreeWalk; import org.eclipse.jgit.treewalk.filter.PathFilter; import org.eclipse.jgit.util.GitDateFormatter; import org.eclipse.jgit.util.GitDateFormatter.Format; @@ -38,6 +41,7 @@ import java.io.OutputStream; import java.util.Arrays; import java.util.Map; +import java.util.Set; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -62,19 +66,17 @@ RevWalk walk = new RevWalk(repo); try { - boolean showCommit; + boolean showCommit, isFile; AbstractTreeIterator oldTree; AbstractTreeIterator newTree; try { // If we are viewing the diff between a commit and one of its parents, // include the commit detail in the rendered page. showCommit = isParentOf(walk, view.getOldRevision(), view.getRevision()); + isFile = showCommit ? isFile(walk, view) : false; oldTree = getTreeIterator(walk, view.getOldRevision().getId()); newTree = getTreeIterator(walk, view.getRevision().getId()); - } catch (MissingObjectException e) { - res.setStatus(SC_NOT_FOUND); - return; - } catch (IncorrectObjectTypeException e) { + } catch (MissingObjectException | IncorrectObjectTypeException e) { res.setStatus(SC_NOT_FOUND); return; } @@ -82,11 +84,15 @@ Map<String, Object> data = getData(req); data.put("title", "Diff - " + view.getRevisionRange()); if (showCommit) { + Set<Field> fs = CommitSoyData.DEFAULT_FIELDS; + if (isFile) { + fs = Field.setOf(fs, Field.PARENT_BLAME_URL); + } GitDateFormatter df = new GitDateFormatter(Format.DEFAULT); data.put("commit", new CommitSoyData() .setLinkifier(linkifier) .setArchiveFormat(getArchiveFormat(getAccess(req))) - .toSoyData(req, walk.parseCommit(view.getRevision().getId()), df)); + .toSoyData(req, walk.parseCommit(view.getRevision().getId()), fs, df)); } if (!data.containsKey("repositoryName") && (view.getRepositoryName() != null)) { data.put("repositoryName", view.getRepositoryName()); @@ -124,6 +130,21 @@ } } + private static boolean isFile(RevWalk walk, GitilesView view) throws IOException { + if (view.getPathPart().equals("")) { + return false; + } + TreeWalk tw = TreeWalk.forPath( + walk.getObjectReader(), + view.getPathPart(), + walk.parseTree(view.getRevision().getId())); + try { + return (tw.getRawMode(0) & FileMode.TYPE_FILE) > 0; + } finally { + tw.release(); + } + } + private String[] renderAndSplit(Map<String, Object> data) { String html = renderer.newRenderer("gitiles.diffDetail") .setData(data)
diff --git a/gitiles-servlet/src/main/resources/com/google/gitiles/templates/ObjectDetail.soy b/gitiles-servlet/src/main/resources/com/google/gitiles/templates/ObjectDetail.soy index c6ca22e..66a0d75 100644 --- a/gitiles-servlet/src/main/resources/com/google/gitiles/templates/ObjectDetail.soy +++ b/gitiles-servlet/src/main/resources/com/google/gitiles/templates/ObjectDetail.soy
@@ -25,6 +25,7 @@ * sha: SHA-1. * url: URL to view the parent commit. * diffUrl: URL to display diffs relative to this parent. + * blameUrl: optional URL to display blame of a file at this parent. * @param message list of commit message parts, where each part contains: * text: raw text of the part. * url: optional URL that should be linked to from the part. @@ -74,6 +75,9 @@ <a href="{$parent.url}">{$parent.sha}</a> <span class="diff-link"> [<a href="{$parent.diffUrl}">{msg desc="text for the parent diff link"}diff{/msg}</a>] + {if isNonnull($parent.blameUrl)} + {sp}[<a href="{$parent.blameUrl}">{msg desc="text for the parent blame link"}blame{/msg}</a>] + {/if} </span> </td> </tr>