opendev/gerrit
Code Issues Proposed changes

Refactor: Add a new class to represent an object that can be diffed

Browse Source 
In the change screen a user can select various objects as base for the
diff view: a regular patch set, the base of a patch set, the parent of
a merge, the auto-merge of a merge or an edit patch set. At the moment
all of these are represented by a PatchSet.Id:

* regular patch set: PatchSet.Id with id > 0
* base of a patch set: null
* the parent of a merge: PatchSet.Id with id < 0
* auto-merge of a merge: null
* edit patch set: PatchSet.Id with id == 0

This means if we only look at the patch set ID and it is null, we
don't know if it's the base of a patch set or the auto-merge of a
merge. To understand what null means, we must know if we are looking
at a merge commit, but this knowledge is not available in all classes.

This change refactors the UI classes to take a DiffObject as base
instead of a PatchSet.Id so that in future we can differentiate
between the base of a patch set and the auto-merge of a merge (see
follow-up change).

Change-Id: Ie3097ae4ff6435eef909f62eced8e6e82ff65de6
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2016-09-14 12:55:36 +02:00
parent 2b9836c141
commit 1a865ce7fd
17 changed files with 330 additions and 141 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -14,6 +14,7 @@
package com.google.gerrit.client.diff;
import com.google.gerrit.client.DiffObject;
import com.google.gerrit.client.Dispatcher;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.account.DiffPreferences;
@@ -87,7 +88,7 @@ public class Header extends Composite {
@UiField Image preferences;
private final KeyCommandSet keys;
private final PatchSet.Id base;
private final DiffObject base;
private final PatchSet.Id patchSetId;
private final String path;
private final DiffView diffScreenType;
@@ -99,12 +100,12 @@ public class Header extends Composite {
private PreferencesAction prefsAction;
private ReviewedState reviewedState;
Header(KeyCommandSet keys, PatchSet.Id base, PatchSet.Id patchSetId,
Header(KeyCommandSet keys, DiffObject base, DiffObject patchSetId,
String path, DiffView diffSreenType, DiffPreferences prefs) {
initWidget(uiBinder.createAndBindUi(this));
this.keys = keys;
this.base = base;
this.patchSetId = patchSetId;
this.patchSetId = patchSetId.asPatchSetId();
this.path = path;
this.diffScreenType = diffSreenType;
this.prefs = prefs;
@@ -113,9 +114,9 @@ public class Header extends Composite {
reviewed.getElement().getStyle().setVisibility(Visibility.HIDDEN);
}
SafeHtml.setInnerHTML(filePath, formatPath(path));
up.setTargetHistoryToken(PageLinks.toChange(
patchSetId.getParentKey(),
base != null ? base.getId() : null, patchSetId.getId()));
up.setTargetHistoryToken(
PageLinks.toChange(patchSetId.asPatchSetId().getParentKey(),
base.asString(), patchSetId.asPatchSetId().getId()));
}
public static SafeHtml formatPath(String path) {
@@ -147,16 +148,17 @@ public class Header extends Composite {
@Override
protected void onLoad() {
DiffApi.list(patchSetId, base, new GerritCallback<NativeMap<FileInfo>>() {
@Override
public void onSuccess(NativeMap<FileInfo> result) {
files = result.values();
FileInfo.sortFileInfoByPath(files);
fileNumber.setInnerText(
Integer.toString(Natives.asList(files).indexOf(result.get(path)) + 1));
fileCount.setInnerText(Integer.toString(files.length()));
}
});
DiffApi.list(patchSetId, base.asPatchSetId(),
new GerritCallback<NativeMap<FileInfo>>() {
@Override
public void onSuccess(NativeMap<FileInfo> result) {
files = result.values();
FileInfo.sortFileInfoByPath(files);
fileNumber.setInnerText(Integer
.toString(Natives.asList(files).indexOf(result.get(path)) + 1));
fileCount.setInnerText(Integer.toString(files.length()));
}
});
if (Gerrit.isSignedIn()) {
ChangeApi.revision(patchSetId).view("files")