Fix inconsistent behavior of diff view when viewing binary files

In the new change screen, if the user clicks on a binary file in
the file list, the unified view is used.  Then when navigating to
a previous or next file that is not binary, the diff view stays in
the old unified setting.  It is only possible to get back to the
new side-by-side view by going back to the change screen and then
opening the non-binary file from there.

To keep consistent behavior of diff view, always use unified diff
for binary files.  Use the user's preference setting for non-binary
files.

Change-Id: Ie4ece4cb740df8a69cbf3d2d42e4b0fb05461520
This commit is contained in:
Bruce Zu
2014-03-26 16:03:41 +08:00
committed by David Pursehouse
parent bb9c63a3a9
commit 72bced0061
2 changed files with 22 additions and 6 deletions

View File

@@ -529,7 +529,7 @@ public class Dispatcher {
} }
} }
private static boolean isChangeScreen2() { public static boolean isChangeScreen2() {
if (changeScreen2) { if (changeScreen2) {
return true; return true;
} }
@@ -593,6 +593,9 @@ public class Dispatcher {
} }
if ("".equals(panel)) { if ("".equals(panel)) {
if (isChangeScreen2()) {
return new SideBySide2(baseId, id.getParentKey(), id.get());
}
return new PatchScreen.SideBySide( // return new PatchScreen.SideBySide( //
id, // id, //
patchIndex, // patchIndex, //

View File

@@ -22,6 +22,7 @@ import com.google.gerrit.client.ui.ListenableAccountDiffPreference;
import com.google.gerrit.client.ui.NavigationTable; import com.google.gerrit.client.ui.NavigationTable;
import com.google.gerrit.client.ui.PatchLink; import com.google.gerrit.client.ui.PatchLink;
import com.google.gerrit.common.data.PatchSetDetail; import com.google.gerrit.common.data.PatchSetDetail;
import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.DiffView;
import com.google.gerrit.reviewdb.client.Patch; import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.Patch.ChangeType; import com.google.gerrit.reviewdb.client.Patch.ChangeType;
import com.google.gerrit.reviewdb.client.Patch.Key; import com.google.gerrit.reviewdb.client.Patch.Key;
@@ -244,21 +245,23 @@ public class PatchTable extends Composite {
/** /**
* @return a link to the the given patch. * @return a link to the the given patch.
* @param index The patch to link to * @param index The patch to link to
* @param patchType The type of patch display * @param screenType The screen type of patch display
* @param before A string to display at the beginning of the href text * @param before A string to display at the beginning of the href text
* @param after A string to display at the end of the href text * @param after A string to display at the end of the href text
*/ */
public PatchLink createLink(int index, PatchScreen.Type patchType, public PatchLink createLink(int index, PatchScreen.Type screenType,
SafeHtml before, SafeHtml after) { SafeHtml before, SafeHtml after) {
Patch patch = patchList.get(index); Patch patch = patchList.get(index);
Key thisKey = patch.getKey(); Key thisKey = patch.getKey();
PatchLink link; PatchLink link;
if (patchType == PatchScreen.Type.SIDE_BY_SIDE) {
link = new PatchLink.SideBySide("", base, thisKey, index, detail, this); if (isUnifiedPatchLink(patch, screenType)) {
} else {
link = new PatchLink.Unified("", base, thisKey, index, detail, this); link = new PatchLink.Unified("", base, thisKey, index, detail, this);
} else {
link = new PatchLink.SideBySide("", base, thisKey, index, detail, this);
} }
SafeHtmlBuilder text = new SafeHtmlBuilder(); SafeHtmlBuilder text = new SafeHtmlBuilder();
text.append(before); text.append(before);
text.append(getFileNameOnly(patch)); text.append(getFileNameOnly(patch));
@@ -267,6 +270,16 @@ public class PatchTable extends Composite {
return link; return link;
} }
private static boolean isUnifiedPatchLink(final Patch patch,
final PatchScreen.Type screenType) {
if (Dispatcher.isChangeScreen2()) {
return (patch.getPatchType().equals(PatchType.BINARY)
|| Gerrit.getUserAccount().getGeneralPreferences().getDiffView()
.equals(DiffView.UNIFIED_DIFF));
}
return screenType == PatchScreen.Type.UNIFIED;
}
private static String getFileNameOnly(Patch patch) { private static String getFileNameOnly(Patch patch) {
// Note: use '/' here and not File.pathSeparator since git paths // Note: use '/' here and not File.pathSeparator since git paths
// are always separated by / // are always separated by /