From 7209b57cca7b9d1fc04b0f1fe5d94a97ce33ff25 Mon Sep 17 00:00:00 2001 From: Michael Zhou Date: Wed, 16 Mar 2016 15:16:41 -0400 Subject: [PATCH] Dispatcher: Fix patch url generation Now that we have a new unified diff screen, we need an explicit ",sidebyside" appended to the URL. We also don't need to check if the DiffInfo is binary(). Both SideBySide and Unified can handle binary diffs just fine. Remove the DiffScreenType enum from DiffScreen and just use DiffView. Change-Id: I86faaeb14b481d854805949cd649b14f49932fa0 --- .../main/java/com/google/gerrit/client/Dispatcher.java | 6 +++--- .../java/com/google/gerrit/client/diff/DiffScreen.java | 9 ++++----- .../main/java/com/google/gerrit/client/diff/Header.java | 8 ++++---- .../com/google/gerrit/client/diff/PatchSetSelectBox.java | 8 +++++--- .../java/com/google/gerrit/client/diff/SideBySide.java | 3 ++- .../main/java/com/google/gerrit/client/diff/Unified.java | 5 +++-- 6 files changed, 21 insertions(+), 18 deletions(-) diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java index 043a54ad6b..32604b3618 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java @@ -112,12 +112,12 @@ public class Dispatcher { public static String toSideBySide(PatchSet.Id diffBase, PatchSet.Id revision, String fileName) { - return toPatch("", diffBase, revision, fileName, null, 0); + return toPatch("sidebyside", diffBase, revision, fileName, null, 0); } public static String toSideBySide(PatchSet.Id diffBase, PatchSet.Id revision, String fileName, DisplaySide side, int line) { - return toPatch("", diffBase, revision, fileName, side, line); + return toPatch("sidebyside", diffBase, revision, fileName, side, line); } public static String toUnified(PatchSet.Id diffBase, @@ -477,7 +477,7 @@ public class Dispatcher { codemirror(token, baseId, id, side, line, false); } } else if ("sidebyside".equals(panel)) { - codemirror(token, null, id, side, line, false); + codemirror(token, baseId, id, side, line, false); } else if ("unified".equals(panel)) { unified(token, baseId, id, side, line); } else if ("unified1".equals(panel)) { diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffScreen.java index d657a12028..4649d81d14 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffScreen.java @@ -39,6 +39,7 @@ import com.google.gerrit.client.rpc.RestApi; import com.google.gerrit.client.rpc.ScreenLoadCallback; import com.google.gerrit.client.ui.Screen; import com.google.gerrit.common.PageLinks; +import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DiffView; import com.google.gerrit.extensions.client.ListChangesOption; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Patch; @@ -94,15 +95,12 @@ abstract class DiffScreen extends Screen { } } - enum DiffScreenType { - SIDE_BY_SIDE, UNIFIED - } - private final Change.Id changeId; final PatchSet.Id base; final PatchSet.Id revision; final String path; final DiffPreferences prefs; + final DiffView diffScreenType; private DisplaySide startSide; private int startLine; @@ -129,13 +127,14 @@ abstract class DiffScreen extends Screen { String path, DisplaySide startSide, int startLine, - DiffScreenType diffScreenType) { + DiffView diffScreenType) { this.base = base; this.revision = revision; this.changeId = revision.getParentKey(); this.path = path; this.startSide = startSide; this.startLine = startLine; + this.diffScreenType = diffScreenType; prefs = DiffPreferences.create(Gerrit.getDiffPreferences()); handlers = new ArrayList<>(6); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.java index b723e8fd2c..a554df6b85 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.java @@ -20,7 +20,6 @@ import com.google.gerrit.client.changes.ChangeApi; import com.google.gerrit.client.changes.ReviewInfo; import com.google.gerrit.client.changes.Util; import com.google.gerrit.client.diff.DiffInfo.Region; -import com.google.gerrit.client.diff.DiffScreen.DiffScreenType; import com.google.gerrit.client.info.ChangeInfo; import com.google.gerrit.client.info.ChangeInfo.RevisionInfo; import com.google.gerrit.client.info.FileInfo; @@ -34,6 +33,7 @@ import com.google.gerrit.client.rpc.Natives; import com.google.gerrit.client.rpc.RestApi; import com.google.gerrit.client.ui.InlineHyperlink; import com.google.gerrit.common.PageLinks; +import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DiffView; import com.google.gerrit.reviewdb.client.Patch; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gwt.core.client.GWT; @@ -90,7 +90,7 @@ public class Header extends Composite { private final PatchSet.Id base; private final PatchSet.Id patchSetId; private final String path; - private final DiffScreenType diffScreenType; + private final DiffView diffScreenType; private boolean hasPrev; private boolean hasNext; private String nextPath; @@ -98,7 +98,7 @@ public class Header extends Composite { private ReviewedState reviewedState; Header(KeyCommandSet keys, PatchSet.Id base, PatchSet.Id patchSetId, - String path, DiffScreenType diffSreenType) { + String path, DiffView diffSreenType) { initWidget(uiBinder.createAndBindUi(this)); this.keys = keys; this.base = base; @@ -270,7 +270,7 @@ public class Header extends Composite { } private String url(FileInfo info) { - return diffScreenType == DiffScreenType.UNIFIED || info.binary() + return diffScreenType == DiffView.UNIFIED_DIFF ? Dispatcher.toUnified(base, patchSetId, info.path()) : Dispatcher.toSideBySide(base, patchSetId, info.path()); } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSetSelectBox.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSetSelectBox.java index 8547d1a25b..9f106cef0a 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSetSelectBox.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSetSelectBox.java @@ -21,6 +21,7 @@ import com.google.gerrit.client.info.WebLinkInfo; import com.google.gerrit.client.patches.PatchUtil; import com.google.gerrit.client.rpc.Natives; import com.google.gerrit.client.ui.InlineHyperlink; +import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DiffView; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Patch; import com.google.gerrit.reviewdb.client.PatchSet; @@ -146,9 +147,10 @@ class PatchSetSelectBox extends Composite { PatchSet.Id diffBase = sideA ? id : other.idActive; PatchSet.Id revision = sideA ? other.idActive : id; - return new InlineHyperlink(label, parent instanceof SideBySide - ? Dispatcher.toSideBySide(diffBase, revision, path) - : Dispatcher.toUnified(diffBase, revision, path)); + return new InlineHyperlink(label, + parent.diffScreenType == DiffView.SIDE_BY_SIDE + ? Dispatcher.toSideBySide(diffBase, revision, path) + : Dispatcher.toUnified(diffBase, revision, path)); } private Anchor createDownloadLink() { diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide.java index de08e8b432..ed174d2a51 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide.java @@ -23,6 +23,7 @@ import com.google.gerrit.client.patches.PatchUtil; import com.google.gerrit.client.projects.ConfigInfoCache; import com.google.gerrit.client.rpc.ScreenLoadCallback; import com.google.gerrit.client.ui.InlineHyperlink; +import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DiffView; import com.google.gerrit.reviewdb.client.Patch; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gwt.core.client.GWT; @@ -73,7 +74,7 @@ public class SideBySide extends DiffScreen { String path, DisplaySide startSide, int startLine) { - super(base, revision, path, startSide, startLine, DiffScreenType.SIDE_BY_SIDE); + super(base, revision, path, startSide, startLine, DiffView.SIDE_BY_SIDE); diffTable = new SideBySideTable(this, base, revision, path); add(uiBinder.createAndBindUi(this)); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Unified.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Unified.java index f95b607151..21b7a4c713 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Unified.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Unified.java @@ -23,6 +23,7 @@ import com.google.gerrit.client.patches.PatchUtil; import com.google.gerrit.client.projects.ConfigInfoCache; import com.google.gerrit.client.rpc.ScreenLoadCallback; import com.google.gerrit.client.ui.InlineHyperlink; +import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DiffView; import com.google.gerrit.reviewdb.client.Patch; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gwt.core.client.GWT; @@ -77,7 +78,7 @@ public class Unified extends DiffScreen { String path, DisplaySide startSide, int startLine) { - super(base, revision, path, startSide, startLine, DiffScreenType.UNIFIED); + super(base, revision, path, startSide, startLine, DiffView.UNIFIED_DIFF); diffTable = new UnifiedTable(this, base, revision, path); add(uiBinder.createAndBindUi(this)); @@ -205,7 +206,7 @@ public class Unified extends DiffScreen { registerCmEvents(cm); setPrefsAction(new PreferencesAction(this, prefs)); - header.init(getPrefsAction(), getSideBySideDiffLink(), diff.sideBySideWebLinks()); + header.init(getPrefsAction(), getSideBySideDiffLink(), diff.unifiedWebLinks()); setAutoHideDiffHeader(prefs.autoHideDiffTableHeader()); setupSyntaxHighlighting();