DiffScreen: Respect the "Skip Deleted" diff preference

The effect of this preference wasn't migrated from the old diff screen
to the current one, and the prefernce hasn't been available in the diff
preferences box, but the preference itself was migrated from ReviewDb to
NoteDb (the Git backend).

This change updates the diff screen header to respect this preference.
Now that the side-by-side and the unified diff screens share the same
code base, this feature is made available in both of them.

Note that currently, the navigation links and keyboard shortcuts are not
updated upon toggling the option in PreferencesBox: they are only
updated when the preference is saved and the diff screen is reloaded.

To support updating without refreshing, we will need to replace the
KeyCommand in GlobalKey, which is of low priority and will be
implemented in a future change.

Bug: Issue 3445
Change-Id: I3e98f58a24a419dc7579525c89986cff4d0a4ee9
This commit is contained in:
Michael Zhou
2016-04-11 06:00:41 -04:00
parent ed2525fcdd
commit 9ee5cf15dc
4 changed files with 65 additions and 20 deletions

View File

@@ -138,7 +138,8 @@ abstract class DiffScreen extends Screen {
prefs = DiffPreferences.create(Gerrit.getDiffPreferences()); prefs = DiffPreferences.create(Gerrit.getDiffPreferences());
handlers = new ArrayList<>(6); handlers = new ArrayList<>(6);
keysNavigation = new KeyCommandSet(Gerrit.C.sectionNavigation()); keysNavigation = new KeyCommandSet(Gerrit.C.sectionNavigation());
header = new Header(keysNavigation, base, revision, path, diffScreenType); header = new Header(
keysNavigation, base, revision, path, diffScreenType, prefs);
} }
@Override @Override

View File

