Fix handling of comment sides

Previously, the new diff UI confused the display side (A or B)
with the side stored in CommentInfo (PARENT or REVISION), causing
comments to be displayed on the wrong side. Added DisplaySide to
differentiate the two and fixed the logic for displaying comments.

Change-Id: I2e1cbe93ccce78ce2dfd715f876a27fe7846640d
This commit is contained in:
Michael Zhou
2013-08-12 23:46:46 -07:00
parent 0d85f15381
commit eb10727840
9 changed files with 153 additions and 111 deletions

View File

@@ -16,6 +16,7 @@ package com.google.gerrit.client.diff;
import com.google.gerrit.client.changes.CommentInfo; import com.google.gerrit.client.changes.CommentInfo;
import com.google.gerrit.client.diff.PaddingManager.PaddingWidgetWrapper; import com.google.gerrit.client.diff.PaddingManager.PaddingWidgetWrapper;
import com.google.gerrit.client.diff.SideBySide2.DisplaySide;
import com.google.gerrit.client.diff.SideBySide2.DiffChunkInfo; import com.google.gerrit.client.diff.SideBySide2.DiffChunkInfo;
import com.google.gerrit.client.diff.SidePanel.GutterWrapper; import com.google.gerrit.client.diff.SidePanel.GutterWrapper;
import com.google.gwt.core.client.Scheduler; import com.google.gwt.core.client.Scheduler;
@@ -33,6 +34,11 @@ abstract class CommentBox extends Composite {
private SideBySide2 parent; private SideBySide2 parent;
private DiffChunkInfo diffChunkInfo; private DiffChunkInfo diffChunkInfo;
private GutterWrapper gutterWrapper; private GutterWrapper gutterWrapper;
private DisplaySide side;
CommentBox(DisplaySide side) {
this.side = side;
}
@Override @Override
protected void onLoad() { protected void onLoad() {
@@ -49,8 +55,7 @@ abstract class CommentBox extends Composite {
assert selfWidgetWrapper != null; assert selfWidgetWrapper != null;
selfWidgetWrapper.getWidget().changed(); selfWidgetWrapper.getWidget().changed();
if (diffChunkInfo != null) { if (diffChunkInfo != null) {
parent.resizePaddingOnOtherSide(getCommentInfo().side(), parent.resizePaddingOnOtherSide(side, diffChunkInfo.getEnd());
diffChunkInfo.getEnd());
} else { } else {
assert widgetManager != null; assert widgetManager != null;
widgetManager.resizePaddingWidget(); widgetManager.resizePaddingWidget();
@@ -97,4 +102,8 @@ abstract class CommentBox extends Composite {
GutterWrapper getGutterWrapper() { GutterWrapper getGutterWrapper() {
return gutterWrapper; return gutterWrapper;
} }
DisplaySide getSide() {
return side;
}
} }

View File

@@ -14,7 +14,7 @@
package com.google.gerrit.client.diff; package com.google.gerrit.client.diff;
import com.google.gerrit.common.changes.Side; import com.google.gerrit.client.diff.SideBySide2.DisplaySide;
import com.google.gwt.core.client.GWT; import com.google.gwt.core.client.GWT;
import com.google.gwt.dom.client.Element; import com.google.gwt.dom.client.Element;
import com.google.gwt.resources.client.CssResource; import com.google.gwt.resources.client.CssResource;
@@ -87,10 +87,10 @@ class DiffTable extends Composite {
private SideBySide2 host; private SideBySide2 host;
DiffTable(SideBySide2 host, String path) { DiffTable(SideBySide2 host, String path) {
patchSelectBoxA = new PatchSelectBox2(this, Side.PARENT); patchSelectBoxA = new PatchSelectBox2(this, DisplaySide.A);
patchSelectBoxB = new PatchSelectBox2(this, Side.REVISION); patchSelectBoxB = new PatchSelectBox2(this, DisplaySide.B);
fileCommentPanelA = new FileCommentPanel(host, this, path, Side.PARENT); fileCommentPanelA = new FileCommentPanel(host, this, path, DisplaySide.A);
fileCommentPanelB = new FileCommentPanel(host, this, path, Side.REVISION); fileCommentPanelB = new FileCommentPanel(host, this, path, DisplaySide.B);
initWidget(uiBinder.createAndBindUi(this)); initWidget(uiBinder.createAndBindUi(this));
this.host = host; this.host = host;
} }
@@ -111,21 +111,21 @@ class DiffTable extends Composite {
host.resizeCodeMirror(); host.resizeCodeMirror();
} }
private FileCommentPanel getPanelFromSide(Side side) { private FileCommentPanel getPanelFromSide(DisplaySide side) {
return side == Side.PARENT ? fileCommentPanelA : fileCommentPanelB; return side == DisplaySide.A ? fileCommentPanelA : fileCommentPanelB;
} }
void createOrEditFileComment(Side side) { void createOrEditFileComment(DisplaySide side) {
getPanelFromSide(side).createOrEditFileComment(); getPanelFromSide(side).createOrEditFileComment();
updateFileCommentVisibility(false); updateFileCommentVisibility(false);
} }
void addFileCommentBox(CommentBox box, Side side) { void addFileCommentBox(CommentBox box) {
getPanelFromSide(side).addFileComment(box); getPanelFromSide(box.getSide()).addFileComment(box);
} }
void onRemoveDraftBox(DraftBox box, Side side) { void onRemoveDraftBox(DraftBox box) {
getPanelFromSide(side).onRemoveDraftBox(box); getPanelFromSide(box.getSide()).onRemoveDraftBox(box);
} }
int getHeaderHeight() { int getHeaderHeight() {

View File

@@ -18,9 +18,9 @@ import com.google.gerrit.client.FormatUtil;
import com.google.gerrit.client.changes.CommentApi; import com.google.gerrit.client.changes.CommentApi;
import com.google.gerrit.client.changes.CommentInfo; import com.google.gerrit.client.changes.CommentInfo;
import com.google.gerrit.client.changes.CommentInput; import com.google.gerrit.client.changes.CommentInput;
import com.google.gerrit.client.diff.SideBySide2.DisplaySide;
import com.google.gerrit.client.rpc.GerritCallback; import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.ui.CommentLinkProcessor; import com.google.gerrit.client.ui.CommentLinkProcessor;
import com.google.gerrit.common.changes.Side;
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.JavaScriptObject; import com.google.gwt.core.client.JavaScriptObject;
@@ -84,9 +84,12 @@ class DraftBox extends CommentBox {
DraftBox( DraftBox(
SideBySide2 parent, SideBySide2 parent,
CodeMirror cm, CodeMirror cm,
DisplaySide side,
CommentLinkProcessor clp, CommentLinkProcessor clp,
PatchSet.Id id, PatchSet.Id id,
CommentInfo info) { CommentInfo info) {
super(side);
this.parent = parent; this.parent = parent;
this.cm = cm; this.cm = cm;
this.linkProcessor = clp; this.linkProcessor = clp;
@@ -215,15 +218,14 @@ class DraftBox extends CommentBox {
if (replyToBox != null) { if (replyToBox != null) {
replyToBox.unregisterReplyBox(); replyToBox.unregisterReplyBox();
} }
Side side = comment.side();
removeFromParent(); removeFromParent();
if (!getCommentInfo().has_line()) { if (!getCommentInfo().has_line()) {
parent.removeFileCommentBox(this, side); parent.removeFileCommentBox(this);
return; return;
} }
PaddingManager manager = getPaddingManager(); PaddingManager manager = getPaddingManager();
manager.remove(this); manager.remove(this);
parent.removeDraft(this, side, comment.line() - 1); parent.removeDraft(this, comment.line() - 1);
cm.focus(); cm.focus();
getSelfWidgetWrapper().getWidget().clear(); getSelfWidgetWrapper().getWidget().clear();
getGutterWrapper().remove(); getGutterWrapper().remove();

View File

@@ -16,7 +16,7 @@ package com.google.gerrit.client.diff;
import com.google.gerrit.client.Gerrit; import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.changes.CommentInfo; import com.google.gerrit.client.changes.CommentInfo;
import com.google.gerrit.common.changes.Side; import com.google.gerrit.client.diff.SideBySide2.DisplaySide;
import com.google.gwt.user.client.ui.Composite; import com.google.gwt.user.client.ui.Composite;
import com.google.gwt.user.client.ui.FlowPanel; import com.google.gwt.user.client.ui.FlowPanel;
@@ -33,11 +33,11 @@ class FileCommentPanel extends Composite {
private SideBySide2 parent; private SideBySide2 parent;
private DiffTable table; private DiffTable table;
private String path; private String path;
private Side side; private DisplaySide side;
private List<CommentBox> boxes; private List<CommentBox> boxes;
private FlowPanel body; private FlowPanel body;
FileCommentPanel(SideBySide2 host, DiffTable table, String path, Side side) { FileCommentPanel(SideBySide2 host, DiffTable table, String path, DisplaySide side) {
this.parent = host; this.parent = host;
this.table = table; this.table = table;
this.path = path; this.path = path;
@@ -54,10 +54,10 @@ class FileCommentPanel extends Composite {
if (boxes.isEmpty()) { if (boxes.isEmpty()) {
CommentInfo info = CommentInfo.createFile( CommentInfo info = CommentInfo.createFile(
path, path,
side, parent.getStoredSideFromDisplaySide(side),
null, null,
null); null);
addFileComment(parent.addDraftBox(info)); addFileComment(parent.addDraftBox(info, side));
} else { } else {
CommentBox box = boxes.get(boxes.size() - 1); CommentBox box = boxes.get(boxes.size() - 1);
if (box instanceof DraftBox) { if (box instanceof DraftBox) {

View File

@@ -14,7 +14,7 @@
package com.google.gerrit.client.diff; package com.google.gerrit.client.diff;
import com.google.gerrit.common.changes.Side; import com.google.gerrit.client.diff.SideBySide2.DisplaySide;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
@@ -98,8 +98,8 @@ class LineMapper {
* - | 6 * - | 6
* ... * ...
*/ */
LineOnOtherInfo lineOnOther(Side mySide, int line) { LineOnOtherInfo lineOnOther(DisplaySide mySide, int line) {
List<LineGap> lineGaps = mySide == Side.PARENT ? lineMapAtoB : lineMapBtoA; List<LineGap> lineGaps = mySide == DisplaySide.A ? lineMapAtoB : lineMapBtoA;
// Create a dummy LineGap for the search. // Create a dummy LineGap for the search.
int ret = Collections.binarySearch(lineGaps, new LineGap(line)); int ret = Collections.binarySearch(lineGaps, new LineGap(line));
if (ret == -1) { if (ret == -1) {

View File

@@ -15,8 +15,8 @@
package com.google.gerrit.client.diff; package com.google.gerrit.client.diff;
import com.google.gerrit.client.Gerrit; import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.diff.SideBySide2.DisplaySide;
import com.google.gerrit.client.patches.PatchUtil; import com.google.gerrit.client.patches.PatchUtil;
import com.google.gerrit.common.changes.Side;
import com.google.gwt.core.client.GWT; import com.google.gwt.core.client.GWT;
import com.google.gwt.event.dom.client.ClickEvent; import com.google.gwt.event.dom.client.ClickEvent;
import com.google.gwt.uibinder.client.UiBinder; import com.google.gwt.uibinder.client.UiBinder;
@@ -38,9 +38,9 @@ class PatchSelectBox2 extends Composite {
Image icon; Image icon;
private DiffTable table; private DiffTable table;
private Side side; private DisplaySide side;
PatchSelectBox2(DiffTable table, Side side) { PatchSelectBox2(DiffTable table, DisplaySide side) {
initWidget(uiBinder.createAndBindUi(this)); initWidget(uiBinder.createAndBindUi(this));
icon.setTitle(PatchUtil.C.addFileCommentToolTip()); icon.setTitle(PatchUtil.C.addFileCommentToolTip());
icon.addStyleName(Gerrit.RESOURCES.css().link()); icon.addStyleName(Gerrit.RESOURCES.css().link());

View File

@@ -21,6 +21,7 @@ import com.google.gerrit.client.changes.CommentApi;
import com.google.gerrit.client.changes.CommentInfo; import com.google.gerrit.client.changes.CommentInfo;
import com.google.gerrit.client.changes.CommentInput; import com.google.gerrit.client.changes.CommentInput;
import com.google.gerrit.client.changes.Util; import com.google.gerrit.client.changes.Util;
import com.google.gerrit.client.diff.SideBySide2.DisplaySide;
import com.google.gerrit.client.patches.PatchUtil; import com.google.gerrit.client.patches.PatchUtil;
import com.google.gerrit.client.rpc.GerritCallback; import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.ui.CommentLinkProcessor; import com.google.gerrit.client.ui.CommentLinkProcessor;
@@ -68,9 +69,12 @@ class PublishedBox extends CommentBox {
PublishedBox( PublishedBox(
SideBySide2 parent, SideBySide2 parent,
DisplaySide side,
CommentLinkProcessor clp, CommentLinkProcessor clp,
PatchSet.Id psId, PatchSet.Id psId,
CommentInfo info) { CommentInfo info) {
super(side);
this.parent = parent; this.parent = parent;
this.psId = psId; this.psId = psId;
this.comment = info; this.comment = info;
@@ -137,7 +141,7 @@ class PublishedBox extends CommentBox {
} }
DraftBox addReplyBox() { DraftBox addReplyBox() {
DraftBox box = parent.addDraftBox(parent.createReply(comment)); DraftBox box = parent.addDraftBox(parent.createReply(comment), getSide());
registerReplyBox(box); registerReplyBox(box);
return box; return box;
} }
@@ -148,7 +152,7 @@ class PublishedBox extends CommentBox {
} else if (replyBox == null) { } else if (replyBox == null) {
DraftBox box = addReplyBox(); DraftBox box = addReplyBox();
if (!getCommentInfo().has_line()) { if (!getCommentInfo().has_line()) {
parent.addFileCommentBox(box, comment.side()); parent.addFileCommentBox(box);
} }
} else { } else {
openReplyBox(); openReplyBox();
@@ -176,10 +180,10 @@ class PublishedBox extends CommentBox {
public void onSuccess(CommentInfo result) { public void onSuccess(CommentInfo result) {
done.setEnabled(true); done.setEnabled(true);
setOpen(false); setOpen(false);
DraftBox box = parent.addDraftBox(result); DraftBox box = parent.addDraftBox(result, getSide());
registerReplyBox(box); registerReplyBox(box);
if (!getCommentInfo().has_line()) { if (!getCommentInfo().has_line()) {
parent.addFileCommentBox(box, comment.side()); parent.addFileCommentBox(box);
} }
} }
}); });

View File

@@ -88,6 +88,10 @@ public class SideBySide2 extends Screen {
interface Binder extends UiBinder<FlowPanel, SideBySide2> {} interface Binder extends UiBinder<FlowPanel, SideBySide2> {}
private static Binder uiBinder = GWT.create(Binder.class); private static Binder uiBinder = GWT.create(Binder.class);
enum DisplaySide {
A, B;
}
private static final JsArrayString EMPTY = private static final JsArrayString EMPTY =
JavaScriptObject.createArray().cast(); JavaScriptObject.createArray().cast();
@@ -106,8 +110,10 @@ public class SideBySide2 extends Screen {
private Timer scrollTimerA; private Timer scrollTimerA;
private Timer scrollTimerB; private Timer scrollTimerB;
private HandlerRegistration resizeHandler; private HandlerRegistration resizeHandler;
private JsArray<CommentInfo> published; private JsArray<CommentInfo> publishedBase;
private JsArray<CommentInfo> drafts; private JsArray<CommentInfo> publishedRevision;
private JsArray<CommentInfo> draftsBase;
private JsArray<CommentInfo> draftsRevision;
private DiffInfo diff; private DiffInfo diff;
private LineMapper mapper; private LineMapper mapper;
private CommentLinkProcessor commentLinkProcessor; private CommentLinkProcessor commentLinkProcessor;
@@ -174,16 +180,14 @@ public class SideBySide2 extends Screen {
} }
})); }));
if (base != null) { if (base != null) {
CommentApi.comments(base, group.add(getCommentCallback(false))); CommentApi.comments(base, group.add(getCommentCallback(DisplaySide.A, false)));
} }
CommentApi.comments(revision, group.add(getCommentCallback(false))); CommentApi.comments(revision, group.add(getCommentCallback(DisplaySide.B, false)));
if (Gerrit.isSignedIn()) { if (Gerrit.isSignedIn()) {
if (base != null) { if (base != null) {
CommentApi.drafts(base, group.add(getCommentCallback(true))); CommentApi.drafts(base, group.add(getCommentCallback(DisplaySide.A, true)));
} }
CommentApi.drafts(revision, group.add(getCommentCallback(true))); CommentApi.drafts(revision, group.add(getCommentCallback(DisplaySide.B, true)));
} else {
drafts = JsArray.createArray().cast();
} }
ConfigInfoCache.get(revision.getParentKey(), group.addFinal( ConfigInfoCache.get(revision.getParentKey(), group.addFinal(
new ScreenLoadCallback<ConfigInfoCache.Entry>(SideBySide2.this) { new ScreenLoadCallback<ConfigInfoCache.Entry>(SideBySide2.this) {
@@ -312,31 +316,30 @@ public class SideBySide2 extends Screen {
} }
private GerritCallback<NativeMap<JsArray<CommentInfo>>> getCommentCallback( private GerritCallback<NativeMap<JsArray<CommentInfo>>> getCommentCallback(
final boolean toDrafts) { final DisplaySide side, final boolean toDrafts) {
return new GerritCallback<NativeMap<JsArray<CommentInfo>>>() { return new GerritCallback<NativeMap<JsArray<CommentInfo>>>() {
@Override @Override
public void onSuccess(NativeMap<JsArray<CommentInfo>> result) { public void onSuccess(NativeMap<JsArray<CommentInfo>> result) {
JsArray<CommentInfo> in = result.get(path); JsArray<CommentInfo> in = result.get(path);
if (in != null) { if (in != null) {
addAllToCommentList(in, toDrafts); if (toDrafts) {
if (side == DisplaySide.A) {
draftsBase = in;
} else {
draftsRevision = in;
}
} else {
if (side == DisplaySide.A) {
publishedBase = in;
} else {
publishedRevision = in;
}
}
} }
} }
}; };
} }
private void addAllToCommentList(JsArray<CommentInfo> in, boolean toDrafts) {
if (toDrafts && drafts == null) {
drafts = in;
} else if (!toDrafts && published == null) {
published = in;
} else {
JsArray<CommentInfo> dest = toDrafts ? drafts : published;
for (int i = 0; i < in.length(); i++) {
dest.push(in.get(i));
}
}
}
private void display(DiffInfo diffInfo) { private void display(DiffInfo diffInfo) {
cmA = displaySide(diffInfo.meta_a(), diffInfo.text_a(), diffTable.cmA); cmA = displaySide(diffInfo.meta_a(), diffInfo.text_a(), diffTable.cmA);
cmB = displaySide(diffInfo.meta_b(), diffInfo.text_b(), diffTable.cmB); cmB = displaySide(diffInfo.meta_b(), diffInfo.text_b(), diffTable.cmB);
@@ -347,17 +350,22 @@ public class SideBySide2 extends Screen {
lineActiveBoxMap = new HashMap<LineHandle, CommentBox>(); lineActiveBoxMap = new HashMap<LineHandle, CommentBox>();
lineLastPublishedBoxMap = new HashMap<LineHandle, PublishedBox>(); lineLastPublishedBoxMap = new HashMap<LineHandle, PublishedBox>();
linePaddingManagerMap = new HashMap<LineHandle, PaddingManager>(); linePaddingManagerMap = new HashMap<LineHandle, PaddingManager>();
if (published != null) { if (publishedBase != null || publishedRevision != null) {
publishedMap = new HashMap<String, PublishedBox>(published.length()); publishedMap = new HashMap<String, PublishedBox>();
renderPublished();
} }
if (drafts != null) { if (publishedBase != null) {
renderDrafts(); renderPublished(publishedBase);
}
if (publishedRevision != null) {
renderPublished(publishedRevision);
}
if (draftsBase != null) {
renderDrafts(draftsBase);
}
if (draftsRevision != null) {
renderDrafts(draftsRevision);
} }
renderSkips(); renderSkips();
published = null;
drafts = null;
skips = null;
registerCmEvents(cmA); registerCmEvents(cmA);
registerCmEvents(cmB); registerCmEvents(cmB);
resizeHandler = Window.addResizeHandler(new ResizeHandler() { resizeHandler = Window.addResizeHandler(new ResizeHandler() {
@@ -464,11 +472,11 @@ public class SideBySide2 extends Screen {
private DraftBox addNewDraft(CodeMirror cm, int line) { private DraftBox addNewDraft(CodeMirror cm, int line) {
return addDraftBox(CommentInfo.createRange( return addDraftBox(CommentInfo.createRange(
path, path,
getSideFromCm(cm), getStoredSideFromDisplaySide(getSideFromCm(cm)),
line + 1, line + 1,
null, null,
null, null,
null)); null), getSideFromCm(cm));
} }
CommentInfo createReply(CommentInfo replyTo) { CommentInfo createReply(CommentInfo replyTo) {
@@ -480,10 +488,9 @@ public class SideBySide2 extends Screen {
} }
} }
DraftBox addDraftBox(CommentInfo info) { DraftBox addDraftBox(CommentInfo info, DisplaySide side) {
Side side = info.side();
CodeMirror cm = getCmFromSide(side); CodeMirror cm = getCmFromSide(side);
final DraftBox box = new DraftBox(this, cm, commentLinkProcessor, final DraftBox box = new DraftBox(this, cm, side, commentLinkProcessor,
getPatchSetIdFromSide(side), info); getPatchSetIdFromSide(side), info);
if (info.id() == null) { if (info.id() == null) {
Scheduler.get().scheduleDeferred(new ScheduledCommand() { Scheduler.get().scheduleDeferred(new ScheduledCommand() {
@@ -505,8 +512,8 @@ public class SideBySide2 extends Screen {
CommentBox addCommentBox(CommentInfo info, CommentBox box) { CommentBox addCommentBox(CommentInfo info, CommentBox box) {
diffTable.add(box); diffTable.add(box);
Side mySide = info.side(); DisplaySide side = box.getSide();
CodeMirror cm = mySide == Side.PARENT ? cmA : cmB; CodeMirror cm = getCmFromSide(side);
CodeMirror other = otherCm(cm); CodeMirror other = otherCm(cm);
int line = info.line() - 1; // CommentInfo is 1-based, but CM is 0-based int line = info.line() - 1; // CommentInfo is 1-based, but CM is 0-based
LineHandle handle = cm.getLineHandle(line); LineHandle handle = cm.getLineHandle(line);
@@ -519,9 +526,9 @@ public class SideBySide2 extends Screen {
addPaddingWidget(cm, DiffTable.style.padding(), line, 0, Unit.PX, 0)); addPaddingWidget(cm, DiffTable.style.padding(), line, 0, Unit.PX, 0));
linePaddingManagerMap.put(handle, manager); linePaddingManagerMap.put(handle, manager);
} }
int lineToPad = mapper.lineOnOther(mySide, line).getLine(); int lineToPad = mapper.lineOnOther(side, line).getLine();
LineHandle otherHandle = other.getLineHandle(lineToPad); LineHandle otherHandle = other.getLineHandle(lineToPad);
DiffChunkInfo myChunk = getDiffChunk(mySide, line); DiffChunkInfo myChunk = getDiffChunk(side, line);
DiffChunkInfo otherChunk = getDiffChunk(getSideFromCm(other), lineToPad); DiffChunkInfo otherChunk = getDiffChunk(getSideFromCm(other), lineToPad);
PaddingManager otherManager; PaddingManager otherManager;
if (linePaddingManagerMap.containsKey(otherHandle)) { if (linePaddingManagerMap.containsKey(otherHandle)) {
@@ -553,20 +560,20 @@ public class SideBySide2 extends Screen {
return box; return box;
} }
void removeDraft(DraftBox box, Side side, int line) { void removeDraft(DraftBox box, int line) {
LineHandle handle = getCmFromSide(side).getLineHandle(line); LineHandle handle = getCmFromSide(box.getSide()).getLineHandle(line);
lineActiveBoxMap.remove(handle); lineActiveBoxMap.remove(handle);
if (lineLastPublishedBoxMap.containsKey(handle)) { if (lineLastPublishedBoxMap.containsKey(handle)) {
lineActiveBoxMap.put(handle, lineLastPublishedBoxMap.get(handle)); lineActiveBoxMap.put(handle, lineLastPublishedBoxMap.get(handle));
} }
} }
void addFileCommentBox(CommentBox box, Side side) { void addFileCommentBox(CommentBox box) {
diffTable.addFileCommentBox(box, side); diffTable.addFileCommentBox(box);
} }
void removeFileCommentBox(DraftBox box, Side side) { void removeFileCommentBox(DraftBox box) {
diffTable.onRemoveDraftBox(box, side); diffTable.onRemoveDraftBox(box);
} }
private List<CommentInfo> sortComment(JsArray<CommentInfo> unsorted) { private List<CommentInfo> sortComment(JsArray<CommentInfo> unsorted) {
@@ -583,16 +590,24 @@ public class SideBySide2 extends Screen {
return sorted; return sorted;
} }
private void renderPublished() { private void renderPublished(JsArray<CommentInfo> published) {
List<CommentInfo> sorted = sortComment(published); List<CommentInfo> sorted = sortComment(published);
for (CommentInfo info : sorted) { for (CommentInfo info : sorted) {
Side side = info.side(); DisplaySide side;
if (info.side() == Side.PARENT) {
if (base != null) {
continue;
}
side = DisplaySide.A;
} else {
side = published == publishedBase ? DisplaySide.A : DisplaySide.B;
}
CodeMirror cm = getCmFromSide(side); CodeMirror cm = getCmFromSide(side);
PublishedBox box = new PublishedBox(this, commentLinkProcessor, PublishedBox box = new PublishedBox(this, side, commentLinkProcessor,
getPatchSetIdFromSide(side), info); getPatchSetIdFromSide(side), info);
publishedMap.put(info.id(), box); publishedMap.put(info.id(), box);
if (!info.has_line()) { if (!info.has_line()) {
diffTable.addFileCommentBox(box, side); diffTable.addFileCommentBox(box);
continue; continue;
} }
int line = info.line() - 1; int line = info.line() - 1;
@@ -603,21 +618,29 @@ public class SideBySide2 extends Screen {
} }
} }
private void renderDrafts() { private void renderDrafts(JsArray<CommentInfo> drafts) {
List<CommentInfo> sorted = sortComment(drafts); List<CommentInfo> sorted = sortComment(drafts);
for (CommentInfo info : sorted) { for (CommentInfo info : sorted) {
Side side = info.side(); DisplaySide side;
if (info.side() == Side.PARENT) {
if (base != null) {
continue;
}
side = DisplaySide.A;
} else {
side = drafts == draftsBase ? DisplaySide.A : DisplaySide.B;
}
DraftBox box = new DraftBox( DraftBox box = new DraftBox(
this, getCmFromSide(side), commentLinkProcessor, this, getCmFromSide(side), side, commentLinkProcessor,
getPatchSetIdFromSide(side), info); getPatchSetIdFromSide(side), info);
if (published != null) { if (publishedBase != null || publishedRevision != null) {
PublishedBox replyToBox = publishedMap.get(info.in_reply_to()); PublishedBox replyToBox = publishedMap.get(info.in_reply_to());
if (replyToBox != null) { if (replyToBox != null) {
replyToBox.registerReplyBox(box); replyToBox.registerReplyBox(box);
} }
} }
if (!info.has_line()) { if (!info.has_line()) {
diffTable.addFileCommentBox(box, side); diffTable.addFileCommentBox(box);
continue; continue;
} }
lineActiveBoxMap.put( lineActiveBoxMap.put(
@@ -714,16 +737,20 @@ public class SideBySide2 extends Screen {
return me == cmA ? cmB : cmA; return me == cmA ? cmB : cmA;
} }
private PatchSet.Id getPatchSetIdFromSide(Side side) { private PatchSet.Id getPatchSetIdFromSide(DisplaySide side) {
return side == Side.PARENT && base != null ? base : revision; return side == DisplaySide.A && base != null ? base : revision;
} }
private CodeMirror getCmFromSide(Side side) { private CodeMirror getCmFromSide(DisplaySide side) {
return side == Side.PARENT ? cmA : cmB; return side == DisplaySide.A ? cmA : cmB;
} }
private Side getSideFromCm(CodeMirror cm) { private DisplaySide getSideFromCm(CodeMirror cm) {
return cm == cmA ? Side.PARENT : Side.REVISION; return cm == cmA ? DisplaySide.A : DisplaySide.B;
}
Side getStoredSideFromDisplaySide(DisplaySide side) {
return side == DisplaySide.A && base == null ? Side.PARENT : Side.REVISION;
} }
private void markEdit(CodeMirror cm, JsArrayString lines, private void markEdit(CodeMirror cm, JsArrayString lines,
@@ -974,7 +1001,7 @@ public class SideBySide2 extends Screen {
}; };
} }
private DiffChunkInfo getDiffChunk(Side side, int line) { private DiffChunkInfo getDiffChunk(DisplaySide side, int line) {
for (DiffChunkInfo info : diffChunks) { for (DiffChunkInfo info : diffChunks) {
if (info.getSide() == side && info.getStart() <= line && if (info.getSide() == side && info.getStart() <= line &&
line <= info.getEnd()) { line <= info.getEnd()) {
@@ -984,7 +1011,7 @@ public class SideBySide2 extends Screen {
return null; return null;
} }
void resizePaddingOnOtherSide(Side mySide, int line) { void resizePaddingOnOtherSide(DisplaySide mySide, int line) {
CodeMirror cm = getCmFromSide(mySide); CodeMirror cm = getCmFromSide(mySide);
LineHandle handle = cm.getLineHandle(line); LineHandle handle = cm.getLineHandle(line);
final LinePaddingWidgetWrapper otherWrapper = linePaddingOnOtherSideMap.get(handle); final LinePaddingWidgetWrapper otherWrapper = linePaddingOnOtherSideMap.get(handle);
@@ -1030,7 +1057,7 @@ public class SideBySide2 extends Screen {
} }
// TODO: Maybe integrate this with PaddingManager. // TODO: Maybe integrate this with PaddingManager.
private RenderLineHandler resizeLinePadding(final Side side) { private RenderLineHandler resizeLinePadding(final DisplaySide side) {
return new RenderLineHandler() { return new RenderLineHandler() {
@Override @Override
public void handle(final CodeMirror instance, final LineHandle handle, public void handle(final CodeMirror instance, final LineHandle handle,
@@ -1121,17 +1148,17 @@ public class SideBySide2 extends Screen {
} }
static class DiffChunkInfo { static class DiffChunkInfo {
private Side side; private DisplaySide side;
private int start; private int start;
private int end; private int end;
DiffChunkInfo(Side side, int start, int end) { DiffChunkInfo(DisplaySide side, int start, int end) {
this.side = side; this.side = side;
this.start = start; this.start = start;
this.end = end; this.end = end;
} }
Side getSide() { DisplaySide getSide() {
return side; return side;
} }

View File

@@ -17,7 +17,7 @@ package com.google.gerrit.client.diff;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import com.google.gerrit.client.diff.LineMapper.LineOnOtherInfo; import com.google.gerrit.client.diff.LineMapper.LineOnOtherInfo;
import com.google.gerrit.common.changes.Side; import com.google.gerrit.client.diff.SideBySide2.DisplaySide;
import org.junit.Test; import org.junit.Test;
@@ -53,9 +53,9 @@ public class LineMapperTest {
LineMapper mapper = new LineMapper(); LineMapper mapper = new LineMapper();
mapper.appendCommon(10); mapper.appendCommon(10);
assertEquals(new LineOnOtherInfo(9, true), assertEquals(new LineOnOtherInfo(9, true),
mapper.lineOnOther(Side.PARENT, 9)); mapper.lineOnOther(DisplaySide.A, 9));
assertEquals(new LineOnOtherInfo(9, true), assertEquals(new LineOnOtherInfo(9, true),
mapper.lineOnOther(Side.REVISION, 9)); mapper.lineOnOther(DisplaySide.B, 9));
} }
@Test @Test
@@ -63,9 +63,9 @@ public class LineMapperTest {
LineMapper mapper = new LineMapper(); LineMapper mapper = new LineMapper();
mapper.appendCommon(10); mapper.appendCommon(10);
assertEquals(new LineOnOtherInfo(10, true), assertEquals(new LineOnOtherInfo(10, true),
mapper.lineOnOther(Side.PARENT, 10)); mapper.lineOnOther(DisplaySide.A, 10));
assertEquals(new LineOnOtherInfo(10, true), assertEquals(new LineOnOtherInfo(10, true),
mapper.lineOnOther(Side.REVISION, 10)); mapper.lineOnOther(DisplaySide.B, 10));
} }
@Test @Test
@@ -73,7 +73,7 @@ public class LineMapperTest {
LineMapper mapper = new LineMapper(); LineMapper mapper = new LineMapper();
mapper.appendInsert(10); mapper.appendInsert(10);
assertEquals(new LineOnOtherInfo(-1, false), assertEquals(new LineOnOtherInfo(-1, false),
mapper.lineOnOther(Side.REVISION, 9)); mapper.lineOnOther(DisplaySide.B, 9));
} }
@Test @Test
@@ -81,9 +81,9 @@ public class LineMapperTest {
LineMapper mapper = new LineMapper(); LineMapper mapper = new LineMapper();
mapper.appendInsert(10); mapper.appendInsert(10);
assertEquals(new LineOnOtherInfo(0, true), assertEquals(new LineOnOtherInfo(0, true),
mapper.lineOnOther(Side.REVISION, 10)); mapper.lineOnOther(DisplaySide.B, 10));
assertEquals(new LineOnOtherInfo(10, true), assertEquals(new LineOnOtherInfo(10, true),
mapper.lineOnOther(Side.PARENT, 0)); mapper.lineOnOther(DisplaySide.A, 0));
} }
@Test @Test
@@ -91,7 +91,7 @@ public class LineMapperTest {
LineMapper mapper = new LineMapper(); LineMapper mapper = new LineMapper();
mapper.appendDelete(10); mapper.appendDelete(10);
assertEquals(new LineOnOtherInfo(-1, false), assertEquals(new LineOnOtherInfo(-1, false),
mapper.lineOnOther(Side.PARENT, 9)); mapper.lineOnOther(DisplaySide.A, 9));
} }
@Test @Test
@@ -99,8 +99,8 @@ public class LineMapperTest {
LineMapper mapper = new LineMapper(); LineMapper mapper = new LineMapper();
mapper.appendDelete(10); mapper.appendDelete(10);
assertEquals(new LineOnOtherInfo(0, true), assertEquals(new LineOnOtherInfo(0, true),
mapper.lineOnOther(Side.PARENT, 10)); mapper.lineOnOther(DisplaySide.A, 10));
assertEquals(new LineOnOtherInfo(10, true), assertEquals(new LineOnOtherInfo(10, true),
mapper.lineOnOther(Side.REVISION, 0)); mapper.lineOnOther(DisplaySide.B, 0));
} }
} }