Change the default for commits to revision view One of the biggest complaints we've received is that it takes too many clicks to get from the repo index page to actually view some source code. So, when clicking on a revision, show the tree. This replaces the log view as the default for a branch, which apparently users didn't find very useful (but we may add back as an additional link as part of rethinking the repo index page). Change-Id: I3d69dabd34643fa3664fa00877d11ea38c48de4a
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/GitilesView.java b/gitiles-servlet/src/main/java/com/google/gitiles/GitilesView.java index 05c31a7..1d45101 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/GitilesView.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/GitilesView.java
@@ -425,11 +425,7 @@ url.append(repositoryName).append('/'); break; case REVISION: - url.append(repositoryName).append("/+"); - if (!getRevision().nameIsId()) { - url.append("show"); // Default for /+/master is +log. - } - url.append('/').append(revision.getName()); + url.append(repositoryName).append("/+/").append(revision.getName()); break; case PATH: url.append(repositoryName).append("/+/").append(revision.getName()).append('/') @@ -445,13 +441,7 @@ url.append('/').append(path); break; case LOG: - url.append(repositoryName).append("/+"); - if (getRevision().nameIsId() || oldRevision != Revision.NULL || path != null) { - // Default for /+/c0ffee/(...) is +show. - // Default for /+/c0ffee..deadbeef(/...) is +diff. - url.append("log"); - } - url.append('/'); + url.append(repositoryName).append("/+log/"); if (oldRevision != Revision.NULL) { url.append(oldRevision.getName()).append(".."); }
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/ViewFilter.java b/gitiles-servlet/src/main/java/com/google/gitiles/ViewFilter.java index 8874b1d..e7532ab 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/ViewFilter.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/ViewFilter.java
@@ -18,10 +18,6 @@ import static com.google.common.base.Preconditions.checkNotNull; import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; -import org.eclipse.jgit.http.server.ServletUtils; -import org.eclipse.jgit.http.server.glue.WrappedRequest; -import org.eclipse.jgit.lib.Constants; - import java.io.IOException; import java.util.Map; @@ -30,6 +26,9 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jgit.http.server.ServletUtils; +import org.eclipse.jgit.http.server.glue.WrappedRequest; + /** Filter to parse URLs and convert them to {@link GitilesView}s. */ public class ViewFilter extends AbstractHttpFilter { // TODO(dborowitz): Make this public in JGit (or implement getRegexGroup @@ -147,14 +146,8 @@ return command; } else if (result.getOldRevision() != null) { return CMD_DIFF; - } - Revision rev = result.getRevision(); - if (rev.getPeeledType() != Constants.OBJ_COMMIT - || !path.isEmpty() - || result.getRevision().nameIsId()) { - return CMD_SHOW; } else { - return CMD_LOG; + return CMD_SHOW; } } }
diff --git a/gitiles-servlet/src/test/java/com/google/gitiles/GitilesViewTest.java b/gitiles-servlet/src/test/java/com/google/gitiles/GitilesViewTest.java index a8a5c4f..cf246b9 100644 --- a/gitiles-servlet/src/test/java/com/google/gitiles/GitilesViewTest.java +++ b/gitiles-servlet/src/test/java/com/google/gitiles/GitilesViewTest.java
@@ -134,12 +134,12 @@ assertNull(view.getTreePath()); assertTrue(HOST.getParameters().isEmpty()); - assertEquals("/b/foo/bar/+show/master", view.toUrl()); + assertEquals("/b/foo/bar/+/master", view.toUrl()); assertEquals( ImmutableList.of( breadcrumb("host", "/b/?format=HTML"), breadcrumb("foo/bar", "/b/foo/bar/"), - breadcrumb("master", "/b/foo/bar/+show/master")), + breadcrumb("master", "/b/foo/bar/+/master")), view.getBreadcrumbs()); } @@ -166,7 +166,7 @@ ImmutableList.of( breadcrumb("host", "/b/?format=HTML"), breadcrumb("foo/bar", "/b/foo/bar/"), - breadcrumb("master", "/b/foo/bar/+show/master"), + breadcrumb("master", "/b/foo/bar/+/master"), breadcrumb(".", "/b/foo/bar/+/master/")), view.getBreadcrumbs()); } @@ -194,7 +194,7 @@ ImmutableList.of( breadcrumb("host", "/b/?format=HTML"), breadcrumb("foo/bar", "/b/foo/bar/"), - breadcrumb("master", "/b/foo/bar/+show/master"), + breadcrumb("master", "/b/foo/bar/+/master"), breadcrumb(".", "/b/foo/bar/+/master/"), breadcrumb("file", "/b/foo/bar/+/master/file")), view.getBreadcrumbs()); @@ -223,7 +223,7 @@ ImmutableList.of( breadcrumb("host", "/b/?format=HTML"), breadcrumb("foo/bar", "/b/foo/bar/"), - breadcrumb("master", "/b/foo/bar/+show/master"), + breadcrumb("master", "/b/foo/bar/+/master"), breadcrumb(".", "/b/foo/bar/+/master/"), breadcrumb("path", "/b/foo/bar/+/master/path"), breadcrumb("to", "/b/foo/bar/+/master/path/to"), @@ -353,12 +353,12 @@ assertNull(view.getTreePath()); assertTrue(HOST.getParameters().isEmpty()); - assertEquals("/b/foo/bar/+/master", view.toUrl()); + assertEquals("/b/foo/bar/+log/master", view.toUrl()); assertEquals( ImmutableList.of( breadcrumb("host", "/b/?format=HTML"), breadcrumb("foo/bar", "/b/foo/bar/"), - breadcrumb("master", "/b/foo/bar/+/master")), + breadcrumb("master", "/b/foo/bar/+log/master")), view.getBreadcrumbs()); } @@ -413,7 +413,7 @@ ImmutableList.of( breadcrumb("host", "/b/?format=HTML"), breadcrumb("foo/bar", "/b/foo/bar/"), - breadcrumb("master", "/b/foo/bar/+/master"), + breadcrumb("master", "/b/foo/bar/+log/master"), breadcrumb("path", "/b/foo/bar/+log/master/path"), breadcrumb("to", "/b/foo/bar/+log/master/path/to"), breadcrumb("a", "/b/foo/bar/+log/master/path/to/a"),
diff --git a/gitiles-servlet/src/test/java/com/google/gitiles/ViewFilterTest.java b/gitiles-servlet/src/test/java/com/google/gitiles/ViewFilterTest.java index be0978d..800bd38 100644 --- a/gitiles-servlet/src/test/java/com/google/gitiles/ViewFilterTest.java +++ b/gitiles-servlet/src/test/java/com/google/gitiles/ViewFilterTest.java
@@ -66,8 +66,8 @@ String hexBranch = hex.substring(0, 10); repo.branch(hexBranch).commit().create(); - assertEquals(Type.LOG, getView("/repo/+/master").getType()); - assertEquals(Type.LOG, getView("/repo/+/" + hexBranch).getType()); + assertEquals(Type.REVISION, getView("/repo/+/master").getType()); + assertEquals(Type.REVISION, getView("/repo/+/" + hexBranch).getType()); assertEquals(Type.REVISION, getView("/repo/+/" + hex).getType()); assertEquals(Type.REVISION, getView("/repo/+/" + hex.substring(0, 7)).getType()); assertEquals(Type.PATH, getView("/repo/+/master/").getType());