@@ -16,6 +16,7 @@ package com.google.gerrit.client.diff;
import com.google.gerrit.client.Dispatcher; import com.google.gerrit.client.Dispatcher;
import com.google.gerrit.client.Gerrit; import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.account.DiffPreferences;
import com.google.gerrit.client.changes.ChangeApi; import com.google.gerrit.client.changes.ChangeApi;
import com.google.gerrit.client.changes.ReviewInfo; import com.google.gerrit.client.changes.ReviewInfo;
import com.google.gerrit.client.changes.Util; import com.google.gerrit.client.changes.Util;
@@ -35,6 +36,7 @@ import com.google.gerrit.client.ui.InlineHyperlink;
import com.google.gerrit.common.PageLinks; import com.google.gerrit.common.PageLinks;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DiffView; import com.google.gerrit.extensions.client.GeneralPreferencesInfo.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.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gwt.core.client.GWT; import com.google.gwt.core.client.GWT;
import com.google.gwt.core.client.JsArray; import com.google.gwt.core.client.JsArray;
@@ -91,6 +93,7 @@ public class Header extends Composite {
private final PatchSet.Id patchSetId; private final PatchSet.Id patchSetId;
private final String path; private final String path;
private final DiffView diffScreenType; private final DiffView diffScreenType;
private final DiffPreferences prefs;
private boolean hasPrev; private boolean hasPrev;
private boolean hasNext; private boolean hasNext;
private String nextPath; private String nextPath;
@@ -98,13 +101,14 @@ public class Header extends Composite {
private ReviewedState reviewedState; private ReviewedState reviewedState;
Header(KeyCommandSet keys, PatchSet.Id base, PatchSet.Id patchSetId, Header(KeyCommandSet keys, PatchSet.Id base, PatchSet.Id patchSetId,
String path, DiffView diffSreenType) { String path, DiffView diffSreenType, DiffPreferences prefs) {
initWidget(uiBinder.createAndBindUi(this)); initWidget(uiBinder.createAndBindUi(this));
this.keys = keys; this.keys = keys;
this.base = base; this.base = base;
this.patchSetId = patchSetId; this.patchSetId = patchSetId;
this.path = path; this.path = path;
this.diffScreenType = diffSreenType; this.diffScreenType = diffSreenType;
this.prefs = prefs;
if (!Gerrit.isSignedIn()) { if (!Gerrit.isSignedIn()) {
reviewed.getElement().getStyle().setVisibility(Visibility.HIDDEN); reviewed.getElement().getStyle().setVisibility(Visibility.HIDDEN);
@@ -141,6 +145,17 @@ public class Header extends Composite {
return b; return b;
} }
private int findCurrentFileIndex(JsArray<FileInfo> files) {
int currIndex = 0;
for (int i = 0; i < files.length(); i++) {
if (path.equals(files.get(i).path())) {
currIndex = i;
break;
}
}
return currIndex;
}
@Override @Override
protected void onLoad() { protected void onLoad() {
DiffApi.list(patchSetId, base, new GerritCallback<NativeMap<FileInfo>>() { DiffApi.list(patchSetId, base, new GerritCallback<NativeMap<FileInfo>>() {
@@ -151,24 +166,7 @@ public class Header extends Composite {
fileNumber.setInnerText( fileNumber.setInnerText(
Integer.toString(Natives.asList(files).indexOf(result.get(path)) + 1)); Integer.toString(Natives.asList(files).indexOf(result.get(path)) + 1));
fileCount.setInnerText(Integer.toString(files.length())); fileCount.setInnerText(Integer.toString(files.length()));
int index = 0; // TODO: Maybe use patchIndex. setupPrevNextFiles(files, findCurrentFileIndex(files));
for (int i = 0; i < files.length(); i++) {
if (path.equals(files.get(i).path())) {
index = i;
break;
}
}
FileInfo nextInfo = index == files.length() - 1
? null
: files.get(index + 1);
KeyCommand p = setupNav(prev, '[', PatchUtil.C.previousFileHelp(),
index == 0 ? null : files.get(index - 1));
KeyCommand n = setupNav(next, ']', PatchUtil.C.nextFileHelp(),
nextInfo);
if (p != null && n != null) {
keys.pair(p, n);
}
nextPath = nextInfo != null ? nextInfo.path() : null;
} }
}); });
@@ -302,6 +300,37 @@ public class Header extends Composite {
} }
} }
void setupPrevNextFiles(JsArray<FileInfo> files, int currIndex) {
FileInfo prevInfo = null;
FileInfo nextInfo = null;
for (int i = currIndex - 1; i >= 0; i--) {
FileInfo curr = files.get(i);
if (prefs.skipDeleted() && ChangeType.DELETED.matches(curr.status())) {
continue;
} else {
prevInfo = curr;
break;
}
}
for (int i = currIndex + 1; i < files.length(); i++) {
FileInfo curr = files.get(i);
if (prefs.skipDeleted() && ChangeType.DELETED.matches(curr.status())) {
continue;
} else {
nextInfo = curr;
break;
}
}
KeyCommand p = setupNav(prev, '[', PatchUtil.C.previousFileHelp(),
prevInfo);
KeyCommand n = setupNav(next, ']', PatchUtil.C.nextFileHelp(),
nextInfo);
if (p != null && n != null) {
keys.pair(p, n);
}
nextPath = nextInfo != null ? nextInfo.path() : null;
}
Runnable toggleReviewed() { Runnable toggleReviewed() {
return new Runnable() { return new Runnable() {
@Override @Override

View File

@@ -102,6 +102,7 @@ public class PreferencesBox extends Composite {
@UiField ToggleButton expandAllComments; @UiField ToggleButton expandAllComments;
@UiField ToggleButton renderEntireFile; @UiField ToggleButton renderEntireFile;
@UiField ToggleButton matchBrackets; @UiField ToggleButton matchBrackets;
@UiField ToggleButton skipDeleted;
@UiField ListBox theme; @UiField ListBox theme;
@UiField Element modeLabel; @UiField Element modeLabel;
@UiField ListBox mode; @UiField ListBox mode;
@@ -194,6 +195,7 @@ public class PreferencesBox extends Composite {
manualReview.setValue(prefs.manualReview()); manualReview.setValue(prefs.manualReview());
expandAllComments.setValue(prefs.expandAllComments()); expandAllComments.setValue(prefs.expandAllComments());
matchBrackets.setValue(prefs.matchBrackets()); matchBrackets.setValue(prefs.matchBrackets());
skipDeleted.setValue(!prefs.skipDeleted());
setTheme(prefs.theme()); setTheme(prefs.theme());
if (view == null || view.canRenderEntireFile(prefs)) { if (view == null || view.canRenderEntireFile(prefs)) {
@@ -497,6 +499,12 @@ public class PreferencesBox extends Composite {
prefs.matchBrackets()); prefs.matchBrackets());
} }
@UiHandler("skipDeleted")
void onSkipDeleted(ValueChangeEvent<Boolean> e) {
prefs.skipDeleted(!e.getValue());
// TODO: Update the navigation links on the current DiffScreen
}
@UiHandler("theme") @UiHandler("theme")
void onTheme(@SuppressWarnings("unused") ChangeEvent e) { void onTheme(@SuppressWarnings("unused") ChangeEvent e) {
final Theme newTheme = getSelectedTheme(); final Theme newTheme = getSelectedTheme();

View File

@@ -296,6 +296,13 @@ limitations under the License.
<g:downFace><ui:msg>On</ui:msg></g:downFace> <g:downFace><ui:msg>On</ui:msg></g:downFace>
</g:ToggleButton></td> </g:ToggleButton></td>
</tr> </tr>
<tr>
<th><ui:msg>Skip Deleted Files</ui:msg></th>
<td><g:ToggleButton ui:field='skipDeleted'>
<g:upFace><ui:msg>Yes</ui:msg></g:upFace>
<g:downFace><ui:msg>No</ui:msg></g:downFace>
</g:ToggleButton></td>
</tr>
<tr> <tr>
<td></td> <td></td>
<td> <td>