opendev/gerrit
Code Issues Proposed changes

Merge "SideBySide2: Rewrite padding mechanism to be based on diff chunks"

Browse Source
This commit is contained in:
Shawn Pearce
2013-07-24 14:00:27 +00:00
committed by Gerrit Code Review
parent c6e8ab59f7 dbeedb11bf
commit 89c3c582b6
6 changed files with 158 additions and 129 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentBox.java
View File

@@ -15,6 +15,7 @@
package com.google.gerrit.client.diff;
import com.google.gerrit.client.changes.CommentInfo;
import com.google.gerrit.client.diff.SideBySide2.DiffChunkInfo;
import com.google.gwt.core.client.Scheduler;
import com.google.gwt.core.client.Scheduler.ScheduledCommand;
import com.google.gwt.user.client.ui.Composite;
@@ -29,14 +30,22 @@ abstract class CommentBox extends Composite {
private PaddingManager widgetManager;
private LineWidget selfWidget;
private SideBySide2 parent;
private DiffChunkInfo diffChunkInfo;
void resizePaddingWidget() {
Scheduler.get().scheduleDeferred(new ScheduledCommand(){
selfWidget.changed();
Scheduler.get().scheduleDeferred(new ScheduledCommand() {
@Override
public void execute() {
assert selfWidget != null;
assert widgetManager != null;
selfWidget.changed();
widgetManager.resizePaddingWidget();
if (diffChunkInfo != null) {
parent.resizePaddingOnOtherSide(getCommentInfo().side(),
diffChunkInfo.getEnd());
} else {
assert selfWidget != null;
assert widgetManager != null;
widgetManager.resizePaddingWidget();
}
}
});
}
@@ -52,6 +61,14 @@ abstract class CommentBox extends Composite {
return widgetManager;
}
void setDiffChunkInfo(DiffChunkInfo info) {
this.diffChunkInfo = info;
}
void setParent(SideBySide2 parent) {
this.parent = parent;
}
void setPaddingManager(PaddingManager manager) {
widgetManager = manager;
}

7
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DraftBox.java
View File

@@ -25,6 +25,7 @@ import com.google.gwt.core.client.GWT;
import com.google.gwt.core.client.JavaScriptObject;
import com.google.gwt.core.client.Scheduler;
import com.google.gwt.core.client.Scheduler.RepeatingCommand;
import com.google.gwt.core.client.Scheduler.ScheduledCommand;
import com.google.gwt.dom.client.Element;
import com.google.gwt.event.dom.client.ClickEvent;
import com.google.gwt.event.dom.client.ClickHandler;
@@ -216,6 +217,12 @@ class DraftBox extends CommentBox {
parent.removeDraft(this, comment.side(), comment.line() - 1);
removeFromParent();
Scheduler.get().scheduleDeferred(new ScheduledCommand() {
@Override
public void execute() {
resizePaddingWidget();
}
});
getSelfWidget().clear();
PaddingManager manager = getPaddingManager();

71
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PaddingManager.java
View File

@@ -27,31 +27,22 @@ import java.util.List;
* changes as necessary. PaddingManager calculates padding by taking the
* difference of the sum of CommentBox heights on the two sides.
*
* Note that in the case of an insertion or deletion gap, A PaddingManager
* can map to a list of managers on the other side. The padding needed is then
* calculated from the sum of all their heights.
*
* TODO: Let PaddingManager also take care of the paddings introduced by
* insertions and deletions.
*/
class PaddingManager {
private List<CommentBox> comments;
private PaddingWidgetWrapper wrapper;
private List<PaddingManager> others;
private PaddingManager other;
PaddingManager(PaddingWidgetWrapper padding) {
comments = new ArrayList<CommentBox>();
others = new ArrayList<PaddingManager>();
this.wrapper = padding;
}
static void link(PaddingManager a, PaddingManager b) {
if (!a.others.contains(b)) {
a.others.add(b);
}
if (!b.others.contains(a)) {
b.others.add(a);
}
a.other = b;
b.other = a;
}
private int getMyTotalHeight() {
@@ -62,49 +53,22 @@ class PaddingManager {
return total;
}
/**
* If this instance is on the insertion side, its counterpart on the other
* side will map to a group of PaddingManagers on this side, so we calculate
* the group's total height instead of an individual one's.
*/
private int getGroupTotalHeight() {
if (others.size() > 1) {
return getMyTotalHeight();
} else {
return others.get(0).getOthersTotalHeight();
}
}
private int getOthersTotalHeight() {
int total = 0;
for (PaddingManager manager : others) {
total += manager.getMyTotalHeight();
}
return total;
}
private void setPaddingHeight(int height) {
SideBySide2.setHeightInPx(wrapper.element, height);
wrapper.widget.changed();
}
void resizePaddingWidget() {
if (others.isEmpty()) {
throw new IllegalStateException("resizePaddingWidget() called before linking");
}
int myHeight = getGroupTotalHeight();
int othersHeight = getOthersTotalHeight();
assert other != null;
int myHeight = getMyTotalHeight();
int othersHeight = other.getMyTotalHeight();
int paddingNeeded = othersHeight - myHeight;
if (paddingNeeded < 0) {
for (PaddingManager manager : others.get(0).others) {
manager.setPaddingHeight(0);
}
others.get(others.size() - 1).setPaddingHeight(-paddingNeeded);
setPaddingHeight(0);
other.setPaddingHeight(-paddingNeeded);
} else {
setPaddingHeight(paddingNeeded);
for (PaddingManager other : others) {
other.setPaddingHeight(0);
}
other.setPaddingHeight(0);
}
}
@@ -144,15 +108,22 @@ class PaddingManager {
}
static class LinePaddingWidgetWrapper extends PaddingWidgetWrapper {
private boolean common;
private int chunkLength;
private int otherLine;
LinePaddingWidgetWrapper(PaddingWidgetWrapper pair, boolean common) {
LinePaddingWidgetWrapper(PaddingWidgetWrapper pair, int otherLine, int chunkLength) {
super(pair.widget, pair.element);
this.common = common;
this.otherLine = otherLine;
this.chunkLength = chunkLength;
}
boolean isCommon() {
return common;
int getChunkLength() {
return chunkLength;
}
int getOtherLine() {
return otherLine;
}
}
}

156
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java
View File

@@ -111,8 +111,8 @@ public class SideBySide2 extends Screen {
private Map<LineHandle, CommentBox> lineActiveBoxMap;
private Map<LineHandle, PublishedBox> lineLastPublishedBoxMap;
private Map<LineHandle, PaddingManager> linePaddingManagerMap;
private Map<LineHandle, LinePaddingWidgetWrapper> linePaddingWidgetMap;
private Map<LineHandle, Element> lineElementMap;
private Map<LineHandle, LinePaddingWidgetWrapper> linePaddingOnOtherSideMap;
private List<DiffChunkInfo> diffChunks;
private List<SkippedLine> skips;
private int context;
@@ -337,8 +337,8 @@ public class SideBySide2 extends Screen {
cmA = displaySide(diffInfo.meta_a(), diffInfo.text_a(), diffTable.cmA);
cmB = displaySide(diffInfo.meta_b(), diffInfo.text_b(), diffTable.cmB);
skips = new ArrayList<SkippedLine>();
linePaddingWidgetMap = new HashMap<LineHandle, LinePaddingWidgetWrapper>();
lineElementMap = new HashMap<LineHandle, Element>();
linePaddingOnOtherSideMap = new HashMap<LineHandle, LinePaddingWidgetWrapper>();
diffChunks = new ArrayList<DiffChunkInfo>();
render(diffInfo);
allBoxes = new ArrayList<CommentBox>();
lineActiveBoxMap = new HashMap<LineHandle, CommentBox>();
@@ -433,17 +433,19 @@ public class SideBySide2 extends Screen {
mapper.appendCommon(commonCnt);
if (aLength < bLength) { // Edit with insertion
int insertCnt = bLength - aLength;
insertEmptyLines(cmA, mapper.getLineA(), mapper.getLineB(),
insertCnt, false);
mapper.appendInsert(insertCnt);
} else if (aLength > bLength) { // Edit with deletion
int deleteCnt = aLength - bLength;
insertEmptyLines(cmB, mapper.getLineB(), mapper.getLineA(),
deleteCnt, false);
mapper.appendDelete(deleteCnt);
}
insertEmptyLines(cmA, mapper.getLineA(), origLineB, commonCnt, true);
insertEmptyLines(cmB, mapper.getLineB(), origLineA, commonCnt, true);
int chunkEndA = mapper.getLineA() - 1;
int chunkEndB = mapper.getLineB() - 1;
if (bLength > 0) {
addDiffChunkAndPadding(cmA, chunkEndA, chunkEndB, bLength);
}
if (aLength > 0) {
addDiffChunkAndPadding(cmB, chunkEndB, chunkEndA, aLength);
}
markEdit(cmA, currentA, current.edit_a(), origLineA);
markEdit(cmB, currentB, current.edit_b(), origLineB);
}
@@ -494,7 +496,7 @@ public class SideBySide2 extends Screen {
} else {
// Estimated height at 28px, fixed by deferring after display
manager = new PaddingManager(
addPaddingWidget(cm, DiffTable.style.padding(), line, 28, Unit.PX, 0));
addPaddingWidget(cm, DiffTable.style.padding(), line, 0, Unit.PX, 0));
linePaddingManagerMap.put(handle, manager);
}
int lineToPad = mapper.lineOnOther(mySide, line).getLine();
@@ -503,7 +505,7 @@ public class SideBySide2 extends Screen {
PaddingManager.link(manager, linePaddingManagerMap.get(otherHandle));
} else {
PaddingManager otherManager = new PaddingManager(
addPaddingWidget(other, DiffTable.style.padding(), lineToPad, 28, Unit.PX, 0));
addPaddingWidget(other, DiffTable.style.padding(), lineToPad, 0, Unit.PX, 0));
linePaddingManagerMap.put(otherHandle, otherManager);
PaddingManager.link(manager, otherManager);
}
@@ -515,6 +517,8 @@ public class SideBySide2 extends Screen {
LineWidget boxWidget = cm.addLineWidget(line, box.getElement(), config);
box.setPaddingManager(manager);
box.setSelfWidget(boxWidget);
box.setParent(this);
box.setDiffChunkInfo(getDiffChunk(mySide, line));
allBoxes.add(box);
return box;
}
@@ -711,21 +715,14 @@ public class SideBySide2 extends Screen {
}
}
private void insertEmptyLines(CodeMirror cm, int nextLine, int lineOnOther,
int cnt, boolean common) {
for (int i = 0; i < cnt; i++, lineOnOther++) {
LineHandle handle = otherCm(cm).getLineHandle(lineOnOther);
/**
* Link a line with its padding widget on the other side. Note that we
* also add a collapsed padding for common lines, because they may
* linewrap differently due to intraline edits, in which case we detect
* the difference in line height and resize the padding widget.
*/
linePaddingWidgetMap.put(handle, new LinePaddingWidgetWrapper(
// -1 to compensate for the line we went past when this method is called.
addPaddingWidget(cm, DiffTable.style.padding(), nextLine - 1,
common ? 0 : 1.1, Unit.EM, null), common));
}
private void addDiffChunkAndPadding(CodeMirror cmToPad, int lineToPad,
int lineOnOther, int chunkSize) {
CodeMirror otherCm = otherCm(cmToPad);
linePaddingOnOtherSideMap.put(otherCm.getLineHandle(lineOnOther),
new LinePaddingWidgetWrapper(addPaddingWidget(cmToPad, DiffTable.style.padding(),
lineToPad, 0, Unit.EM, null), lineToPad, chunkSize));
diffChunks.add(new DiffChunkInfo(getSideFromCm(otherCm),
lineOnOther - chunkSize + 1, lineOnOther));
}
private PaddingWidgetWrapper addPaddingWidget(CodeMirror cm, String style,
@@ -875,50 +872,59 @@ public class SideBySide2 extends Screen {
};
}
private DiffChunkInfo getDiffChunk(Side side, int line) {
for (DiffChunkInfo info : diffChunks) {
if (info.getSide() == side && info.getStart() <= line &&
line <= info.getEnd()) {
return info;
}
}
return null;
}
void resizePaddingOnOtherSide(Side mySide, int line) {
CodeMirror cm = getCmFromSide(mySide);
LineHandle handle = cm.getLineHandle(line);
final LinePaddingWidgetWrapper otherWrapper = linePaddingOnOtherSideMap.get(handle);
double myChunkHeight = cm.heightAtLine(line + 1) -
cm.heightAtLine(line - otherWrapper.getChunkLength() + 1);
Element otherPadding = otherWrapper.getElement();
CodeMirror otherCm = otherCm(cm);
int otherLine = otherWrapper.getOtherLine();
LineHandle other = otherCm.getLineHandle(otherLine);
if (linePaddingOnOtherSideMap.containsKey(other)) {
LinePaddingWidgetWrapper myWrapper = linePaddingOnOtherSideMap.get(other);
Element myPadding = linePaddingOnOtherSideMap.get(other).getElement();
myChunkHeight -= myPadding.getOffsetHeight();
double otherChunkHeight = otherCm.heightAtLine(otherLine + 1) -
otherCm.heightAtLine(otherLine - myWrapper.getChunkLength() + 1) -
otherPadding.getOffsetHeight();
double delta = myChunkHeight - otherChunkHeight;
if (delta > 0) {
setHeightInPx(myPadding, 0);
setHeightInPx(otherPadding, delta);
} else {
setHeightInPx(myPadding, -delta);
setHeightInPx(otherPadding, 0);
}
myWrapper.getWidget().changed();
} else {
setHeightInPx(otherPadding, myChunkHeight);
}
otherWrapper.getWidget().changed();
}
// TODO: Maybe integrate this with PaddingManager.
private RenderLineHandler resizeEmptyLine(final Side side) {
return new RenderLineHandler() {
@Override
public void handle(final CodeMirror instance, final LineHandle handle,
final Element element) {
if (linePaddingWidgetMap.containsKey(handle)) {
/**
* This needs to be deferred because CM fires "renderLine" **before**
* the line is actually added to the DOM.
*/
Element element) {
if (linePaddingOnOtherSideMap.containsKey(handle)) {
Scheduler.get().scheduleDeferred(new ScheduledCommand() {
@Override
public void execute() {
LinePaddingWidgetWrapper wrapper = linePaddingWidgetMap.get(handle);
int myLineHeight = element.getOffsetHeight();
Element otherPadding = wrapper.getElement();
if (!wrapper.isCommon()) {
setHeightInPx(otherPadding, myLineHeight);
} else {
/**
* We have to pay the cost of keeping track of the actual DOM
* elements ourselves, because CM doesn't provide an interface
* to query for them, and the only place we can ever have legit
* access to the line element is in this event handler.
*/
lineElementMap.put(handle, element);
// The lines are always aligned since they are in a common region.
int otherLine = mapper.lineOnOther(side,
instance.getLineNumber(handle)).getLine();
LineHandle other = otherCm(instance).getLineHandle(otherLine);
if (lineElementMap.containsKey(other)) {
Element otherElement = lineElementMap.get(other);
Element myPadding = linePaddingWidgetMap.get(other).getElement();
int delta = myLineHeight - otherElement.getOffsetHeight();
if (delta >= 0) {
setHeightInPx(otherPadding, delta);
setHeightInPx(myPadding, 0);
} else {
setHeightInPx(otherPadding, 0);
setHeightInPx(myPadding, -delta);
}
}
}
resizePaddingOnOtherSide(side, instance.getLineNumber(handle));
}
});
}
@@ -986,6 +992,30 @@ public class SideBySide2 extends Screen {
}
}
static class DiffChunkInfo {
private Side side;
private int start;
private int end;
DiffChunkInfo(Side side, int start, int end) {
this.side = side;
this.start = start;
this.end = end;
}
Side getSide() {
return side;
}
int getStart() {
return start;
}
int getEnd() {
return end;
}
}
private static class NoOpKeyCommand extends KeyCommand {
private NoOpKeyCommand(int mask, int key, String help) {
super(mask, key, help);

14
gerrit-gwtui/src/main/java/net/codemirror/lib/CodeMirror.java vendored
View File

@@ -52,9 +52,9 @@ public class CodeMirror extends JavaScriptObject {
public final native void setValue(String v) /*-{ this.setValue(v); }-*/;
public final native void setWidth(int w) /*-{ this.setSize(w, null); }-*/;
public final native void setWidth(double w) /*-{ this.setSize(w, null); }-*/;
public final native void setWidth(String w) /*-{ this.setSize(w, null); }-*/;
public final native void setHeight(int h) /*-{ this.setSize(null, h); }-*/;
public final native void setHeight(double h) /*-{ this.setSize(null, h); }-*/;
public final native void setHeight(String h) /*-{ this.setSize(null, h); }-*/;
public final native void refresh() /*-{ this.refresh(); }-*/;
@@ -120,19 +120,23 @@ public class CodeMirror extends JavaScriptObject {
return this.addLineWidget(line, node, options);
}-*/;
public final native int lineAtHeight(int height) /*-{
public final native int lineAtHeight(double height) /*-{
return this.lineAtHeight(height);
}-*/;
public final native double heightAtLine(int line) /*-{
return this.heightAtLine(line);
}-*/;
public final native CodeMirrorDoc getDoc() /*-{
return this.getDoc();
}-*/;
public final native void scrollTo(int x, int y) /*-{
public final native void scrollTo(double x, double y) /*-{
this.scrollTo(x, y);
}-*/;
public final native void scrollToY(int y) /*-{
public final native void scrollToY(double y) /*-{
this.scrollTo(null, y);
}-*/;

12
gerrit-gwtui/src/main/java/net/codemirror/lib/ScrollInfo.java vendored
View File

@@ -23,12 +23,12 @@ public class ScrollInfo extends JavaScriptObject {
return createObject().cast();
}
public final native int getLeft() /*-{ return this.left; }-*/;
public final native int getTop() /*-{ return this.top; }-*/;
public final native int getWidth() /*-{ return this.width; }-*/;
public final native int getHeight() /*-{ return this.height; }-*/;
public final native int getClientWidth() /*-{ return this.clientWidth; }-*/;
public final native int getClientHeight() /*-{ return this.clientHeight; }-*/;
public final native double getLeft() /*-{ return this.left; }-*/;
public final native double getTop() /*-{ return this.top; }-*/;
public final native double getWidth() /*-{ return this.width; }-*/;
public final native double getHeight() /*-{ return this.height; }-*/;
public final native double getClientWidth() /*-{ return this.clientWidth; }-*/;
public final native double getClientHeight() /*-{ return this.clientHeight; }-*/;
protected ScrollInfo() {