opendev/gerrit
Code Issues Proposed changes

Implementing UI for range comment

Browse Source 
Added integration of CodeMirror UI and range comment backend.
Implemented highlighting of commented ranges.

Change-Id: I826fdc7d88a564389467c3b6a5ce9eabc75efae7
This commit is contained in:
Michael Zhou
2013-07-26 16:19:16 -07:00
parent eb10727840
commit 8b21a8f169
9 changed files with 132 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -23,6 +23,11 @@ import com.google.gwt.core.client.Scheduler;
import com.google.gwt.core.client.Scheduler.ScheduledCommand;
import com.google.gwt.user.client.ui.Composite;
import net.codemirror.lib.CodeMirror;
import net.codemirror.lib.Configuration;
import net.codemirror.lib.TextMarker;
import net.codemirror.lib.TextMarker.FromTo;
/** An HtmlPanel for displaying a comment */
abstract class CommentBox extends Composite {
static {
@@ -32,12 +37,26 @@ abstract class CommentBox extends Composite {
private PaddingManager widgetManager;
private PaddingWidgetWrapper selfWidgetWrapper;
private SideBySide2 parent;
private CodeMirror cm;
private DisplaySide side;
private DiffChunkInfo diffChunkInfo;
private GutterWrapper gutterWrapper;
private DisplaySide side;
private FromTo fromTo;
private TextMarker rangeMarker;
private TextMarker rangeHighlightMarker;
CommentBox(DisplaySide side) {
CommentBox(CodeMirror cm, CommentInfo info, DisplaySide side) {
this.cm = cm;
this.side = side;
CommentRange range = info.range();
if (range != null) {
fromTo = FromTo.create(range);
rangeMarker = cm.markText(
fromTo.getFrom(),
fromTo.getTo(),
Configuration.create()
.set("className", DiffTable.style.range()));
}
}
@Override
@@ -69,6 +88,7 @@ abstract class CommentBox extends Composite {
void setOpen(boolean open) {
resizePaddingWidget();
setRangeHighlight(open);
}
PaddingManager getPaddingManager() {
@@ -99,6 +119,27 @@ abstract class CommentBox extends Composite {
gutterWrapper = wrapper;
}
void setRangeHighlight(boolean highlight) {
if (fromTo != null) {
if (highlight && rangeHighlightMarker == null) {
rangeHighlightMarker = cm.markText(
fromTo.getFrom(),
fromTo.getTo(),
Configuration.create()
.set("className", DiffTable.style.rangeHighlight()));
} else if (!highlight && rangeHighlightMarker != null) {
rangeHighlightMarker.clear();
rangeHighlightMarker = null;
}
}
}
void clearRange() {
if (rangeMarker != null) {
rangeMarker.clear();
}
}
GutterWrapper getGutterWrapper() {
return gutterWrapper;
}
@@ -106,4 +147,8 @@ abstract class CommentBox extends Composite {
DisplaySide getSide() {
return side;
}
CodeMirror getCm() {
return cm;
}
}

15
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentRange.java
View File

@@ -16,6 +16,9 @@ package com.google.gerrit.client.diff;
import com.google.gwt.core.client.JavaScriptObject;
import net.codemirror.lib.LineCharacter;
import net.codemirror.lib.TextMarker.FromTo;
public class CommentRange extends JavaScriptObject {
public static CommentRange create(int sl, int sc, int el, int ec) {
CommentRange r = createObject().cast();
@@ -23,6 +26,18 @@ public class CommentRange extends JavaScriptObject {
return r;
}
public static CommentRange create(FromTo fromTo) {
if (fromTo == null) {
return null;
}
LineCharacter from = fromTo.getFrom();
LineCharacter to = fromTo.getTo();
return create(
from.getLine() + 1, from.getCh(),
to.getLine() + 1, to.getCh());
}
public final native int start_line() /*-{ return this.start_line; }-*/;
public final native int start_character() /*-{ return this.start_character; }-*/;
public final native int end_line() /*-{ return this.end_line; }-*/;

2
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.java
View File

@@ -40,6 +40,8 @@ class DiffTable extends Composite {
String activeLine();
String activeLineBg();
String hideNumber();
String range();
String rangeHighlight();
}
@UiField

6
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.ui.xml
View File

@@ -76,6 +76,12 @@ limitations under the License.
.activeLineBg {
background-color: #E0FFFF !important;
}
.range {
background-color: #ffd500 !important;
}
.rangeHighlight {
background-color: #ffff00 !important;
}
.cm-searching {
background-color: #ffa !important;
}

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

@@ -59,7 +59,6 @@ class DraftBox extends CommentBox {
private static final int MAX_LINES = 30;
private final SideBySide2 parent;
private final CodeMirror cm;
private final CommentLinkProcessor linkProcessor;
private final PatchSet.Id psId;
private CommentInfo comment;
@@ -82,18 +81,17 @@ class DraftBox extends CommentBox {
@UiField Button discard2;
DraftBox(
SideBySide2 parent,
SideBySide2 sideBySide,
CodeMirror cm,
DisplaySide side,
CommentLinkProcessor clp,
PatchSet.Id id,
CommentInfo info) {
super(side);
super(cm, info, side);
this.parent = parent;
this.cm = cm;
this.linkProcessor = clp;
this.psId = id;
parent = sideBySide;
linkProcessor = clp;
psId = id;
initWidget(uiBinder.createAndBindUi(this));
expandTimer = new Timer() {
@@ -139,7 +137,7 @@ class DraftBox extends CommentBox {
message.setHTML(linkProcessor.apply(
new SafeHtmlBuilder().append(msg).wikify()));
}
this.comment = info;
comment = info;
}
@Override
@@ -181,6 +179,7 @@ class DraftBox extends CommentBox {
UIObject.setVisible(p_view, !edit);
UIObject.setVisible(p_edit, edit);
setRangeHighlight(edit);
if (edit) {
final String msg = comment.message() != null
? comment.message().trim()
@@ -218,6 +217,8 @@ class DraftBox extends CommentBox {
if (replyToBox != null) {
replyToBox.unregisterReplyBox();
}
clearRange();
setRangeHighlight(false);
removeFromParent();
if (!getCommentInfo().has_line()) {
parent.removeFileCommentBox(this);
@@ -226,7 +227,7 @@ class DraftBox extends CommentBox {
PaddingManager manager = getPaddingManager();
manager.remove(this);
parent.removeDraft(this, comment.line() - 1);
cm.focus();
getCm().focus();
getSelfWidgetWrapper().getWidget().clear();
getGutterWrapper().remove();
Scheduler.get().scheduleDeferred(new ScheduledCommand() {
@@ -294,7 +295,7 @@ class DraftBox extends CommentBox {
} else {
CommentApi.updateDraft(psId, original.id(), input, cb);
}
cm.focus();
getCm().focus();
}
private void enableEdit(boolean on) {
@@ -311,7 +312,7 @@ class DraftBox extends CommentBox {
removeUI();
} else {
setEdit(false);
cm.focus();
getCm().focus();
}
}
@@ -349,7 +350,7 @@ class DraftBox extends CommentBox {
return;
} else {
setEdit(false);
cm.focus();
getCm().focus();
return;
}
}

5
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PublishedBox.java
View File

@@ -40,6 +40,8 @@ import com.google.gwt.user.client.ui.UIObject;
import com.google.gwt.user.client.ui.Widget;
import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder;
import net.codemirror.lib.CodeMirror;
/** An HtmlPanel for displaying a published comment */
class PublishedBox extends CommentBox {
interface Binder extends UiBinder<HTMLPanel, PublishedBox> {}
@@ -69,11 +71,12 @@ class PublishedBox extends CommentBox {
PublishedBox(
SideBySide2 parent,
CodeMirror cm,
DisplaySide side,
CommentLinkProcessor clp,
PatchSet.Id psId,
CommentInfo info) {
super(side);
super(cm, info, side);
this.parent = parent;
this.psId = psId;

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

@@ -76,6 +76,7 @@ import net.codemirror.lib.LineCharacter;
import net.codemirror.lib.LineWidget;
import net.codemirror.lib.ModeInjector;
import net.codemirror.lib.ScrollInfo;
import net.codemirror.lib.TextMarker.FromTo;
import java.util.ArrayList;
import java.util.Collections;
@@ -469,22 +470,23 @@ public class SideBySide2 extends Screen {
}
}
private DraftBox addNewDraft(CodeMirror cm, int line) {
private DraftBox addNewDraft(CodeMirror cm, int line, FromTo fromTo) {
DisplaySide side = getSideFromCm(cm);
return addDraftBox(CommentInfo.createRange(
path,
getStoredSideFromDisplaySide(getSideFromCm(cm)),
getStoredSideFromDisplaySide(side),
line + 1,
null,
null,
null), getSideFromCm(cm));
CommentRange.create(fromTo)), side);
}
CommentInfo createReply(CommentInfo replyTo) {
if (!replyTo.has_line()) {
if (!replyTo.has_line() && replyTo.range() == null) {
return CommentInfo.createFile(path, replyTo.side(), replyTo.id(), null);
} else {
return CommentInfo.createRange(path, replyTo.side(), replyTo.line(),
replyTo.id(), null, null);
replyTo.id(), null, replyTo.range());
}
}
@@ -603,7 +605,7 @@ public class SideBySide2 extends Screen {
side = published == publishedBase ? DisplaySide.A : DisplaySide.B;
}
CodeMirror cm = getCmFromSide(side);
PublishedBox box = new PublishedBox(this, side, commentLinkProcessor,
PublishedBox box = new PublishedBox(this, cm, side, commentLinkProcessor,
getPatchSetIdFromSide(side), info);
publishedMap.put(info.id(), box);
if (!info.has_line()) {
@@ -866,7 +868,7 @@ public class SideBySide2 extends Screen {
if (info.isAligned()) {
double myHeight = cm.heightAtLine(line);
double otherHeight = other.heightAtLine(info.getLine());
if (myHeight != otherHeight) {
if (myHeight != otherHeight) {
other.scrollToY(other.getScrollInfo().getTop() + otherHeight - myHeight);
other.setScrollSetAt(System.currentTimeMillis());
}
@@ -906,7 +908,7 @@ public class SideBySide2 extends Screen {
other.removeLineClass(otherActiveLine,
LineClassWhere.BACKGROUND, DiffTable.style.activeLineBg());
}
LineHandle handle = cm.getLineHandleVisualStart(cm.getCursor().getLine());
LineHandle handle = cm.getLineHandleVisualStart(cm.getCursor("end").getLine());
cm.setActiveLine(handle);
if (cm.somethingSelected()) {
return;
@@ -932,9 +934,12 @@ public class SideBySide2 extends Screen {
@Override
public void handle(CodeMirror instance, int line, String gutter,
NativeEvent clickEvent) {
instance.setCursor(LineCharacter.create(line));
instance.setActiveLine(instance.getLineHandle(line));
insertNewDraft(instance).run();
if (!(cm.hasActiveLine() &&
instance.getLineNumber(cm.getActiveLine()) == line)) {
instance.setCursor(LineCharacter.create(line));
instance.setActiveLine(cm.getLineHandle(line));
}
insertNewDraft(cm).run();
}
};
}
@@ -953,8 +958,12 @@ public class SideBySide2 extends Screen {
LineHandle handle = cm.getActiveLine();
int line = cm.getLineNumber(handle);
CommentBox box = lineActiveBoxMap.get(handle);
if (box == null) {
lineActiveBoxMap.put(handle, addNewDraft(cm, line));
FromTo fromTo = cm.getSelectedRange();
if (cm.somethingSelected()) {
lineActiveBoxMap.put(handle,
addNewDraft(cm, line, fromTo.getTo().getLine() == line ? fromTo : null));
} else if (box == null) {
lineActiveBoxMap.put(handle, addNewDraft(cm, line, null));
} else if (box instanceof DraftBox) {
((DraftBox) box).setEdit(true);
} else {

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

@@ -19,6 +19,8 @@ import com.google.gwt.dom.client.Element;
import com.google.gwt.dom.client.NativeEvent;
import com.google.gwt.user.client.rpc.AsyncCallback;
import net.codemirror.lib.TextMarker.FromTo;
/**
* Glue to connect CodeMirror to be callable from GWT.
*
@@ -205,6 +207,10 @@ public class CodeMirror extends JavaScriptObject {
return this.getCursor(start);
}-*/;
public final FromTo getSelectedRange() {
return FromTo.create(getCursor("start"), getCursor("end"));
};
public final native void setCursor(LineCharacter lineCh) /*-{
this.setCursor(lineCh);
}-*/;

17
gerrit-gwtui/src/main/java/net/codemirror/lib/TextMarker.java vendored
View File

@@ -14,6 +14,7 @@
package net.codemirror.lib;
import com.google.gerrit.client.diff.CommentRange;
import com.google.gwt.core.client.JavaScriptObject;
/** Object that represents a text marker within CodeMirror */
@@ -30,9 +31,25 @@ public class TextMarker extends JavaScriptObject {
}
public static class FromTo extends JavaScriptObject {
public static FromTo create(LineCharacter from, LineCharacter to) {
FromTo fromTo = createObject().cast();
fromTo.setFrom(from);
fromTo.setTo(to);
return fromTo;
}
public static FromTo create(CommentRange range) {
return create(
LineCharacter.create(range.start_line() - 1, range.start_character()),
LineCharacter.create(range.end_line() - 1, range.end_character()));
}
public final native LineCharacter getFrom() /*-{ return this.from; }-*/;
public final native LineCharacter getTo() /*-{ return this.to; }-*/;
public final native void setFrom(LineCharacter from) /*-{ this.from = from; }-*/;
public final native void setTo(LineCharacter to) /*-{ this.to = to; }-*/;
protected FromTo() {
}
}