opendev/gerrit
Code Issues Proposed changes

Dispatcher: Fix patch url generation

Browse Source 
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
This commit is contained in:
Michael Zhou
2016-03-16 15:16:41 -04:00
committed by David Pursehouse
parent e76c28b9d7
commit 7209b57cca
6 changed files with 21 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java
View File

@@ -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)) {

9
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffScreen.java
View File

@@ -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);

8
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.java
View File

@@ -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());
}

8
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSetSelectBox.java
View File

@@ -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() {

3
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide.java
View File

@@ -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));

5
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Unified.java
View File

@@ -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();