Merge "Update org.pegdown:pegdown to 1.2.1"
diff --git a/Documentation/intro-change-screen.txt b/Documentation/intro-change-screen.txt
index a85a0a6..a8f46e4 100644
--- a/Documentation/intro-change-screen.txt
+++ b/Documentation/intro-change-screen.txt
@@ -225,12 +225,25 @@
* Related Changes
-This tab page shows open changes on which the current change depends and open
-changes that depend on the current change. For merge commits it shows also the
-closed changes that are going to be merged into the destination branch by
-accepting the merge commit. It is kind of a dependency too, but not in the
-sense of "these changes have to be submitted in order to submit this
-change".
+This tab page shows changes on which the current change depends (ancestors) and
+open changes that depend on the current change (descendants). For merge commits
+it shows also the closed changes that are going to be merged into the
+destination branch by accepting the merge commit. It is kind of a dependency
+too, but not in the sense of "these changes have to be submitted in order to
+submit this change".
+
+Indicators are appended on the commit message headlines of related changes to
+signify dependencies on outdated changes, or commits that are not associated to
+changes under review:
+
+If the selected patch set of a change is older than its latest patch set,
+the change is marked with an orange dot.
+
+If a descendant change depends on a patch set that is older than the selected
+patch set of a change, the descendent change is marked with a tilde (~).
+
+If an ancestor commit is not associated to a Gerrit change, i.e. has been pushed
+directly to the repository bypassing review, it is marked with a black dot.
This tab is only visible if related changes for the current active
change exist.
diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt
index 81df883..cd79812 100644
--- a/Documentation/user-search.txt
+++ b/Documentation/user-search.txt
@@ -182,8 +182,8 @@
+
Changes that match 'TEXT' string in any comment left by a reviewer.
-[[file]]
-file:'PATH', f:'PATH'::
+[[path]]
+path:'PATH'::
+
Matches any change touching file at 'PATH'. By default exact path
matching is used, but regular expressions can be enabled by starting
@@ -202,6 +202,18 @@
files named like 'name1.xml', 'name2.xml', and 'name3.xml' use
`file:"^name[1-3].xml"`.
+[[file]]
+file:'NAME', f:'NAME'::
++
+Matches any change touching a file containing the path component
+'NAME'. For example a `file:src` will match changes that modify
+files named `gerrit-server/src/main/java/Foo.java`. Name matching
+is exact match, `file:Foo.java` finds any change touching a file
+named exactly `Foo.java` and does not match `AbstractFoo.java`.
++
+Regular expression matching can be enabled by starting the string
+with `^`. In this mode `file:` is an alias of `path:` (see above).
+
[[has]]
has:draft::
+
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GerritServer.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GerritServer.java
index 9a138f0..6caa2a6 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GerritServer.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GerritServer.java
@@ -170,15 +170,7 @@
}
private static InetAddress getLocalHost() throws UnknownHostException {
- try {
- return InetAddress.getLocalHost();
- } catch (UnknownHostException e1) {
- try {
- return InetAddress.getByName("localhost");
- } catch (UnknownHostException e2) {
- return InetAddress.getByName("127.0.0.1");
- }
- }
+ return InetAddress.getLoopbackAddress();
}
private File sitePath;
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/Gerrit.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/Gerrit.java
index 7972e52..9a6db2f 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/Gerrit.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/Gerrit.java
@@ -276,7 +276,7 @@
public static GitwebLink getGitwebLink() {
GitwebConfig gw = getConfig().getGitwebLink();
- return gw != null ? new GitwebLink(gw) : null;
+ return gw != null && gw.type != null ? new GitwebLink(gw) : null;
}
/** Site theme information (site specific colors)/ */
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.java
index 4b7e14a..52b988f 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.java
@@ -52,7 +52,7 @@
}
@UiField Style style;
- @UiField Element commitName;
+ @UiField CopyableLabel commitName;
@UiField AnchorElement browserLink;
@UiField InlineHyperlink authorNameEmail;
@UiField Element authorDate;
@@ -99,7 +99,7 @@
}
}
- commitName.setInnerText(revision);
+ commitName.setText(revision);
idText.setText("Change-Id: " + change.change_id());
idText.setPreviewText(change.change_id());
formatLink(commit.author(), authorNameEmail,
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.ui.xml
index 8b543ba..c02e9f8 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.ui.xml
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.ui.xml
@@ -100,7 +100,7 @@
<ui:attribute name='title'/>
</x:InlineHyperlink>
</td>
- <td ui:field='authorDate' class='{style.date}'/>
+ <td ui:field='authorDate' class='{style.date}' colspan="2"/>
</tr>
<tr>
<th><ui:msg>Committer</ui:msg></th>
@@ -109,12 +109,12 @@
<ui:attribute name='title'/>
</x:InlineHyperlink>
</td>
- <td ui:field='committerDate' class='{style.date}'/>
+ <td ui:field='committerDate' class='{style.date}' colspan="2"/>
</tr>
<tr>
<th><ui:msg>Commit</ui:msg></th>
- <td ui:field='commitName'/>
- <td><a ui:field='browserLink' href=""/></td>
+ <td><clippy:CopyableLabel styleName='{style.clippy}' ui:field='commitName'/></td>
+ <td><a style="margin-left:16px;" ui:field='browserLink' href=""/></td>
</tr>
<tr>
<th><ui:msg>Change-Id</ui:msg></th>
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/History.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/History.java
index c461ae9..62652de 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/History.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/History.java
@@ -104,7 +104,7 @@
for (String path : map.keySet()) {
for (CommentInfo c : Natives.asList(map.get(path))) {
- c.setPath(path);
+ c.path(path);
if (c.author() != null) {
AuthorRevision k = new AuthorRevision(c.author(), id);
List<CommentInfo> l = byAuthor.get(k);
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ReplyBox.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ReplyBox.java
index dfece94..77e6867 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ReplyBox.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ReplyBox.java
@@ -428,7 +428,7 @@
private static List<CommentInfo> copyPath(String path, JsArray<CommentInfo> l) {
for (int i = 0; i < l.length(); i++) {
- l.get(i).setPath(path);
+ l.get(i).path(path);
}
return Natives.asList(l);
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentApi.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentApi.java
index d87cb5e..66cd485 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentApi.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentApi.java
@@ -43,13 +43,13 @@
revision(id, "drafts").id(draftId).get(cb);
}
- public static void createDraft(PatchSet.Id id, CommentInput content,
+ public static void createDraft(PatchSet.Id id, CommentInfo content,
AsyncCallback<CommentInfo> cb) {
revision(id, "drafts").put(content, cb);
}
public static void updateDraft(PatchSet.Id id, String draftId,
- CommentInput content, AsyncCallback<CommentInfo> cb) {
+ CommentInfo content, AsyncCallback<CommentInfo> cb) {
revision(id, "drafts").id(draftId).put(content, cb);
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentInfo.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentInfo.java
index 0879a50..e1ec47a 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentInfo.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentInfo.java
@@ -23,42 +23,65 @@
import java.sql.Timestamp;
public class CommentInfo extends JavaScriptObject {
- public static CommentInfo createRange(String path, Side side, int line,
- String in_reply_to, String message, CommentRange range) {
- CommentInfo info = createFile(path, side, in_reply_to, message);
- info.setRange(range);
- info.setLine(range == null ? line : range.end_line());
- return info;
+ public static CommentInfo create(String path, Side side,
+ int line, CommentRange range) {
+ CommentInfo n = createObject().cast();
+ n.path(path);
+ n.side(side);
+ if (range != null) {
+ n.line(range.end_line());
+ n.range(range);
+ } else if (line > 0) {
+ n.line(line);
+ }
+ return n;
}
- public static CommentInfo createFile(String path, Side side,
- String in_reply_to, String message) {
- CommentInfo info = createObject().cast();
- info.setPath(path);
- info.setSide(side);
- info.setInReplyTo(in_reply_to);
- info.setMessage(message);
- return info;
+ public static CommentInfo createReply(CommentInfo r) {
+ CommentInfo n = createObject().cast();
+ n.path(r.path());
+ n.side(r.side());
+ n.in_reply_to(r.id());
+ if (r.has_range()) {
+ n.line(r.range().end_line());
+ n.range(r.range());
+ } else if (r.has_line()) {
+ n.line(r.line());
+ }
+ return n;
}
- private final native void setId(String id) /*-{ this.id = id; }-*/;
- public final native void setPath(String path) /*-{ this.path = path; }-*/;
-
- private final void setSide(Side side) {
- setSideRaw(side.toString());
+ public static CommentInfo copy(CommentInfo s) {
+ CommentInfo n = createObject().cast();
+ n.path(s.path());
+ n.side(s.side());
+ n.id(s.id());
+ n.in_reply_to(s.in_reply_to());
+ n.message(s.message());
+ if (s.has_range()) {
+ n.line(s.range().end_line());
+ n.range(s.range());
+ } else if (s.has_line()) {
+ n.line(s.line());
+ }
+ return n;
}
- private final native void setSideRaw(String side) /*-{ this.side = side; }-*/;
- private final native void setLine(int line) /*-{ this.line = line; }-*/;
+ public final native void path(String p) /*-{ this.path = p }-*/;
+ public final native void id(String i) /*-{ this.id = i }-*/;
+ public final native void line(int n) /*-{ this.line = n }-*/;
+ public final native void range(CommentRange r) /*-{ this.range = r }-*/;
+ public final native void in_reply_to(String i) /*-{ this.in_reply_to = i }-*/;
+ public final native void message(String m) /*-{ this.message = m }-*/;
- private final native void setInReplyTo(String in_reply_to) /*-{
- this.in_reply_to = in_reply_to;
- }-*/;
+ public final void side(Side side) {
+ sideRaw(side.toString());
+ }
+ private final native void sideRaw(String s) /*-{ this.side = s }-*/;
- private final native void setMessage(String message) /*-{ this.message = message; }-*/;
-
- public final native String id() /*-{ return this.id; }-*/;
- public final native String path() /*-{ return this.path; }-*/;
+ public final native String path() /*-{ return this.path }-*/;
+ public final native String id() /*-{ return this.id }-*/;
+ public final native String in_reply_to() /*-{ return this.in_reply_to }-*/;
public final Side side() {
String s = sideRaw();
@@ -68,10 +91,6 @@
}
private final native String sideRaw() /*-{ return this.side }-*/;
- public final native int line() /*-{ return this.line || 0; }-*/;
- public final native String in_reply_to() /*-{ return this.in_reply_to; }-*/;
- public final native String message() /*-{ return this.message; }-*/;
-
public final Timestamp updated() {
Timestamp r = updatedTimestamp();
if (r == null) {
@@ -83,17 +102,16 @@
}
return r;
}
- private final native String updatedRaw() /*-{ return this.updated; }-*/;
+ private final native String updatedRaw() /*-{ return this.updated }-*/;
private final native Timestamp updatedTimestamp() /*-{ return this._ts }-*/;
private final native void updatedTimestamp(Timestamp t) /*-{ this._ts = t }-*/;
- public final native AccountInfo author() /*-{ return this.author; }-*/;
-
- public final native boolean has_line() /*-{ return this.hasOwnProperty('line'); }-*/;
-
- public final native CommentRange range() /*-{ return this.range; }-*/;
-
- public final native void setRange(CommentRange range) /*-{ this.range = range; }-*/;
+ public final native AccountInfo author() /*-{ return this.author }-*/;
+ public final native int line() /*-{ return this.line || 0 }-*/;
+ public final native boolean has_line() /*-{ return this.hasOwnProperty('line') }-*/;
+ public final native boolean has_range() /*-{ return this.hasOwnProperty('range') }-*/;
+ public final native CommentRange range() /*-{ return this.range }-*/;
+ public final native String message() /*-{ return this.message }-*/;
protected CommentInfo() {
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentInput.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentInput.java
deleted file mode 100644
index c96a67f..0000000
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentInput.java
+++ /dev/null
@@ -1,82 +0,0 @@
-// Copyright (C) 2013 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.client.changes;
-
-import com.google.gerrit.client.diff.CommentRange;
-import com.google.gerrit.common.changes.Side;
-import com.google.gwt.core.client.JavaScriptObject;
-import com.google.gwtjsonrpc.client.impl.ser.JavaSqlTimestamp_JsonSerializer;
-
-import java.sql.Timestamp;
-
-public class CommentInput extends JavaScriptObject {
- public static CommentInput create(CommentInfo original) {
- CommentInput input = createObject().cast();
- input.setId(original.id());
- input.setPath(original.path());
- input.setSide(original.side());
- if (original.has_line()) {
- input.setLine(original.line());
- }
- input.setRange(original.range());
- input.setInReplyTo(original.in_reply_to());
- input.setMessage(original.message());
- return input;
- }
-
- public final native void setId(String id) /*-{ this.id = id; }-*/;
- public final native void setPath(String path) /*-{ this.path = path; }-*/;
-
- public final void setSide(Side side) {
- setSideRaw(side.toString());
- }
- private final native void setSideRaw(String side) /*-{ this.side = side; }-*/;
-
- public final native void setLine(int line) /*-{ this.line = line; }-*/;
-
- public final native void setInReplyTo(String in_reply_to) /*-{
- this.in_reply_to = in_reply_to;
- }-*/;
-
- public final native void setMessage(String message) /*-{ this.message = message; }-*/;
- public final native String id() /*-{ return this.id; }-*/;
- public final native String path() /*-{ return this.path; }-*/;
-
- public final Side side() {
- String s = sideRaw();
- return s != null
- ? Side.valueOf(s)
- : Side.REVISION;
- }
- private final native String sideRaw() /*-{ return this.side }-*/;
-
- public final native int line() /*-{ return this.line; }-*/;
- public final native String in_reply_to() /*-{ return this.in_reply_to; }-*/;
- public final native String message() /*-{ return this.message; }-*/;
-
- public final Timestamp updated() {
- return JavaSqlTimestamp_JsonSerializer.parseTimestamp(updatedRaw());
- }
- private final native String updatedRaw() /*-{ return this.updated; }-*/;
-
- public final native boolean has_line() /*-{ return this.hasOwnProperty('line'); }-*/;
-
- public final native CommentRange range() /*-{ return this.range; }-*/;
-
- public final native void setRange(CommentRange range) /*-{ this.range = range; }-*/;
-
- protected CommentInput() {
- }
-}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ChunkManager.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ChunkManager.java
index ee28ece..1ae8bbd 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ChunkManager.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ChunkManager.java
@@ -16,26 +16,24 @@
import com.google.gerrit.client.diff.DiffInfo.Region;
import com.google.gerrit.client.diff.DiffInfo.Span;
-import com.google.gerrit.client.diff.PaddingManager.LinePaddingWidgetWrapper;
import com.google.gerrit.client.rpc.Natives;
import com.google.gwt.core.client.JsArray;
import com.google.gwt.core.client.JsArrayString;
import com.google.gwt.dom.client.Element;
import com.google.gwt.dom.client.Style.Unit;
+import com.google.gwt.user.client.DOM;
import net.codemirror.lib.CodeMirror;
import net.codemirror.lib.CodeMirror.LineClassWhere;
-import net.codemirror.lib.CodeMirror.LineHandle;
import net.codemirror.lib.Configuration;
import net.codemirror.lib.LineCharacter;
+import net.codemirror.lib.LineWidget;
import net.codemirror.lib.TextMarker;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
-import java.util.HashMap;
import java.util.List;
-import java.util.Map;
/** Colors modified regions for {@link SideBySide2}. */
class ChunkManager {
@@ -48,7 +46,7 @@
private List<DiffChunkInfo> chunks;
private List<TextMarker> markers;
private List<Runnable> undo;
- private Map<LineHandle, LinePaddingWidgetWrapper> paddingOnOtherSide;
+ private List<LineWidget> padding;
ChunkManager(SideBySide2 host,
CodeMirror cmA,
@@ -66,7 +64,15 @@
}
DiffChunkInfo getFirst() {
- return chunks.isEmpty() ? null : chunks.get(0);
+ if (!chunks.isEmpty()) {
+ for (DiffChunkInfo d : chunks) {
+ if (d.getSide() == DisplaySide.B) {
+ return d;
+ }
+ }
+ return chunks.get(0);
+ }
+ return null;
}
void reset() {
@@ -77,8 +83,8 @@
for (Runnable r : undo) {
r.run();
}
- for (LinePaddingWidgetWrapper x : paddingOnOtherSide.values()) {
- x.getWidget().clear();
+ for (LineWidget w : padding) {
+ w.clear();
}
}
@@ -86,7 +92,7 @@
chunks = new ArrayList<DiffChunkInfo>();
markers = new ArrayList<TextMarker>();
undo = new ArrayList<Runnable>();
- paddingOnOtherSide = new HashMap<LineHandle, LinePaddingWidgetWrapper>();
+ padding = new ArrayList<LineWidget>();
String diffColor = diff.meta_a() == null || diff.meta_b() == null
? DiffTable.style.intralineBg()
@@ -118,16 +124,18 @@
colorLines(cmB, color, startB, bLen);
markEdit(cmA, startA, a, region.edit_a());
markEdit(cmB, startB, b, region.edit_b());
+ addPadding(cmA, startA + aLen - 1, bLen - aLen);
+ addPadding(cmB, startB + bLen - 1, aLen - bLen);
addGutterTag(region, startA, startB);
mapper.appendReplace(aLen, bLen);
int endA = mapper.getLineA() - 1;
int endB = mapper.getLineB() - 1;
if (aLen > 0) {
- addDiffChunkAndPadding(cmB, endB, endA, aLen, bLen > 0);
+ addDiffChunk(cmB, endB, endA, aLen, bLen > 0);
}
if (bLen > 0) {
- addDiffChunkAndPadding(cmA, endA, endB, bLen, aLen > 0);
+ addDiffChunk(cmA, endA, endB, bLen, aLen > 0);
}
}
@@ -194,13 +202,37 @@
}
}
- private void addDiffChunkAndPadding(CodeMirror cmToPad, int lineToPad,
+ /**
+ * Insert a new padding div below the given line.
+ *
+ * @param cm parent CodeMirror to add extra space into.
+ * @param line line to put the padding below.
+ * @param len number of lines to pad. Padding is inserted only if
+ * {@code len >= 1}.
+ */
+ private void addPadding(CodeMirror cm, int line, int len) {
+ if (0 < len) {
+ // DiffTable adds 1px bottom padding to each line to preserve
+ // sufficient space for underscores commonly appearing in code.
+ // Padding should be 1em + 1px high for each line. Add within
+ // the browser using height + padding-bottom.
+ Element pad = DOM.createDiv();
+ pad.setClassName(DiffTable.style.padding());
+ pad.getStyle().setHeight(len, Unit.EM);
+ pad.getStyle().setPaddingBottom(len, Unit.PX);
+ padding.add(cm.addLineWidget(
+ line == -1 ? 0 : line,
+ pad,
+ Configuration.create()
+ .set("coverGutter", true)
+ .set("noHScroll", true)
+ .set("above", line == -1)));
+ }
+ }
+
+ private void addDiffChunk(CodeMirror cmToPad, int lineToPad,
int lineOnOther, int chunkSize, boolean edit) {
- CodeMirror otherCm = host.otherCm(cmToPad);
- paddingOnOtherSide.put(otherCm.getLineHandle(lineOnOther),
- new LinePaddingWidgetWrapper(host.addPaddingWidget(cmToPad,
- lineToPad, 0, Unit.EM, null), lineToPad, chunkSize));
- chunks.add(new DiffChunkInfo(otherCm.side(),
+ chunks.add(new DiffChunkInfo(host.otherCm(cmToPad).side(),
lineOnOther - chunkSize + 1, lineOnOther, edit));
}
@@ -283,62 +315,4 @@
}
return null;
}
-
- void resizePadding(final CodeMirror cm,
- final LineHandle line,
- final DisplaySide side) {
- if (paddingOnOtherSide.containsKey(line)) {
- host.defer(new Runnable() {
- @Override
- public void run() {
- resizePaddingOnOtherSide(side, cm.getLineNumber(line));
- }
- });
- }
- }
-
- void resizePaddingOnOtherSide(DisplaySide mySide, int line) {
- CodeMirror cm = host.getCmFromSide(mySide);
- LineHandle handle = cm.getLineHandle(line);
- final LinePaddingWidgetWrapper otherWrapper = paddingOnOtherSide.get(handle);
- double myChunkHeight = cm.heightAtLine(line + 1) -
- cm.heightAtLine(line - otherWrapper.getChunkLength() + 1);
- Element otherPadding = otherWrapper.getElement();
- int otherPaddingHeight = otherPadding.getOffsetHeight();
- CodeMirror otherCm = host.otherCm(cm);
- int otherLine = otherWrapper.getOtherLine();
- LineHandle other = otherCm.getLineHandle(otherLine);
- if (paddingOnOtherSide.containsKey(other)) {
- LinePaddingWidgetWrapper myWrapper = paddingOnOtherSide.get(other);
- Element myPadding = paddingOnOtherSide.get(other).getElement();
- int myPaddingHeight = myPadding.getOffsetHeight();
- myChunkHeight -= myPaddingHeight;
- double otherChunkHeight = otherCm.heightAtLine(otherLine + 1) -
- otherCm.heightAtLine(otherLine - myWrapper.getChunkLength() + 1) -
- otherPaddingHeight;
- double delta = myChunkHeight - otherChunkHeight;
- if (delta > 0) {
- if (myPaddingHeight != 0) {
- myPadding.getStyle().setHeight((double) 0, Unit.PX);
- myWrapper.getWidget().changed();
- }
- if (otherPaddingHeight != delta) {
- otherPadding.getStyle().setHeight(delta, Unit.PX);
- otherWrapper.getWidget().changed();
- }
- } else {
- if (myPaddingHeight != -delta) {
- myPadding.getStyle().setHeight(-delta, Unit.PX);
- myWrapper.getWidget().changed();
- }
- if (otherPaddingHeight != 0) {
- otherPadding.getStyle().setHeight((double) 0, Unit.PX);
- otherWrapper.getWidget().changed();
- }
- }
- } else if (otherPaddingHeight != myChunkHeight) {
- otherPadding.getStyle().setHeight(myChunkHeight, Unit.PX);
- otherWrapper.getWidget().changed();
- }
- }
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentBox.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentBox.java
index 83d2f99..0e82c4cf 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentBox.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentBox.java
@@ -15,7 +15,6 @@
package com.google.gerrit.client.diff;
import com.google.gerrit.client.changes.CommentInfo;
-import com.google.gerrit.client.diff.PaddingManager.PaddingWidgetWrapper;
import com.google.gerrit.client.diff.SidePanel.GutterWrapper;
import com.google.gwt.event.dom.client.MouseOutEvent;
import com.google.gwt.event.dom.client.MouseOutHandler;
@@ -34,23 +33,17 @@
Resources.I.style().ensureInjected();
}
- private PaddingManager widgetManager;
- private PaddingWidgetWrapper selfWidgetWrapper;
- private CommentManager commentManager;
- private CodeMirror cm;
- private DiffChunkInfo diffChunkInfo;
+ private final CommentGroup group;
private GutterWrapper gutterWrapper;
private FromTo fromTo;
private TextMarker rangeMarker;
private TextMarker rangeHighlightMarker;
- CommentBox(CommentManager commentManager, CodeMirror cm, CommentInfo info) {
- this.commentManager = commentManager;
- this.cm = cm;
- CommentRange range = info.range();
+ CommentBox(CommentGroup group, CommentRange range) {
+ this.group = group;
if (range != null) {
fromTo = FromTo.create(range);
- rangeMarker = cm.markText(
+ rangeMarker = group.getCm().markText(
fromTo.getFrom(),
fromTo.getTo(),
Configuration.create()
@@ -70,62 +63,21 @@
}, MouseOutEvent.getType());
}
- @Override
- protected void onLoad() {
- resizePaddingWidget();
- }
-
- void resizePaddingWidget() {
- if (!getCommentInfo().has_line()) {
- return;
- }
- commentManager.getSideBySide2().defer(new Runnable() {
- @Override
- public void run() {
- assert selfWidgetWrapper != null;
- selfWidgetWrapper.getWidget().changed();
- if (diffChunkInfo != null) {
- commentManager.getSideBySide2().getChunkManager()
- .resizePaddingOnOtherSide(cm.side(), diffChunkInfo.getEnd());
- } else {
- assert widgetManager != null;
- widgetManager.resizePaddingWidget();
- }
- }
- });
- }
-
abstract CommentInfo getCommentInfo();
abstract boolean isOpen();
void setOpen(boolean open) {
- resizePaddingWidget();
+ group.resize();
setRangeHighlight(open);
getCm().focus();
}
+ CommentGroup getCommentGroup() {
+ return group;
+ }
+
CommentManager getCommentManager() {
- return commentManager;
- }
-
- PaddingManager getPaddingManager() {
- return widgetManager;
- }
-
- void setPaddingManager(PaddingManager manager) {
- widgetManager = manager;
- }
-
- void setSelfWidgetWrapper(PaddingWidgetWrapper wrapper) {
- selfWidgetWrapper = wrapper;
- }
-
- PaddingWidgetWrapper getSelfWidgetWrapper() {
- return selfWidgetWrapper;
- }
-
- void setDiffChunkInfo(DiffChunkInfo info) {
- this.diffChunkInfo = info;
+ return group.getCommentManager();
}
void setGutterWrapper(GutterWrapper wrapper) {
@@ -135,7 +87,7 @@
void setRangeHighlight(boolean highlight) {
if (fromTo != null) {
if (highlight && rangeHighlightMarker == null) {
- rangeHighlightMarker = cm.markText(
+ rangeHighlightMarker = group.getCm().markText(
fromTo.getFrom(),
fromTo.getTo(),
Configuration.create()
@@ -150,6 +102,7 @@
void clearRange() {
if (rangeMarker != null) {
rangeMarker.clear();
+ rangeMarker = null;
}
}
@@ -158,6 +111,6 @@
}
CodeMirror getCm() {
- return cm;
+ return group.getCm();
}
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentBoxUi.css b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentBoxUi.css
index ffce7e6..8efc9f7 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentBoxUi.css
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentBoxUi.css
@@ -13,18 +13,18 @@
* limitations under the License.
*/
-.commentBox {
- position: relative;
- width: 679px;
- min-height: 16px;
+.commentWidgets {
font-family: sans-serif;
background-color: #fcfa96;
border: 1px solid black;
-webkit-box-shadow: 3px 3px 3px #888888;
-moz-box-shadow: 3px 3px 3px #888888;
box-shadow: 3px 3px 3px #888888;
+
+ /* margin-bottom is fixed in CommentGroup.computeHeight() */
margin-bottom: 5px;
margin-right: 5px;
+
-webkit-touch-callout: initial;
-webkit-user-select: initial;
-khtml-user-select: initial;
@@ -32,7 +32,15 @@
-ms-user-select: initial;
}
-.header { cursor: pointer; }
+.commentBox {
+ position: relative;
+ min-height: 16px;
+}
+
+.header {
+ max-width: 650px;
+ cursor: pointer;
+}
.summary {
color: #777;
@@ -58,13 +66,15 @@
padding-top: 2px;
position: relative;
}
-.contents p,
-.contents ul,
-.contents blockquote {
+.message {
+ overflow-x: auto;
+}
+.message p,
+.message ul,
+.message blockquote {
-webkit-margin-before: 0;
-webkit-margin-after: 0.3em;
}
-
.commentBox button {
margin-right: 3px;
margin-bottom: 1px;
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentGroup.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentGroup.java
new file mode 100644
index 0000000..5294cd0
--- /dev/null
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentGroup.java
@@ -0,0 +1,225 @@
+// Copyright (C) 2013 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.client.diff;
+
+import com.google.gwt.dom.client.Element;
+import com.google.gwt.dom.client.Style.Unit;
+import com.google.gwt.user.client.DOM;
+import com.google.gwt.user.client.Timer;
+import com.google.gwt.user.client.ui.Composite;
+import com.google.gwt.user.client.ui.FlowPanel;
+import com.google.gwt.user.client.ui.SimplePanel;
+
+import net.codemirror.lib.CodeMirror;
+import net.codemirror.lib.Configuration;
+import net.codemirror.lib.LineWidget;
+
+/**
+ * LineWidget attached to a CodeMirror container.
+ *
+ * When a comment is placed on a line a CommentWidget is created on both sides.
+ * The group tracks all comment boxes on that same line, and also includes an
+ * empty padding element to keep subsequent lines vertically aligned.
+ */
+class CommentGroup extends Composite {
+ static void pair(CommentGroup a, CommentGroup b) {
+ a.peer = b;
+ b.peer = a;
+ }
+
+ private final CommentManager manager;
+ private final CodeMirror cm;
+ private final int line;
+ private final FlowPanel comments;
+ private final Element padding;
+ private LineWidget lineWidget;
+ private Timer resizeTimer;
+ private CommentGroup peer;
+
+ CommentGroup(CommentManager manager, CodeMirror cm, int line) {
+ this.manager = manager;
+ this.cm = cm;
+ this.line = line;
+
+ comments = new FlowPanel();
+ comments.setStyleName(Resources.I.style().commentWidgets());
+ comments.setVisible(false);
+ initWidget(new SimplePanel(comments));
+
+ padding = DOM.createDiv();
+ padding.setClassName(DiffTable.style.padding());
+ getElement().appendChild(padding);
+ }
+
+ CommentManager getCommentManager() {
+ return manager;
+ }
+
+ CodeMirror getCm() {
+ return cm;
+ }
+
+ CommentGroup getPeer() {
+ return peer;
+ }
+
+ int getLine() {
+ return line;
+ }
+
+ void add(PublishedBox box) {
+ comments.add(box);
+ comments.setVisible(true);
+ }
+
+ void add(DraftBox box) {
+ PublishedBox p = box.getReplyToBox();
+ if (p != null) {
+ for (int i = 0; i < getBoxCount(); i++) {
+ if (p == getCommentBox(i)) {
+ comments.insert(box, i + 1);
+ comments.setVisible(true);
+ resize();
+ return;
+ }
+ }
+ }
+ comments.add(box);
+ comments.setVisible(true);
+ resize();
+ }
+
+ CommentBox getCommentBox(int i) {
+ return (CommentBox) comments.getWidget(i);
+ }
+
+ int getBoxCount() {
+ return comments.getWidgetCount();
+ }
+
+ void openCloseLast() {
+ if (0 < getBoxCount()) {
+ CommentBox box = getCommentBox(getBoxCount() - 1);
+ box.setOpen(!box.isOpen());
+ }
+ }
+
+ void openCloseAll() {
+ boolean open = false;
+ for (int i = 0; i < getBoxCount(); i++) {
+ if (!getCommentBox(i).isOpen()) {
+ open = true;
+ break;
+ }
+ }
+ for (int i = 0; i < getBoxCount(); i++) {
+ getCommentBox(i).setOpen(open);
+ }
+ }
+
+ void remove(DraftBox box) {
+ comments.remove(box);
+ comments.setVisible(0 < getBoxCount());
+
+ if (0 < getBoxCount() || 0 < peer.getBoxCount()) {
+ resize();
+ } else {
+ detach();
+ peer.detach();
+ }
+ }
+
+ private void detach() {
+ if (lineWidget != null) {
+ lineWidget.clear();
+ lineWidget = null;
+ }
+ manager.clearLine(cm.side(), line);
+ removeFromParent();
+ }
+
+ void attach(DiffTable parent) {
+ parent.add(this);
+ lineWidget = cm.addLineWidget(Math.max(0, line - 1), getElement(),
+ Configuration.create()
+ .set("coverGutter", true)
+ .set("noHScroll", true)
+ .set("above", line <= 0)
+ .set("insertAt", 0));
+ }
+
+ void handleRedraw() {
+ lineWidget.onRedraw(new Runnable() {
+ @Override
+ public void run() {
+ if (canComputeHeight() && peer.canComputeHeight()) {
+ if (resizeTimer != null) {
+ resizeTimer.cancel();
+ resizeTimer = null;
+ }
+ adjustPadding(CommentGroup.this, peer);
+ } else if (resizeTimer == null) {
+ resizeTimer = new Timer() {
+ @Override
+ public void run() {
+ if (canComputeHeight() && peer.canComputeHeight()) {
+ cancel();
+ resizeTimer = null;
+ adjustPadding(CommentGroup.this, peer);
+ }
+ }
+ };
+ resizeTimer.scheduleRepeating(5);
+ }
+ }
+ });
+ }
+
+ @Override
+ protected void onUnload() {
+ super.onUnload();
+ if (resizeTimer != null) {
+ resizeTimer.cancel();
+ }
+ }
+
+ void resize() {
+ if (lineWidget != null) {
+ adjustPadding(this, peer);
+ }
+ }
+
+ private boolean canComputeHeight() {
+ return !comments.isVisible() || comments.getOffsetHeight() > 0;
+ }
+
+ private int computeHeight() {
+ if (comments.isVisible()) {
+ // Include margin-bottom: 5px from CSS class.
+ return comments.getOffsetHeight() + 5;
+ }
+ return 0;
+ }
+
+ private static void adjustPadding(CommentGroup a, CommentGroup b) {
+ int apx = a.computeHeight();
+ int bpx = b.computeHeight();
+ int h = Math.max(apx, bpx);
+ a.padding.getStyle().setHeight(Math.max(0, h - apx), Unit.PX);
+ b.padding.getStyle().setHeight(Math.max(0, h - bpx), Unit.PX);
+ a.lineWidget.changed();
+ b.lineWidget.changed();
+ }
+}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentManager.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentManager.java
index ce9595c..57c74d2 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentManager.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentManager.java
@@ -16,7 +16,6 @@
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.changes.CommentInfo;
-import com.google.gerrit.client.diff.PaddingManager.PaddingWidgetWrapper;
import com.google.gerrit.client.patches.SkippedLine;
import com.google.gerrit.client.rpc.CallbackGroup;
import com.google.gerrit.client.rpc.Natives;
@@ -24,14 +23,10 @@
import com.google.gerrit.common.changes.Side;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gwt.core.client.JsArray;
-import com.google.gwt.core.client.Scheduler;
-import com.google.gwt.core.client.Scheduler.ScheduledCommand;
-import com.google.gwt.dom.client.Style.Unit;
import net.codemirror.lib.CodeMirror;
-import net.codemirror.lib.Configuration;
-import net.codemirror.lib.LineWidget;
import net.codemirror.lib.CodeMirror.LineHandle;
+import net.codemirror.lib.LineCharacter;
import net.codemirror.lib.TextMarker.FromTo;
import java.util.ArrayList;
@@ -40,6 +35,8 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.SortedMap;
+import java.util.TreeMap;
/** Tracks comment widgets for {@link SideBySide2}. */
class CommentManager {
@@ -50,10 +47,10 @@
private final CommentLinkProcessor commentLinkProcessor;
private final Map<String, PublishedBox> published;
- private final Map<LineHandle, CommentBox> lineActiveBox;
- private final Map<LineHandle, List<PublishedBox>> linePublishedBoxes;
- private final Map<LineHandle, PaddingManager> linePaddingManager;
+ private final SortedMap<Integer, CommentGroup> sideA;
+ private final SortedMap<Integer, CommentGroup> sideB;
private final Set<DraftBox> unsavedDrafts;
+ private boolean attached;
CommentManager(SideBySide2 host,
PatchSet.Id base, PatchSet.Id revision,
@@ -66,9 +63,8 @@
this.commentLinkProcessor = clp;
published = new HashMap<String, PublishedBox>();
- lineActiveBox = new HashMap<LineHandle, CommentBox>();
- linePublishedBoxes = new HashMap<LineHandle, List<PublishedBox>>();
- linePaddingManager = new HashMap<LineHandle, PaddingManager>();
+ sideA = new TreeMap<Integer, CommentGroup>();
+ sideB = new TreeMap<Integer, CommentGroup>();
unsavedDrafts = new HashSet<DraftBox>();
}
@@ -82,7 +78,46 @@
}
}
- void render(CommentsCollections in) {
+ Runnable commentNav(final CodeMirror src, final Direction dir) {
+ return new Runnable() {
+ @Override
+ public void run() {
+ // Every comment appears in both side maps as a linked pair.
+ // It is only necessary to search one side to find a comment
+ // on either side of the editor pair.
+ SortedMap<Integer, CommentGroup> map = map(src.side());
+ int line = src.hasActiveLine()
+ ? src.getLineNumber(src.getActiveLine()) + 1
+ : 0;
+ if (dir == Direction.NEXT) {
+ map = map.tailMap(line + 1);
+ if (map.isEmpty()) {
+ return;
+ }
+ line = map.firstKey();
+ } else {
+ map = map.headMap(line);
+ if (map.isEmpty()) {
+ return;
+ }
+ line = map.lastKey();
+ }
+
+ CommentGroup g = map.get(line);
+ if (g.getBoxCount() == 0) {
+ g = g.getPeer();
+ }
+
+ CodeMirror cm = g.getCm();
+ double y = cm.heightAtLine(g.getLine() - 1, "local");
+ cm.setCursor(LineCharacter.create(g.getLine() - 1));
+ cm.scrollToY(y - 0.5 * cm.getScrollbarV().getClientHeight());
+ cm.focus();
+ }
+ };
+ }
+
+ void render(CommentsCollections in, boolean expandAll) {
if (in.publishedBase != null) {
renderPublished(DisplaySide.A, in.publishedBase);
}
@@ -95,64 +130,100 @@
if (in.draftsRevision != null) {
renderDrafts(DisplaySide.B, in.draftsRevision);
}
+ if (expandAll) {
+ setExpandAllComments(true);
+ }
+ for (CommentGroup g : sideA.values()) {
+ g.attach(host.diffTable);
+ }
+ for (CommentGroup g : sideB.values()) {
+ g.attach(host.diffTable);
+ g.handleRedraw();
+ }
+ attached = true;
}
private void renderPublished(DisplaySide forSide, JsArray<CommentInfo> in) {
for (CommentInfo info : Natives.asList(in)) {
DisplaySide side = displaySide(info, forSide);
- if (side == null) {
- continue;
+ if (side != null) {
+ CommentGroup group = group(side, info.line());
+ PublishedBox box = new PublishedBox(
+ group,
+ commentLinkProcessor,
+ getPatchSetIdFromSide(side),
+ info);
+ group.add(box);
+ box.setGutterWrapper(host.diffTable.sidePanel.addGutter(
+ host.getCmFromSide(side),
+ Math.max(0, info.line() - 1),
+ SidePanel.GutterType.COMMENT));
+ published.put(info.id(), box);
}
-
- CodeMirror cm = host.getCmFromSide(side);
- PublishedBox box = new PublishedBox(this, cm, commentLinkProcessor,
- getPatchSetIdFromSide(side), info);
- published.put(info.id(), box);
- if (!info.has_line()) {
- host.diffTable.addFileCommentBox(box);
- continue;
- }
-
- int line = info.line() - 1;
- LineHandle handle = cm.getLineHandle(line);
- if (linePublishedBoxes.containsKey(handle)) {
- linePublishedBoxes.get(handle).add(box);
- } else {
- List<PublishedBox> list = new ArrayList<PublishedBox>(4);
- list.add(box);
- linePublishedBoxes.put(handle, list);
- }
- lineActiveBox.put(handle, box);
- addCommentBox(info, box);
}
}
private void renderDrafts(DisplaySide forSide, JsArray<CommentInfo> in) {
for (CommentInfo info : Natives.asList(in)) {
DisplaySide side = displaySide(info, forSide);
- if (side == null) {
- continue;
+ if (side != null) {
+ addDraftBox(side, info);
}
-
- CodeMirror cm = host.getCmFromSide(side);
- DraftBox box = new DraftBox(this, cm, commentLinkProcessor,
- getPatchSetIdFromSide(side), info);
- if (info.in_reply_to() != null) {
- PublishedBox r = published.get(info.in_reply_to());
- if (r != null) {
- r.registerReplyBox(box);
- }
- }
- if (!info.has_line()) {
- host.diffTable.addFileCommentBox(box);
- continue;
- }
-
- lineActiveBox.put(cm.getLineHandle(info.line() - 1), box);
- addCommentBox(info, box);
}
}
+ /**
+ * Create a new {@link DraftBox} at the specified line and focus it.
+ *
+ * @param side which side the draft will appear on.
+ * @param line the line the draft will be at. Lines are 1-based. Line 0 is a
+ * special case creating a file level comment.
+ */
+ void insertNewDraft(DisplaySide side, int line) {
+ if (line == 0) {
+ host.getSkipManager().ensureFirstLineIsVisible();
+ }
+
+ CommentGroup group = group(side, line);
+ if (0 < group.getBoxCount()) {
+ CommentBox last = group.getCommentBox(group.getBoxCount() - 1);
+ if (last instanceof DraftBox) {
+ ((DraftBox)last).setEdit(true);
+ } else {
+ ((PublishedBox)last).doReply();
+ }
+ } else {
+ addDraftBox(side, CommentInfo.create(
+ path,
+ getStoredSideFromDisplaySide(side),
+ line,
+ null)).setEdit(true);
+ }
+ }
+
+ DraftBox addDraftBox(DisplaySide side, CommentInfo info) {
+ CommentGroup group = group(side, info.line());
+ DraftBox box = new DraftBox(
+ group,
+ commentLinkProcessor,
+ getPatchSetIdFromSide(side),
+ info);
+
+ if (info.in_reply_to() != null) {
+ PublishedBox r = published.get(info.in_reply_to());
+ if (r != null) {
+ r.setReplyBox(box);
+ }
+ }
+
+ group.add(box);
+ box.setGutterWrapper(host.diffTable.sidePanel.addGutter(
+ host.getCmFromSide(side),
+ Math.max(0, info.line() - 1),
+ SidePanel.GutterType.DRAFT));
+ return box;
+ }
+
private DisplaySide displaySide(CommentInfo info, DisplaySide forSide) {
if (info.side() == Side.PARENT) {
return base == null ? DisplaySide.A : null;
@@ -161,15 +232,21 @@
}
List<SkippedLine> splitSkips(int context, List<SkippedLine> skips) {
+ if (sideB.containsKey(0)) {
+ // Special case of file comment; cannot skip first line.
+ for (SkippedLine skip : skips) {
+ if (skip.getStartB() == 0) {
+ skip.incrementStart(1);
+ }
+ }
+ }
+
// TODO: This is not optimal, but shouldn't be too costly in most cases.
// Maybe rewrite after done keeping track of diff chunk positions.
- for (CommentBox box : lineActiveBox.values()) {
- int boxLine = box.getCommentInfo().line();
- boolean sideA = box.getCm().side() == DisplaySide.A;
-
+ for (int boxLine : sideB.tailMap(1).keySet()) {
List<SkippedLine> temp = new ArrayList<SkippedLine>(skips.size() + 2);
for (SkippedLine skip : skips) {
- int startLine = sideA ? skip.getStartA() : skip.getStartB();
+ int startLine = skip.getStartB();
int deltaBefore = boxLine - startLine;
int deltaAfter = startLine + skip.getSize() - boxLine;
if (deltaBefore < -context || deltaAfter < -context) {
@@ -203,36 +280,33 @@
}
}
+ void clearLine(DisplaySide side, int line) {
+ map(side).remove(line);
+ }
+
Runnable toggleOpenBox(final CodeMirror cm) {
return new Runnable() {
public void run() {
- CommentBox box = lineActiveBox.get(cm.getActiveLine());
- if (box != null) {
- box.setOpen(!box.isOpen());
+ if (cm.hasActiveLine()) {
+ CommentGroup w = map(cm.side()).get(
+ cm.getLineNumber(cm.getActiveLine()) + 1);
+ if (w != null) {
+ w.openCloseLast();
+ }
}
}
};
}
- Runnable openClosePublished(final CodeMirror cm) {
+ Runnable openCloseAll(final CodeMirror cm) {
return new Runnable() {
@Override
public void run() {
if (cm.hasActiveLine()) {
- List<PublishedBox> list =
- linePublishedBoxes.get(cm.getActiveLine());
- if (list == null) {
- return;
- }
- boolean open = false;
- for (PublishedBox box : list) {
- if (!box.isOpen()) {
- open = true;
- break;
- }
- }
- for (PublishedBox box : list) {
- box.setOpen(open);
+ CommentGroup w = map(cm.side()).get(
+ cm.getLineNumber(cm.getActiveLine()) + 1);
+ if (w != null) {
+ w.openCloseAll();
}
}
}
@@ -244,156 +318,38 @@
return new Runnable() {
@Override
public void run() {
- Gerrit.doSignIn(host.getToken());
+ String token = host.getToken();
+ if (cm.hasActiveLine()) {
+ LineHandle handle = cm.getActiveLine();
+ int line = cm.getLineNumber(handle) + 1;
+ token += "@" + (cm.side() == DisplaySide.A ? "a" : "") + line;
+ }
+ Gerrit.doSignIn(token);
}
};
}
return new Runnable() {
public void run() {
- LineHandle handle = cm.getActiveLine();
- int line = cm.getLineNumber(handle);
- CommentBox box = lineActiveBox.get(handle);
- FromTo fromTo = cm.getSelectedRange();
- if (cm.somethingSelected()) {
- lineActiveBox.put(handle,
- newRangeDraft(cm, line, fromTo.getTo().getLine() == line ? fromTo : null));
- cm.setSelection(cm.getCursor());
- } else if (box == null) {
- lineActiveBox.put(handle, newRangeDraft(cm, line, null));
- } else if (box instanceof DraftBox) {
- ((DraftBox) box).setEdit(true);
- } else {
- ((PublishedBox) box).doReply();
+ if (cm.hasActiveLine()) {
+ newDraft(cm);
}
}
};
}
- private DraftBox newRangeDraft(CodeMirror cm, int line, FromTo fromTo) {
- DisplaySide side = cm.side();
- return addDraftBox(CommentInfo.createRange(
- path,
- getStoredSideFromDisplaySide(side),
- line + 1,
- null,
- null,
- CommentRange.create(fromTo)), side);
- }
-
- DraftBox newFileDraft(DisplaySide side) {
- return addDraftBox(CommentInfo.createFile(
- path,
- getStoredSideFromDisplaySide(side),
- null, null), side);
- }
-
- CommentInfo createReply(CommentInfo replyTo) {
- if (!replyTo.has_line() && replyTo.range() == null) {
- return CommentInfo.createFile(path, replyTo.side(), replyTo.id(), null);
+ private void newDraft(CodeMirror cm) {
+ int line = cm.getLineNumber(cm.getActiveLine()) + 1;
+ if (cm.somethingSelected()) {
+ FromTo fromTo = cm.getSelectedRange();
+ addDraftBox(cm.side(), CommentInfo.create(
+ path,
+ getStoredSideFromDisplaySide(cm.side()),
+ line,
+ CommentRange.create(fromTo))).setEdit(true);
+ cm.setSelection(cm.getCursor());
} else {
- return CommentInfo.createRange(path, replyTo.side(), replyTo.line(),
- replyTo.id(), null, replyTo.range());
- }
- }
-
- DraftBox addDraftBox(CommentInfo info, DisplaySide side) {
- CodeMirror cm = host.getCmFromSide(side);
- final DraftBox box = new DraftBox(this, cm, commentLinkProcessor,
- getPatchSetIdFromSide(side), info);
- if (info.id() == null) {
- Scheduler.get().scheduleDeferred(new ScheduledCommand() {
- @Override
- public void execute() {
- box.setOpen(true);
- box.setEdit(true);
- }
- });
- }
- if (!info.has_line()) {
- return box;
- }
- addCommentBox(info, box);
- box.setVisible(true);
- LineHandle handle = cm.getLineHandle(info.line() - 1);
- lineActiveBox.put(handle, box);
- return box;
- }
-
- private CommentBox addCommentBox(CommentInfo info, final CommentBox box) {
- host.diffTable.add(box);
- CodeMirror cm = box.getCm();
- CodeMirror other = host.otherCm(cm);
- int line = info.line() - 1; // CommentInfo is 1-based, but CM is 0-based
- LineHandle handle = cm.getLineHandle(line);
- PaddingManager manager;
- if (linePaddingManager.containsKey(handle)) {
- manager = linePaddingManager.get(handle);
- } else {
- // Estimated height at 28px, fixed by deferring after display
- manager = new PaddingManager(host.addPaddingWidget(cm, line, 0, Unit.PX, 0));
- linePaddingManager.put(handle, manager);
- }
-
- int lineToPad = host.lineOnOther(cm.side(), line).getLine();
- LineHandle otherHandle = other.getLineHandle(lineToPad);
- ChunkManager chunkMgr = host.getChunkManager();
- DiffChunkInfo myChunk = chunkMgr.getDiffChunk(cm.side(), line);
- DiffChunkInfo otherChunk = chunkMgr.getDiffChunk(other.side(), lineToPad);
- PaddingManager otherManager;
- if (linePaddingManager.containsKey(otherHandle)) {
- otherManager = linePaddingManager.get(otherHandle);
- } else {
- otherManager =
- new PaddingManager(host.addPaddingWidget(other, lineToPad, 0, Unit.PX, 0));
- linePaddingManager.put(otherHandle, otherManager);
- }
- if ((myChunk == null && otherChunk == null) || (myChunk != null && otherChunk != null)) {
- PaddingManager.link(manager, otherManager);
- }
-
- int index = manager.getCurrentCount();
- manager.insert(box, index);
- Configuration config = Configuration.create()
- .set("coverGutter", true)
- .set("insertAt", index)
- .set("noHScroll", true);
- LineWidget boxWidget = host.addLineWidget(cm, line, box, config);
- box.setPaddingManager(manager);
- box.setSelfWidgetWrapper(new PaddingWidgetWrapper(boxWidget, box.getElement()));
- if (otherChunk == null) {
- box.setDiffChunkInfo(myChunk);
- }
- box.setGutterWrapper(host.diffTable.sidePanel.addGutter(cm, info.line() - 1,
- box instanceof DraftBox
- ? SidePanel.GutterType.DRAFT
- : SidePanel.GutterType.COMMENT));
- return box;
- }
-
- void removeDraft(DraftBox box) {
- int line = box.getCommentInfo().line() - 1;
- LineHandle handle = box.getCm().getLineHandle(line);
- lineActiveBox.remove(handle);
- if (linePublishedBoxes.containsKey(handle)) {
- List<PublishedBox> list = linePublishedBoxes.get(handle);
- lineActiveBox.put(handle, list.get(list.size() - 1));
- }
- unsavedDrafts.remove(box);
- }
-
- void addFileCommentBox(CommentBox box) {
- host.diffTable.addFileCommentBox(box);
- }
-
- void removeFileCommentBox(DraftBox box) {
- host.diffTable.onRemoveDraftBox(box);
- }
-
- void resizePadding(LineHandle handle) {
- CommentBox box = lineActiveBox.get(handle);
- if (box != null) {
- box.resizePaddingWidget();
+ insertNewDraft(cm.side(), line);
}
}
@@ -411,6 +367,47 @@
}
}
+ private CommentGroup group(DisplaySide side, int line) {
+ CommentGroup w = map(side).get(line);
+ if (w != null) {
+ return w;
+ }
+
+ int lineA, lineB;
+ if (line == 0) {
+ lineA = lineB = 0;
+ } else if (side == DisplaySide.A) {
+ lineA = line;
+ lineB = host.lineOnOther(side, line - 1).getLine() + 1;
+ } else {
+ lineA = host.lineOnOther(side, line - 1).getLine() + 1;
+ lineB = line;
+ }
+
+ CommentGroup a = newGroup(DisplaySide.A, lineA);
+ CommentGroup b = newGroup(DisplaySide.B, lineB);
+ CommentGroup.pair(a, b);
+
+ sideA.put(lineA, a);
+ sideB.put(lineB, b);
+
+ if (attached) {
+ a.attach(host.diffTable);
+ b.attach(host.diffTable);
+ b.handleRedraw();
+ }
+
+ return side == DisplaySide.A ? a : b;
+ }
+
+ private CommentGroup newGroup(DisplaySide side, int line) {
+ return new CommentGroup(this, host.getCmFromSide(side), line);
+ }
+
+ private SortedMap<Integer, CommentGroup> map(DisplaySide side) {
+ return side == DisplaySide.A ? sideA : sideB;
+ }
+
private Side getStoredSideFromDisplaySide(DisplaySide side) {
return side == DisplaySide.A && base == null ? Side.PARENT : Side.REVISION;
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentsCollections.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentsCollections.java
index 587c162..1fe85b3 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentsCollections.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentsCollections.java
@@ -89,8 +89,11 @@
};
}
- private static JsArray<CommentInfo> sort(JsArray<CommentInfo> in) {
+ private JsArray<CommentInfo> sort(JsArray<CommentInfo> in) {
if (in != null) {
+ for (CommentInfo c : Natives.asList(in)) {
+ c.path(path);
+ }
Collections.sort(Natives.asList(in), new Comparator<CommentInfo>() {
@Override
public int compare(CommentInfo a, CommentInfo b) {
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.java
index 3fb8c32..695f2b5 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.java
@@ -24,6 +24,7 @@
import com.google.gwt.uibinder.client.UiBinder;
import com.google.gwt.uibinder.client.UiField;
import com.google.gwt.user.client.ui.Composite;
+import com.google.gwt.user.client.ui.FlowPanel;
import com.google.gwt.user.client.ui.HTMLPanel;
import com.google.gwt.user.client.ui.UIObject;
import com.google.gwt.user.client.ui.Widget;
@@ -50,23 +51,14 @@
String padding();
}
- @UiField
- Element cmA;
-
- @UiField
- Element cmB;
-
- @UiField
- SidePanel sidePanel;
-
- @UiField
- Element patchSetNavRow;
-
- @UiField
- Element patchSetNavCellA;
-
- @UiField
- Element patchSetNavCellB;
+ @UiField Element cmA;
+ @UiField Element cmB;
+ @UiField SidePanel sidePanel;
+ @UiField Element patchSetNavRow;
+ @UiField Element patchSetNavCellA;
+ @UiField Element patchSetNavCellB;
+ @UiField FlowPanel widgets;
+ @UiField static DiffTableStyle style;
@UiField(provided = true)
PatchSetSelectBox2 patchSetSelectBoxA;
@@ -74,36 +66,16 @@
@UiField(provided = true)
PatchSetSelectBox2 patchSetSelectBoxB;
- @UiField
- Element fileCommentRow;
-
- @UiField
- Element fileCommentCellA;
-
- @UiField
- Element fileCommentCellB;
-
- @UiField(provided = true)
- FileCommentPanel fileCommentPanelA;
-
- @UiField(provided = true)
- FileCommentPanel fileCommentPanelB;
-
- @UiField
- static DiffTableStyle style;
-
private SideBySide2 parent;
private boolean headerVisible;
DiffTable(SideBySide2 parent, PatchSet.Id base, PatchSet.Id revision, String path) {
patchSetSelectBoxA = new PatchSetSelectBox2(
- this, DisplaySide.A, revision.getParentKey(), base, path);
+ parent, DisplaySide.A, revision.getParentKey(), base, path);
patchSetSelectBoxB = new PatchSetSelectBox2(
- this, DisplaySide.B, revision.getParentKey(), revision, path);
+ parent, DisplaySide.B, revision.getParentKey(), revision, path);
PatchSetSelectBox2.link(patchSetSelectBoxA, patchSetSelectBoxB);
- fileCommentPanelA = new FileCommentPanel(parent, this, DisplaySide.A);
- fileCommentPanelB = new FileCommentPanel(parent, this, DisplaySide.B);
initWidget(uiBinder.createAndBindUi(this));
this.parent = parent;
this.headerVisible = true;
@@ -117,9 +89,6 @@
headerVisible = show;
Gerrit.setHeaderVisible(show && !parent.getPrefs().hideTopMenu());
UIObject.setVisible(patchSetNavRow, show);
- UIObject.setVisible(fileCommentRow, show
- && (fileCommentPanelA.getBoxCount() > 0
- || fileCommentPanelB.getBoxCount() > 0));
if (show) {
parent.header.removeStyleName(style.fullscreen());
} else {
@@ -128,25 +97,8 @@
parent.resizeCodeMirror();
}
- private FileCommentPanel getPanelFromSide(DisplaySide side) {
- return side == DisplaySide.A ? fileCommentPanelA : fileCommentPanelB;
- }
-
- void createOrEditFileComment(DisplaySide side) {
- getPanelFromSide(side).createOrEditFileComment();
- setHeaderVisible(true);
- }
-
- void addFileCommentBox(CommentBox box) {
- getPanelFromSide(box.getCm().side()).addFileComment(box);
- }
-
- void onRemoveDraftBox(DraftBox box) {
- getPanelFromSide(box.getCm().side()).onRemoveDraftBox(box);
- }
-
int getHeaderHeight() {
- return fileCommentRow.getOffsetHeight() + patchSetSelectBoxA.getOffsetHeight();
+ return patchSetSelectBoxA.getOffsetHeight();
}
void setUpPatchSetNav(JsArray<RevisionInfo> list, DiffInfo info) {
@@ -155,6 +107,6 @@
}
void add(Widget widget) {
- ((HTMLPanel) getWidget()).add(widget);
+ widgets.add(widget);
}
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.ui.xml
index 06dfd3f..3cee248 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.ui.xml
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.ui.xml
@@ -44,14 +44,19 @@
.difftable .CodeMirror-lines { padding: 0; }
.difftable .CodeMirror pre {
padding: 0;
- padding-bottom: 0.11em;
overflow: hidden;
border-right: 0;
width: auto;
}
+
+ /* Preserve space for underscores. If this changes
+ * see ChunkManager.addPadding() and adjust there.
+ */
+ .difftable .CodeMirror pre,
.difftable .CodeMirror pre span {
- padding-bottom: 0.11em;
+ padding-bottom: 1px;
}
+
.contentCell {
padding: 0;
}
@@ -144,14 +149,6 @@
<d:PatchSetSelectBox2 ui:field='patchSetSelectBoxB' />
</td>
</tr>
- <tr ui:field='fileCommentRow' class='{style.fileCommentRow}'>
- <td ui:field='fileCommentCellA' class='{style.fileCommentCell}'>
- <d:FileCommentPanel ui:field='fileCommentPanelA' />
- </td>
- <td ui:field='fileCommentCellB' class='{style.fileCommentCell}'>
- <d:FileCommentPanel ui:field='fileCommentPanelB' />
- </td>
- </tr>
<tr>
<td ui:field='cmA' class='{style.a}'></td>
<td ui:field='cmB' class='{style.b}'></td>
@@ -161,5 +158,6 @@
<td class='{style.sidePanelCell}'><d:SidePanel ui:field='sidePanel'/></td>
</tr>
</table>
+ <g:FlowPanel ui:field='widgets' visible='false'/>
</g:HTMLPanel>
</ui:UiBinder>
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DraftBox.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DraftBox.java
index 711cab0..c618bbe 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DraftBox.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DraftBox.java
@@ -17,7 +17,6 @@
import com.google.gerrit.client.FormatUtil;
import com.google.gerrit.client.changes.CommentApi;
import com.google.gerrit.client.changes.CommentInfo;
-import com.google.gerrit.client.changes.CommentInput;
import com.google.gerrit.client.rpc.CallbackGroup;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.ui.CommentLinkProcessor;
@@ -27,6 +26,7 @@
import com.google.gwt.core.client.Scheduler;
import com.google.gwt.core.client.Scheduler.RepeatingCommand;
import com.google.gwt.dom.client.Element;
+import com.google.gwt.event.dom.client.BlurEvent;
import com.google.gwt.event.dom.client.ClickEvent;
import com.google.gwt.event.dom.client.ClickHandler;
import com.google.gwt.event.dom.client.DoubleClickEvent;
@@ -35,6 +35,8 @@
import com.google.gwt.event.dom.client.KeyDownEvent;
import com.google.gwt.event.dom.client.MouseMoveEvent;
import com.google.gwt.event.dom.client.MouseMoveHandler;
+import com.google.gwt.event.dom.client.MouseUpEvent;
+import com.google.gwt.event.dom.client.MouseUpHandler;
import com.google.gwt.uibinder.client.UiBinder;
import com.google.gwt.uibinder.client.UiField;
import com.google.gwt.uibinder.client.UiHandler;
@@ -47,8 +49,6 @@
import com.google.gwtexpui.globalkey.client.NpTextArea;
import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder;
-import net.codemirror.lib.CodeMirror;
-
/** An HtmlPanel for displaying and editing a draft */
class DraftBox extends CommentBox {
interface Binder extends UiBinder<HTMLPanel, DraftBox> {}
@@ -62,6 +62,8 @@
private CommentInfo comment;
private PublishedBox replyToBox;
private Timer expandTimer;
+ private Timer resizeTimer;
+ private int editAreaHeight;
private boolean autoClosed;
@UiField Widget header;
@@ -80,12 +82,11 @@
@UiField Button discard2;
DraftBox(
- CommentManager manager,
- CodeMirror cm,
+ CommentGroup group,
CommentLinkProcessor clp,
PatchSet.Id id,
CommentInfo info) {
- super(manager, cm, info);
+ super(group, info.range());
linkProcessor = clp;
psId = id;
@@ -112,6 +113,7 @@
}
}
}, ClickEvent.getType());
+
addDomHandler(new DoubleClickHandler() {
@Override
public void onDoubleClick(DoubleClickEvent event) {
@@ -123,12 +125,8 @@
}
}
}, DoubleClickEvent.getType());
- addDomHandler(new MouseMoveHandler() {
- @Override
- public void onMouseMove(MouseMoveEvent event) {
- resizePaddingWidget();
- }
- }, MouseMoveEvent.getType());
+
+ initResizeHandler();
}
private void set(CommentInfo info) {
@@ -170,7 +168,8 @@
if (editArea.getVisibleLines() != rows) {
editArea.setVisibleLines(rows);
}
- resizePaddingWidget();
+ editAreaHeight = editArea.getOffsetHeight();
+ getCommentGroup().resize();
}
boolean isEdit() {
@@ -191,6 +190,7 @@
editArea.setFocus(true);
cancel.setVisible(!isNew());
expandText();
+ editAreaHeight = editArea.getOffsetHeight();
if (msg.length() > 0) {
Scheduler.get().scheduleFixedDelay(new RepeatingCommand() {
@Override
@@ -202,18 +202,24 @@
}
} else {
expandTimer.cancel();
+ resizeTimer.cancel();
}
getCommentManager().setUnsaved(this, edit);
- resizePaddingWidget();
+ getCommentGroup().resize();
}
- void registerReplyToBox(PublishedBox box) {
+ PublishedBox getReplyToBox() {
+ return replyToBox;
+ }
+
+ void setReplyToBox(PublishedBox box) {
replyToBox = box;
}
@Override
protected void onUnload() {
expandTimer.cancel();
+ resizeTimer.cancel();
super.onUnload();
}
@@ -221,20 +227,13 @@
if (replyToBox != null) {
replyToBox.unregisterReplyBox();
}
- clearRange();
+
+ getCommentManager().setUnsaved(this, false);
setRangeHighlight(false);
- removeFromParent();
- if (!getCommentInfo().has_line()) {
- getCommentManager().removeFileCommentBox(this);
- return;
- }
- PaddingManager manager = getPaddingManager();
- manager.remove(this);
- getCommentManager().removeDraft(this);
- getCm().focus();
- getSelfWidgetWrapper().getWidget().clear();
+ clearRange();
getGutterWrapper().remove();
- resizePaddingWidget();
+ getCommentGroup().remove(this);
+ getCm().focus();
}
@UiHandler("message")
@@ -265,9 +264,8 @@
return;
}
- CommentInfo original = comment;
- CommentInput input = CommentInput.create(original);
- input.setMessage(message);
+ CommentInfo input = CommentInfo.copy(comment);
+ input.message(message);
enableEdit(false);
GerritCallback<CommentInfo> cb = new GerritCallback<CommentInfo>() {
@@ -288,11 +286,11 @@
super.onFailure(e);
}
};
- if (original.id() == null) {
+ if (input.id() == null) {
CommentApi.createDraft(psId, input, group == null ? cb : group.add(cb));
} else {
CommentApi.updateDraft(
- psId, original.id(), input, group == null ? cb : group.add(cb));
+ psId, input.id(), input, group == null ? cb : group.add(cb));
}
getCm().focus();
}
@@ -337,6 +335,7 @@
@UiHandler("editArea")
void onKeyDown(KeyDownEvent e) {
+ resizeTimer.cancel();
if ((e.isControlKeyDown() || e.isMetaKeyDown())
&& !e.isAltKeyDown() && !e.isShiftKeyDown()) {
switch (e.getNativeKeyCode()) {
@@ -362,6 +361,40 @@
expandTimer.schedule(250);
}
+ @UiHandler("editArea")
+ void onBlur(BlurEvent e) {
+ resizeTimer.cancel();
+ }
+
+ private void initResizeHandler() {
+ resizeTimer = new Timer() {
+ @Override
+ public void run() {
+ getCommentGroup().resize();
+ }
+ };
+
+ addDomHandler(new MouseMoveHandler() {
+ @Override
+ public void onMouseMove(MouseMoveEvent event) {
+ int h = editArea.getOffsetHeight();
+ if (isEdit() && h != editAreaHeight) {
+ getCommentGroup().resize();
+ resizeTimer.scheduleRepeating(50);
+ editAreaHeight = h;
+ }
+ }
+ }, MouseMoveEvent.getType());
+
+ addDomHandler(new MouseUpHandler() {
+ @Override
+ public void onMouseUp(MouseUpEvent event) {
+ resizeTimer.cancel();
+ getCommentGroup().resize();
+ }
+ }, MouseUpEvent.getType());
+ }
+
private boolean isNew() {
return comment.id() == null;
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DraftBox.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DraftBox.ui.xml
index 52ef0ff..0c97e8b 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DraftBox.ui.xml
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DraftBox.ui.xml
@@ -49,7 +49,7 @@
<div ui:field='date' class='{res.style.date}'/>
</g:HTMLPanel>
<div ui:field='p_view' aria-hidden='true' style='display: NONE'>
- <g:HTML ui:field='message' styleName=''/>
+ <g:HTML ui:field='message' styleName='{res.style.message}'/>
<div style='position: relative'>
<g:Button ui:field='edit'
title='Edit this draft comment'
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/FileCommentPanel.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/FileCommentPanel.java
deleted file mode 100644
index 6282855..0000000
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/FileCommentPanel.java
+++ /dev/null
@@ -1,76 +0,0 @@
-//Copyright (C) 2013 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.client.diff;
-
-import com.google.gerrit.client.Gerrit;
-import com.google.gwt.user.client.ui.Composite;
-import com.google.gwt.user.client.ui.FlowPanel;
-
-import java.util.ArrayList;
-import java.util.List;
-
-/**
- * HTMLPanel to hold file comments.
- * TODO: Need to resize CodeMirror if this is resized since we don't have the
- * system scrollbar.
- */
-class FileCommentPanel extends Composite {
- private final SideBySide2 parent;
- private DiffTable table;
- private DisplaySide side;
- private List<CommentBox> boxes;
- private FlowPanel body;
-
- FileCommentPanel(SideBySide2 host, DiffTable table, DisplaySide side) {
- this.parent = host;
- this.table = table;
- this.side = side;
- boxes = new ArrayList<CommentBox>();
- initWidget(body = new FlowPanel());
- }
-
- void createOrEditFileComment() {
- if (!Gerrit.isSignedIn()) {
- Gerrit.doSignIn(parent.getToken());
- return;
- }
-
- if (boxes.isEmpty()) {
- addFileComment(parent.getCommentManager().newFileDraft(side));
- } else {
- CommentBox box = boxes.get(boxes.size() - 1);
- if (box instanceof DraftBox) {
- ((DraftBox) box).setEdit(true);
- } else {
- addFileComment(((PublishedBox) box).addReplyBox());
- }
- }
- }
-
- int getBoxCount() {
- return boxes.size();
- }
-
- void addFileComment(CommentBox box) {
- boxes.add(box);
- body.add(box);
- table.setHeaderVisible(true);
- }
-
- void onRemoveDraftBox(DraftBox box) {
- boxes.remove(box);
- table.setHeaderVisible(true);
- }
-}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.java
index 9cfa04d..46a7af7 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.java
@@ -310,6 +310,18 @@
}
}
+ Runnable reviewedAndNext() {
+ return new Runnable() {
+ @Override
+ public void run() {
+ if (Gerrit.isSignedIn()) {
+ reviewed.setValue(true, true);
+ }
+ navigate(Direction.NEXT).run();
+ }
+ };
+ }
+
String getNextPath() {
return nextPath;
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/LineMapper.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/LineMapper.java
index 968b1f6..31b6baa 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/LineMapper.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/LineMapper.java
@@ -52,11 +52,9 @@
void appendReplace(int aLen, int bLen) {
appendCommon(Math.min(aLen, bLen));
if (aLen < bLen) { // Edit with insertion
- int insertCnt = bLen - aLen;
- appendInsert(insertCnt);
+ appendInsert(bLen - aLen);
} else if (aLen > bLen) { // Edit with deletion
- int deleteCnt = aLen - bLen;
- appendDelete(deleteCnt);
+ appendDelete(aLen - bLen);
}
}
@@ -114,7 +112,7 @@
* ...
*/
LineOnOtherInfo lineOnOther(DisplaySide mySide, int line) {
- List<LineGap> lineGaps = mySide == DisplaySide.A ? lineMapAtoB : lineMapBtoA;
+ List<LineGap> lineGaps = gapList(mySide);
// Create a dummy LineGap for the search.
int ret = Collections.binarySearch(lineGaps, new LineGap(line));
if (ret == -1) {
@@ -132,6 +130,38 @@
}
}
+ AlignedPair align(DisplaySide mySide, int line) {
+ List<LineGap> gaps = gapList(mySide);
+ int idx = Collections.binarySearch(gaps, new LineGap(line));
+ if (idx == -1) {
+ return new AlignedPair(line, line);
+ }
+
+ LineGap g = gaps.get(0 <= idx ? idx : -idx - 2);
+ if (g.start <= line && line <= g.end && g.end != -1) {
+ if (0 < g.start) {
+ // Line falls within this gap, use alignment before.
+ return new AlignedPair(g.start - 1, g.end + g.delta);
+ }
+ return new AlignedPair(g.end, g.end + g.delta + 1);
+ }
+ return new AlignedPair(line, line + g.delta);
+ }
+
+ private List<LineGap> gapList(DisplaySide mySide) {
+ return mySide == DisplaySide.A ? lineMapAtoB : lineMapBtoA;
+ }
+
+ static class AlignedPair {
+ final int src;
+ final int dst;
+
+ AlignedPair(int s, int d) {
+ src = s;
+ dst = d;
+ }
+ }
+
/**
* @field line The line number on the other side.
* @field aligned Whether the two lines are at the same height when displayed.
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PaddingManager.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PaddingManager.java
deleted file mode 100644
index 3ff7c99..0000000
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PaddingManager.java
+++ /dev/null
@@ -1,170 +0,0 @@
-// Copyright (C) 2013 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.client.diff;
-
-import com.google.gwt.dom.client.Element;
-import com.google.gwt.dom.client.Style.Unit;
-
-import net.codemirror.lib.LineWidget;
-
-import java.util.ArrayList;
-import java.util.List;
-
-/**
- * Manages paddings for CommentBoxes. Each line that may need to be padded owns
- * a PaddingManager instance, which maintains a padding widget whose height
- * 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;
-
- 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);
- }
- }
-
- private int getMyTotalHeight() {
- int total = 0;
- for (CommentBox box : comments) {
- /**
- * This gets the height of CM's line widget div, taking the margin and
- * the horizontal scrollbar into account.
- */
- total += box.getSelfWidgetWrapper().getElement().getParentElement().getOffsetHeight();
- }
- 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) {
- wrapper.element.getStyle().setHeight((double) height, Unit.PX);
- wrapper.widget.changed();
- }
-
- void resizePaddingWidget() {
- if (others.isEmpty()) {
- return;
- }
- int myHeight = getGroupTotalHeight();
- int othersHeight = getOthersTotalHeight();
- int paddingNeeded = othersHeight - myHeight;
- if (paddingNeeded < 0) {
- for (PaddingManager manager : others.get(0).others) {
- manager.setPaddingHeight(0);
- }
- others.get(others.size() - 1).setPaddingHeight(-paddingNeeded);
- } else {
- setPaddingHeight(paddingNeeded);
- for (PaddingManager other : others) {
- other.setPaddingHeight(0);
- }
- }
- }
-
- /** This is unused now because threading info is ignored. */
- int getReplyIndex(CommentBox box) {
- return comments.indexOf(box) + 1;
- }
-
- int getCurrentCount() {
- return comments.size();
- }
-
- void insert(CommentBox box, int index) {
- comments.add(index, box);
- }
-
- void remove(CommentBox box) {
- comments.remove(box);
- }
-
- static class PaddingWidgetWrapper {
- private LineWidget widget;
- private Element element;
-
- PaddingWidgetWrapper(LineWidget w, Element e) {
- widget = w;
- element = e;
- }
-
- LineWidget getWidget() {
- return widget;
- }
-
- Element getElement() {
- return element;
- }
- }
-
- static class LinePaddingWidgetWrapper extends PaddingWidgetWrapper {
- private int chunkLength;
- private int otherLine;
-
- LinePaddingWidgetWrapper(PaddingWidgetWrapper pair, int otherLine, int chunkLength) {
- super(pair.widget, pair.element);
-
- this.otherLine = otherLine;
- this.chunkLength = chunkLength;
- }
-
- int getChunkLength() {
- return chunkLength;
- }
-
- int getOtherLine() {
- return otherLine;
- }
- }
-}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSetSelectBox2.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSetSelectBox2.java
index aca4122..7154c9f 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSetSelectBox2.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSetSelectBox2.java
@@ -49,7 +49,7 @@
@UiField HTMLPanel linkPanel;
@UiField BoxStyle style;
- private DiffTable table;
+ private SideBySide2 parent;
private DisplaySide side;
private boolean sideA;
private String path;
@@ -58,12 +58,16 @@
private PatchSet.Id idActive;
private PatchSetSelectBox2 other;
- PatchSetSelectBox2(DiffTable table, final DisplaySide side,
- final Change.Id changeId, final PatchSet.Id revision, String path) {
+ PatchSetSelectBox2(SideBySide2 parent,
+ DisplaySide side,
+ Change.Id changeId,
+ PatchSet.Id revision,
+ String path) {
initWidget(uiBinder.createAndBindUi(this));
icon.setTitle(PatchUtil.C.addFileCommentToolTip());
icon.addStyleName(Gerrit.RESOURCES.css().link());
- this.table = table;
+
+ this.parent = parent;
this.side = side;
this.sideA = side == DisplaySide.A;
this.changeId = changeId;
@@ -127,6 +131,7 @@
@UiHandler("icon")
void onIconClick(ClickEvent e) {
- table.createOrEditFileComment(side);
+ parent.getCmFromSide(side).scrollToY(0);
+ parent.getCommentManager().insertNewDraft(side, 0);
}
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PublishedBox.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PublishedBox.java
index 823e57a..4b95c6a 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PublishedBox.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PublishedBox.java
@@ -19,7 +19,6 @@
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.changes.CommentApi;
import com.google.gerrit.client.changes.CommentInfo;
-import com.google.gerrit.client.changes.CommentInput;
import com.google.gerrit.client.changes.Util;
import com.google.gerrit.client.patches.PatchUtil;
import com.google.gerrit.client.rpc.GerritCallback;
@@ -39,8 +38,6 @@
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> {}
@@ -68,12 +65,11 @@
AvatarImage avatar;
PublishedBox(
- CommentManager manager,
- CodeMirror cm,
+ CommentGroup group,
CommentLinkProcessor clp,
PatchSet.Id psId,
CommentInfo info) {
- super(manager, cm, info);
+ super(group, info.range());
this.psId = psId;
this.comment = info;
@@ -125,9 +121,9 @@
super.setOpen(open);
}
- void registerReplyBox(DraftBox box) {
+ void setReplyBox(DraftBox box) {
replyBox = box;
- box.registerReplyToBox(this);
+ box.setReplyToBox(this);
}
void unregisterReplyBox() {
@@ -139,21 +135,17 @@
replyBox.setEdit(true);
}
- DraftBox addReplyBox() {
- DraftBox box = getCommentManager().addDraftBox(
- getCommentManager().createReply(comment), getCm().side());
- registerReplyBox(box);
- return box;
+ void addReplyBox() {
+ getCommentManager().addDraftBox(
+ getCm().side(),
+ CommentInfo.createReply(comment)).setEdit(true);
}
void doReply() {
if (!Gerrit.isSignedIn()) {
Gerrit.doSignIn(getCommentManager().getSideBySide2().getToken());
} else if (replyBox == null) {
- DraftBox box = addReplyBox();
- if (!getCommentInfo().has_line()) {
- getCommentManager().addFileCommentBox(box);
- }
+ addReplyBox();
} else {
openReplyBox();
}
@@ -172,19 +164,15 @@
Gerrit.doSignIn(getCommentManager().getSideBySide2().getToken());
} else if (replyBox == null) {
done.setEnabled(false);
- CommentInput input = CommentInput.create(getCommentManager().createReply(comment));
- input.setMessage(PatchUtil.C.cannedReplyDone());
+ CommentInfo input = CommentInfo.createReply(comment);
+ input.message(PatchUtil.C.cannedReplyDone());
CommentApi.createDraft(psId, input,
new GerritCallback<CommentInfo>() {
@Override
public void onSuccess(CommentInfo result) {
done.setEnabled(true);
setOpen(false);
- DraftBox box = getCommentManager().addDraftBox(result, getCm().side());
- registerReplyBox(box);
- if (!getCommentInfo().has_line()) {
- getCommentManager().addFileCommentBox(box);
- }
+ getCommentManager().addDraftBox(getCm().side(), result);
}
});
} else {
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PublishedBox.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PublishedBox.ui.xml
index f3beda1..6e88c5b 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PublishedBox.ui.xml
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PublishedBox.ui.xml
@@ -53,7 +53,8 @@
<div ui:field='summary' class='{res.style.summary}'/>
<div ui:field='date' class='{res.style.date}'/>
</g:HTMLPanel>
- <div ui:field='message' aria-hidden='true' style='display: NONE'/>
+ <div ui:field='message' class='{res.style.message}'
+ aria-hidden='true' style='display: NONE'/>
<div ui:field='buttons' aria-hidden='true' style='display: NONE'>
<g:Button ui:field='reply' styleName=''
title='Reply to this comment'>
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Resources.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Resources.java
index 5356f2c..8c4fc51 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Resources.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Resources.java
@@ -30,8 +30,10 @@
@Source("gear.png") ImageResource gear();
interface Style extends CssResource {
+ String commentWidgets();
String commentBox();
String contents();
+ String message();
String header();
String summary();
String date();
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ScrollSynchronizer.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ScrollSynchronizer.java
index 898f066..033b08b 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ScrollSynchronizer.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ScrollSynchronizer.java
@@ -14,11 +14,9 @@
package com.google.gerrit.client.diff;
-import com.google.gerrit.client.diff.LineMapper.LineOnOtherInfo;
import com.google.gwt.user.client.Timer;
import net.codemirror.lib.CodeMirror;
-import net.codemirror.lib.CodeMirror.Viewport;
import net.codemirror.lib.ScrollInfo;
class ScrollSynchronizer {
@@ -75,7 +73,7 @@
if (active == this) {
ScrollInfo si = src.getScrollInfo();
updateScreenHeader(si);
- dst.scrollTo(si.getLeft(), si.getTop());
+ dst.scrollTo(si.getLeft(), align(si.getTop()));
state = 0;
}
}
@@ -84,37 +82,32 @@
switch (state) {
case 0:
state = 1;
+ dst.scrollToY(align(src.getScrollInfo().getTop()));
break;
case 1:
state = 2;
- return;
+ break;
case 2:
active = null;
fixup.cancel();
- return;
+ break;
}
+ }
+ private double align(double srcTop) {
// Since CM doesn't always take the height of line widgets into
// account when calculating scrollInfo when scrolling too fast (e.g.
// throw scrolling), simply setting scrollTop to be the same doesn't
// guarantee alignment.
//
- // Iterate over the viewport to find the first line that isn't part of
- // an insertion or deletion gap, for which isAligned() will be true.
- // We then manually examine if the lines that should be aligned are at
- // the same height. If not, perform additional scrolling.
- Viewport fromTo = src.getViewport();
- for (int line = fromTo.getFrom(); line <= fromTo.getTo(); line++) {
- LineOnOtherInfo info = mapper.lineOnOther(srcSide, line);
- if (info.isAligned()) {
- double sy = src.heightAtLine(line);
- double dy = dst.heightAtLine(info.getLine());
- if (Math.abs(dy - sy) >= 1) {
- dst.scrollToY(dst.getScrollInfo().getTop() + (dy - sy));
- }
- break;
- }
- }
+ // Find a pair of lines that are aligned and near the top of
+ // the viewport. Use that distance to correct the Y coordinate.
+ int line = src.lineAtHeight(srcTop, "local");
+ LineMapper.AlignedPair p = mapper.align(srcSide, line);
+
+ double sy = src.heightAtLine(p.src, "local");
+ double dy = dst.heightAtLine(p.dst, "local");
+ return Math.max(0, dy + (srcTop - sy));
}
}
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java
index fb3c570..e20686c 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java
@@ -22,7 +22,6 @@
import com.google.gerrit.client.changes.ChangeInfo.RevisionInfo;
import com.google.gerrit.client.changes.ChangeList;
import com.google.gerrit.client.diff.LineMapper.LineOnOtherInfo;
-import com.google.gerrit.client.diff.PaddingManager.PaddingWidgetWrapper;
import com.google.gerrit.client.patches.PatchUtil;
import com.google.gerrit.client.projects.ConfigInfoCache;
import com.google.gerrit.client.rpc.CallbackGroup;
@@ -56,8 +55,6 @@
import com.google.gwt.user.client.rpc.AsyncCallback;
import com.google.gwt.user.client.ui.FlowPanel;
import com.google.gwt.user.client.ui.Image;
-import com.google.gwt.user.client.ui.SimplePanel;
-import com.google.gwt.user.client.ui.Widget;
import com.google.gwtexpui.globalkey.client.GlobalKey;
import com.google.gwtexpui.globalkey.client.KeyCommand;
import com.google.gwtexpui.globalkey.client.KeyCommandSet;
@@ -68,12 +65,10 @@
import net.codemirror.lib.CodeMirror.GutterClickHandler;
import net.codemirror.lib.CodeMirror.LineClassWhere;
import net.codemirror.lib.CodeMirror.LineHandle;
-import net.codemirror.lib.CodeMirror.RenderLineHandler;
import net.codemirror.lib.CodeMirror.Viewport;
import net.codemirror.lib.Configuration;
import net.codemirror.lib.KeyMap;
import net.codemirror.lib.LineCharacter;
-import net.codemirror.lib.LineWidget;
import net.codemirror.lib.ModeInjector;
import net.codemirror.lib.Rect;
@@ -112,7 +107,6 @@
private KeyCommandSet keysAction;
private KeyCommandSet keysComment;
private List<HandlerRegistration> handlers;
- private List<Runnable> deferred;
private PreferencesAction prefsAction;
private int reloadVersionId;
@@ -232,7 +226,7 @@
});
diffTable.sidePanel.adjustGutters(cmB);
- if (startSide == null && diff.meta_b() != null) {
+ if (startLine == 0 && diff.meta_b() != null) {
DiffChunkInfo d = chunkManager.getFirst();
if (d != null) {
startSide = d.getSide();
@@ -294,7 +288,6 @@
cm.on("beforeSelectionChange", onSelectionChange(cm));
cm.on("cursorActivity", updateActiveLine(cm));
cm.on("gutterClick", onGutterClick(cm));
- cm.on("renderLine", resizeLinePadding(cm.side()));
cm.on("viewportChange", adjustGutters(cm));
cm.on("focus", new Runnable() {
@Override
@@ -313,7 +306,10 @@
.on("'c'", commentManager.insertNewDraft(cm))
.on("N", maybeNextVimSearch(cm))
.on("P", chunkManager.diffChunkNav(cm, Direction.PREV))
- .on("Shift-O", commentManager.openClosePublished(cm))
+ .on("Shift-M", header.reviewedAndNext())
+ .on("Shift-N", commentManager.commentNav(cm, Direction.NEXT))
+ .on("Shift-P", commentManager.commentNav(cm, Direction.PREV))
+ .on("Shift-O", commentManager.openCloseAll(cm))
.on("Shift-Left", moveCursorToSide(cm, DisplaySide.A))
.on("Shift-Right", moveCursorToSide(cm, DisplaySide.B))
.on("'i'", new Runnable() {
@@ -412,6 +408,9 @@
keysNavigation.add(
new NoOpKeyCommand(0, 'n', PatchUtil.C.chunkNext2()),
new NoOpKeyCommand(0, 'p', PatchUtil.C.chunkPrev2()));
+ keysNavigation.add(
+ new NoOpKeyCommand(KeyCommand.M_SHIFT, 'n', PatchUtil.C.commentNext()),
+ new NoOpKeyCommand(KeyCommand.M_SHIFT, 'p', PatchUtil.C.commentPrev()));
keysAction = new KeyCommandSet(Gerrit.C.sectionActions());
keysAction.add(new NoOpKeyCommand(0, KeyCodes.KEY_ENTER,
@@ -425,6 +424,13 @@
header.toggleReviewed().run();
}
});
+ keysAction.add(new KeyCommand(
+ KeyCommand.M_SHIFT, 'm', PatchUtil.C.markAsReviewedAndGoToNext()) {
+ @Override
+ public void onKeyPress(KeyPressEvent event) {
+ header.reviewedAndNext().run();
+ }
+ });
keysAction.add(new KeyCommand(0, 'a', PatchUtil.C.openReply()) {
@Override
public void onKeyPress(KeyPressEvent event) {
@@ -486,10 +492,7 @@
cmB.setHeight(height);
render(diff);
- commentManager.render(comments);
- if (prefs.expandAllComments()) {
- commentManager.setExpandAllComments(true);
- }
+ commentManager.render(comments, prefs.expandAllComments());
skipManager.render(prefs.context(), diff);
}
});
@@ -620,42 +623,6 @@
return chunkManager.getLineMapper().lineOnOther(side, line);
}
- PaddingWidgetWrapper addPaddingWidget(CodeMirror cm,
- int line, double height, Unit unit, Integer index) {
- SimplePanel padding = new SimplePanel();
- padding.setStyleName(DiffTable.style.padding());
- padding.getElement().getStyle().setHeight(height, unit);
- Configuration config = Configuration.create()
- .set("coverGutter", true)
- .set("above", line == -1)
- .set("noHScroll", true);
- if (index != null) {
- config = config.set("insertAt", index);
- }
- LineWidget widget = addLineWidget(cm, line == -1 ? 0 : line, padding, config);
- return new PaddingWidgetWrapper(widget, padding.getElement());
- }
-
- /**
- * A LineWidget needs to be added to diffTable in order to respond to browser
- * events, but CodeMirror doesn't render the widget until the containing line
- * is scrolled into viewportMargin, causing it to appear at the bottom of the
- * DOM upon loading. Fix by hiding the widget until it is first scrolled into
- * view (when CodeMirror fires a "redraw" event on the widget).
- */
- LineWidget addLineWidget(CodeMirror cm, int line,
- final Widget widget, Configuration options) {
- widget.setVisible(false);
- LineWidget lineWidget = cm.addLineWidget(line, widget.getElement(), options);
- lineWidget.onFirstRedraw(new Runnable() {
- @Override
- public void run() {
- widget.setVisible(true);
- }
- });
- return lineWidget;
- }
-
private void clearActiveLine(CodeMirror cm) {
if (cm.hasActiveLine()) {
LineHandle activeLine = cm.getActiveLine();
@@ -684,34 +651,37 @@
final CodeMirror other = otherCm(cm);
return new Runnable() {
public void run() {
- /**
- * The rendering of active lines has to be deferred. Reflow
- * caused by adding and removing styles chokes Firefox when arrow
- * key (or j/k) is held down. Performance on Chrome is fine
- * without the deferral.
- */
- defer(new Runnable() {
+ // The rendering of active lines has to be deferred. Reflow
+ // caused by adding and removing styles chokes Firefox when arrow
+ // key (or j/k) is held down. Performance on Chrome is fine
+ // without the deferral.
+ //
+ Scheduler.get().scheduleDeferred(new ScheduledCommand() {
@Override
- public void run() {
- LineHandle handle = cm.getLineHandleVisualStart(
- cm.getCursor("end").getLine());
- if (cm.hasActiveLine() && cm.getActiveLine().equals(handle)) {
- return;
- }
+ public void execute() {
+ operation(new Runnable() {
+ public void run() {
+ LineHandle handle = cm.getLineHandleVisualStart(
+ cm.getCursor("end").getLine());
+ if (cm.hasActiveLine() && cm.getActiveLine().equals(handle)) {
+ return;
+ }
- clearActiveLine(cm);
- clearActiveLine(other);
- cm.setActiveLine(handle);
- cm.addLineClass(
- handle, LineClassWhere.WRAP, DiffTable.style.activeLine());
- LineOnOtherInfo info =
- lineOnOther(cm.side(), cm.getLineNumber(handle));
- if (info.isAligned()) {
- LineHandle oLineHandle = other.getLineHandle(info.getLine());
- other.setActiveLine(oLineHandle);
- other.addLineClass(oLineHandle, LineClassWhere.WRAP,
- DiffTable.style.activeLine());
- }
+ clearActiveLine(cm);
+ clearActiveLine(other);
+ cm.setActiveLine(handle);
+ cm.addLineClass(
+ handle, LineClassWhere.WRAP, DiffTable.style.activeLine());
+ LineOnOtherInfo info =
+ lineOnOther(cm.side(), cm.getLineNumber(handle));
+ if (info.isAligned()) {
+ LineHandle oLineHandle = other.getLineHandle(info.getLine());
+ other.setActiveLine(oLineHandle);
+ other.addLineClass(oLineHandle, LineClassWhere.WRAP,
+ DiffTable.style.activeLine());
+ }
+ }
+ });
}
});
}
@@ -798,39 +768,6 @@
};
}
-
- void defer(Runnable thunk) {
- if (deferred == null) {
- final ArrayList<Runnable> list = new ArrayList<Runnable>();
- deferred = list;
- Scheduler.get().scheduleDeferred(new ScheduledCommand() {
- @Override
- public void execute() {
- deferred = null;
- operation(new Runnable() {
- public void run() {
- for (Runnable thunk : list) {
- thunk.run();
- }
- }
- });
- }
- });
- }
- deferred.add(thunk);
- }
-
- // TODO: Maybe integrate this with PaddingManager.
- private RenderLineHandler resizeLinePadding(final DisplaySide side) {
- return new RenderLineHandler() {
- @Override
- public void handle(CodeMirror cm, LineHandle lh, Element e) {
- commentManager.resizePadding(lh);
- chunkManager.resizePadding(cm, lh, side);
- }
- };
- }
-
void resizeCodeMirror() {
int height = getCodeMirrorHeight();
cmA.setHeight(height);
@@ -873,6 +810,10 @@
return commentManager;
}
+ SkipManager getSkipManager() {
+ return skipManager;
+ }
+
void operation(final Runnable apply) {
cmA.operation(new Runnable() {
@Override
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SkipBar.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SkipBar.java
index ac644ce..6ab456a 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SkipBar.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SkipBar.java
@@ -52,6 +52,7 @@
private final SkipManager manager;
private final CodeMirror cm;
+ private int start;
private LineWidget lineWidget;
private TextMarker textMarker;
private SkipBar otherBar;
@@ -72,6 +73,14 @@
}, ClickEvent.getType());
}
+ int getStart() {
+ return start;
+ }
+
+ DisplaySide getSide() {
+ return cm.side();
+ }
+
void collapse(int start, int end, boolean attach) {
if (attach) {
boolean isNew = lineWidget == null;
@@ -84,16 +93,17 @@
lineWidget = cm.addLineWidget(start - 1, getElement(), cfg);
}
if (isNew) {
- setVisible(false);
lineWidget.onFirstRedraw(new Runnable() {
@Override
public void run() {
int w = cm.getGutterElement().getOffsetWidth();
getElement().getStyle().setPaddingLeft(w, Unit.PX);
- setVisible(true);
}
});
+ } else {
+ manager.move(this, this.start, start);
}
+ this.start = start;
}
textMarker = cm.markText(
@@ -124,22 +134,32 @@
lineWidget.clear();
}
- void expandAll() {
- clearMarkerAndWidget();
- removeFromParent();
- updateSelection();
+ void expandBefore(int cnt) {
+ expandSideBefore(cnt);
+ otherBar.expandSideBefore(cnt);
}
- private void expandBefore() {
+ private void expandSideBefore(int cnt) {
FromTo range = textMarker.find();
int oldStart = range.getFrom().getLine();
- int newStart = oldStart + NUM_ROWS_TO_EXPAND;
+ int newStart = oldStart + cnt;
int end = range.getTo().getLine();
clearMarkerAndWidget();
collapse(newStart, end, true);
updateSelection();
}
+ void expandAll() {
+ expandSideAll();
+ otherBar.expandSideAll();
+ }
+
+ private void expandSideAll() {
+ clearMarkerAndWidget();
+ removeFromParent();
+ updateSelection();
+ }
+
private void expandAfter() {
FromTo range = textMarker.find();
int start = range.getFrom().getLine();
@@ -159,16 +179,14 @@
@UiHandler("skipNum")
void onExpandAll(ClickEvent e) {
- manager.remove(this, otherBar);
- otherBar.expandAll();
expandAll();
+ manager.remove(this, otherBar);
cm.focus();
}
@UiHandler("upArrow")
void onExpandBefore(ClickEvent e) {
- otherBar.expandBefore();
- expandBefore();
+ expandBefore(NUM_ROWS_TO_EXPAND);
cm.focus();
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SkipManager.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SkipManager.java
index 575d8ee..e4be137 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SkipManager.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SkipManager.java
@@ -22,15 +22,16 @@
import net.codemirror.lib.CodeMirror;
import java.util.ArrayList;
-import java.util.HashSet;
import java.util.List;
-import java.util.Set;
+import java.util.SortedMap;
+import java.util.TreeMap;
/** Collapses common regions with {@link SkipBar} for {@link SideBySide2}. */
class SkipManager {
private final SideBySide2 host;
private final CommentManager commentManager;
- private Set<SkipBar> skipBars;
+ private SortedMap<Integer, SkipBar> sideA;
+ private SortedMap<Integer, SkipBar> sideB;
SkipManager(SideBySide2 host, CommentManager commentManager) {
this.host = host;
@@ -71,13 +72,15 @@
CodeMirror cmA = host.getCmFromSide(DisplaySide.A);
CodeMirror cmB = host.getCmFromSide(DisplaySide.B);
- skipBars = new HashSet<SkipBar>();
+ sideA = new TreeMap<Integer, SkipBar>();
+ sideB = new TreeMap<Integer, SkipBar>();
+
for (SkippedLine skip : skips) {
SkipBar barA = newSkipBar(cmA, DisplaySide.A, skip);
SkipBar barB = newSkipBar(cmB, DisplaySide.B, skip);
SkipBar.link(barA, barB);
- skipBars.add(barA);
- skipBars.add(barB);
+ sideA.put(barA.getStart(), barA);
+ sideB.put(barB.getStart(), barB);
if (skip.getStartA() == 0 || skip.getStartB() == 0) {
barA.upArrow.setVisible(false);
@@ -91,23 +94,41 @@
}
}
+ void ensureFirstLineIsVisible() {
+ if (sideB != null) {
+ SkipBar line0 = sideB.get(0);
+ if (line0 != null) {
+ line0.expandBefore(1);
+ }
+ }
+ }
+
void removeAll() {
- if (skipBars != null) {
- for (SkipBar bar : skipBars) {
+ if (sideB != null) {
+ for (SkipBar bar : sideB.values()) {
bar.expandAll();
}
- skipBars = null;
+ sideA = null;
+ sideB = null;
}
}
void remove(SkipBar a, SkipBar b) {
- skipBars.remove(a);
- skipBars.remove(b);
- if (skipBars.isEmpty()) {
- skipBars = null;
+ map(a.getSide()).remove(a.getStart());
+ map(b.getSide()).remove(b.getStart());
+
+ if (sideB.isEmpty()) {
+ sideA = null;
+ sideB = null;
}
}
+ void move(SkipBar bar, int oldLine, int newLine) {
+ SortedMap<Integer, SkipBar> map = map(bar.getSide());
+ map.remove(oldLine);
+ map.put(newLine, bar);
+ }
+
private SkipBar newSkipBar(CodeMirror cm, DisplaySide side, SkippedLine skip) {
int start = side == DisplaySide.A ? skip.getStartA() : skip.getStartB();
int end = start + skip.getSize() - 1;
@@ -117,4 +138,8 @@
bar.collapse(start, end, true);
return bar;
}
+
+ private SortedMap<Integer, SkipBar> map(DisplaySide side) {
+ return side == DisplaySide.A ? sideA : sideB;
+ }
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/AbstractPatchContentTable.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/AbstractPatchContentTable.java
index deaeed9..8f50b10 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/AbstractPatchContentTable.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/AbstractPatchContentTable.java
@@ -20,7 +20,6 @@
import com.google.gerrit.client.account.AccountInfo;
import com.google.gerrit.client.changes.CommentApi;
import com.google.gerrit.client.changes.CommentInfo;
-import com.google.gerrit.client.changes.CommentInput;
import com.google.gerrit.client.changes.PatchTable;
import com.google.gerrit.client.changes.Util;
import com.google.gerrit.client.rpc.GerritCallback;
@@ -941,7 +940,7 @@
if (p == null) {
enableButtons(false);
final PatchSet.Id psId = newComment.getKey().getParentKey().getParentKey();
- CommentInput in = CommentEditorPanel.toInput(newComment);
+ CommentInfo in = CommentEditorPanel.toInput(newComment);
CommentApi.createDraft(psId, in,
new GerritCallback<CommentInfo>() {
@Override
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/CommentEditorPanel.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/CommentEditorPanel.java
index 1bc2c42..32302f4 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/CommentEditorPanel.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/CommentEditorPanel.java
@@ -17,7 +17,6 @@
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.changes.CommentApi;
import com.google.gerrit.client.changes.CommentInfo;
-import com.google.gerrit.client.changes.CommentInput;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.ui.CommentLinkProcessor;
import com.google.gerrit.client.ui.CommentPanel;
@@ -268,7 +267,7 @@
onSave.onFailure(caught);
}
};
- CommentInput input = toInput(comment);
+ CommentInfo input = toInput(comment);
if (wasNew) {
CommentApi.createDraft(psId, input, cb);
} else {
@@ -336,16 +335,16 @@
return null;
}
- public static CommentInput toInput(PatchLineComment c) {
- CommentInput i = CommentInput.createObject().cast();
- i.setId(c.getKey().get());
- i.setPath(c.getKey().getParentKey().get());
- i.setSide(c.getSide() == 0 ? Side.PARENT : Side.REVISION);
+ public static CommentInfo toInput(PatchLineComment c) {
+ CommentInfo i = CommentInfo.createObject().cast();
+ i.id(c.getKey().get());
+ i.path(c.getKey().getParentKey().get());
+ i.side(c.getSide() == 0 ? Side.PARENT : Side.REVISION);
if (c.getLine() > 0) {
- i.setLine(c.getLine());
+ i.line(c.getLine());
}
- i.setInReplyTo(c.getParentUuid());
- i.setMessage(c.getMessage());
+ i.in_reply_to(c.getParentUuid());
+ i.message(c.getMessage());
return i;
}
diff --git a/gerrit-gwtui/src/main/java/net/codemirror/lib/CodeMirror.java b/gerrit-gwtui/src/main/java/net/codemirror/lib/CodeMirror.java
index 150978d..a1883cf 100644
--- a/gerrit-gwtui/src/main/java/net/codemirror/lib/CodeMirror.java
+++ b/gerrit-gwtui/src/main/java/net/codemirror/lib/CodeMirror.java
@@ -327,6 +327,9 @@
public static class Viewport extends JavaScriptObject {
public final native int getFrom() /*-{ return this.from; }-*/;
public final native int getTo() /*-{ return this.to; }-*/;
+ public final boolean contains(int line) {
+ return getFrom() <= line && line < getTo();
+ }
protected Viewport() {
}
diff --git a/gerrit-gwtui/src/test/java/com/google/gerrit/client/diff/LineMapperTest.java b/gerrit-gwtui/src/test/java/com/google/gerrit/client/diff/LineMapperTest.java
index 756e879..25c8270 100644
--- a/gerrit-gwtui/src/test/java/com/google/gerrit/client/diff/LineMapperTest.java
+++ b/gerrit-gwtui/src/test/java/com/google/gerrit/client/diff/LineMapperTest.java
@@ -102,4 +102,31 @@
assertEquals(new LineOnOtherInfo(10, true),
mapper.lineOnOther(DisplaySide.B, 0));
}
+
+ @Test
+ public void testReplaceWithInsertInB() {
+ // 0 c c
+ // 1 a b
+ // 2 a b
+ // 3 - b
+ // 4 - b
+ // 5 c c
+ LineMapper mapper = new LineMapper();
+ mapper.appendCommon(1);
+ mapper.appendReplace(2, 4);
+ mapper.appendCommon(1);
+
+ assertEquals(4, mapper.getLineA());
+ assertEquals(6, mapper.getLineB());
+
+ assertEquals(new LineOnOtherInfo(1, true),
+ mapper.lineOnOther(DisplaySide.B, 1));
+ assertEquals(new LineOnOtherInfo(3, true),
+ mapper.lineOnOther(DisplaySide.B, 5));
+
+ assertEquals(new LineOnOtherInfo(2, true),
+ mapper.lineOnOther(DisplaySide.B, 2));
+ assertEquals(new LineOnOtherInfo(2, false),
+ mapper.lineOnOther(DisplaySide.B, 3));
+ }
}
diff --git a/gerrit-launcher/src/main/java/com/google/gerrit/launcher/GerritLauncher.java b/gerrit-launcher/src/main/java/com/google/gerrit/launcher/GerritLauncher.java
index bd8f081..82981c1 100644
--- a/gerrit-launcher/src/main/java/com/google/gerrit/launcher/GerritLauncher.java
+++ b/gerrit-launcher/src/main/java/com/google/gerrit/launcher/GerritLauncher.java
@@ -156,10 +156,7 @@
final Method main;
try {
main = clazz.getMethod("main", argv.getClass());
- } catch (SecurityException e) {
- System.err.println("fatal: unknown command " + name);
- return 1;
- } catch (NoSuchMethodException e) {
+ } catch (SecurityException | NoSuchMethodException e) {
System.err.println("fatal: unknown command " + name);
return 1;
}
diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index 9d9ce22..b92d1cb 100644
--- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -54,6 +54,7 @@
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
+import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.analysis.standard.StandardAnalyzer;
import org.apache.lucene.analysis.util.CharArraySet;
import org.apache.lucene.document.Document;
@@ -157,6 +158,7 @@
private final ChangeData.Factory changeDataFactory;
private final File dir;
private final Schema<ChangeData> schema;
+ private final QueryBuilder queryBuilder;
private final SubIndex openIndex;
private final SubIndex closedIndex;
@@ -186,6 +188,10 @@
LUCENE_VERSIONS.get(schema),
"unknown Lucene version for index schema: %s", schema);
+ Analyzer analyzer =
+ new StandardAnalyzer(luceneVersion, CharArraySet.EMPTY_SET);
+ queryBuilder = new QueryBuilder(schema, analyzer);
+
IndexWriterConfig openConfig =
getIndexWriterConfig(luceneVersion, cfg, "changes_open");
IndexWriterConfig closedConfig =
@@ -298,7 +304,7 @@
if (!Sets.intersection(statuses, CLOSED_STATUSES).isEmpty()) {
indexes.add(closedIndex);
}
- return new QuerySource(indexes, QueryBuilder.toQuery(schema, p), limit,
+ return new QuerySource(indexes, queryBuilder.toQuery(p), limit,
ChangeQueryBuilder.hasNonTrivialSortKeyAfter(schema, p));
}
diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/QueryBuilder.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/QueryBuilder.java
index f99cce8..faa9fe0 100644
--- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/QueryBuilder.java
+++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/QueryBuilder.java
@@ -33,9 +33,9 @@
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.SortKeyPredicate;
+import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.index.Term;
import org.apache.lucene.search.BooleanQuery;
-import org.apache.lucene.search.FuzzyQuery;
import org.apache.lucene.search.MatchAllDocsQuery;
import org.apache.lucene.search.NumericRangeQuery;
import org.apache.lucene.search.PrefixQuery;
@@ -55,27 +55,34 @@
return intTerm(ID_FIELD, cd.getId().get());
}
- public static Query toQuery(Schema<ChangeData> schema, Predicate<ChangeData> p)
- throws QueryParseException {
+ private final Schema<ChangeData> schema;
+ private final org.apache.lucene.util.QueryBuilder queryBuilder;
+
+ public QueryBuilder(Schema<ChangeData> schema, Analyzer analyzer) {
+ this.schema = schema;
+ queryBuilder = new org.apache.lucene.util.QueryBuilder(analyzer);
+ }
+
+ public Query toQuery(Predicate<ChangeData> p) throws QueryParseException {
if (p instanceof AndPredicate) {
- return and(schema, p);
+ return and(p);
} else if (p instanceof OrPredicate) {
- return or(schema, p);
+ return or(p);
} else if (p instanceof NotPredicate) {
- return not(schema, p);
+ return not(p);
} else if (p instanceof IndexPredicate) {
- return fieldQuery(schema, (IndexPredicate<ChangeData>) p);
+ return fieldQuery((IndexPredicate<ChangeData>) p);
} else {
throw new QueryParseException("cannot create query for index: " + p);
}
}
- private static Query or(Schema<ChangeData> schema, Predicate<ChangeData> p)
+ private Query or(Predicate<ChangeData> p)
throws QueryParseException {
try {
BooleanQuery q = new BooleanQuery();
for (int i = 0; i < p.getChildCount(); i++) {
- q.add(toQuery(schema, p.getChild(i)), SHOULD);
+ q.add(toQuery(p.getChild(i)), SHOULD);
}
return q;
} catch (BooleanQuery.TooManyClauses e) {
@@ -83,7 +90,7 @@
}
}
- private static Query and(Schema<ChangeData> schema, Predicate<ChangeData> p)
+ private Query and(Predicate<ChangeData> p)
throws QueryParseException {
try {
BooleanQuery b = new BooleanQuery();
@@ -95,10 +102,10 @@
if (n instanceof TimestampRangePredicate) {
b.add(notTimestamp((TimestampRangePredicate<ChangeData>) n), MUST);
} else {
- not.add(toQuery(schema, n));
+ not.add(toQuery(n));
}
} else {
- b.add(toQuery(schema, c), MUST);
+ b.add(toQuery(c), MUST);
}
}
for (Query q : not) {
@@ -110,7 +117,7 @@
}
}
- private static Query not(Schema<ChangeData> schema, Predicate<ChangeData> p)
+ private Query not(Predicate<ChangeData> p)
throws QueryParseException {
Predicate<ChangeData> n = p.getChild(0);
if (n instanceof TimestampRangePredicate) {
@@ -120,12 +127,12 @@
// Lucene does not support negation, start with all and subtract.
BooleanQuery q = new BooleanQuery();
q.add(new MatchAllDocsQuery(), MUST);
- q.add(toQuery(schema, n), MUST_NOT);
+ q.add(toQuery(n), MUST_NOT);
return q;
}
- private static Query fieldQuery(Schema<ChangeData> schema,
- IndexPredicate<ChangeData> p) throws QueryParseException {
+ private Query fieldQuery(IndexPredicate<ChangeData> p)
+ throws QueryParseException {
if (p.getType() == FieldType.INTEGER) {
return intQuery(p);
} else if (p.getType() == FieldType.TIMESTAMP) {
@@ -137,7 +144,7 @@
} else if (p.getType() == FieldType.FULL_TEXT) {
return fullTextQuery(p);
} else if (p instanceof SortKeyPredicate) {
- return sortKeyQuery(schema, (SortKeyPredicate) p);
+ return sortKeyQuery((SortKeyPredicate) p);
} else {
throw badFieldType(p.getType());
}
@@ -149,7 +156,7 @@
return new Term(name, bytes);
}
- private static Query intQuery(IndexPredicate<ChangeData> p)
+ private Query intQuery(IndexPredicate<ChangeData> p)
throws QueryParseException {
int value;
try {
@@ -162,7 +169,7 @@
return new TermQuery(intTerm(p.getField().getName(), value));
}
- private static Query sortKeyQuery(Schema<ChangeData> schema, SortKeyPredicate p) {
+ private Query sortKeyQuery(SortKeyPredicate p) {
long min = p.getMinValue(schema);
long max = p.getMaxValue(schema);
return NumericRangeQuery.newLongRange(
@@ -172,7 +179,7 @@
false, false);
}
- private static Query timestampQuery(IndexPredicate<ChangeData> p)
+ private Query timestampQuery(IndexPredicate<ChangeData> p)
throws QueryParseException {
if (p instanceof TimestampRangePredicate) {
TimestampRangePredicate<ChangeData> r =
@@ -186,7 +193,7 @@
throw new QueryParseException("not a timestamp: " + p);
}
- private static Query notTimestamp(TimestampRangePredicate<ChangeData> r)
+ private Query notTimestamp(TimestampRangePredicate<ChangeData> r)
throws QueryParseException {
if (r.getMinTimestamp().getTime() == 0) {
return NumericRangeQuery.newIntRange(
@@ -198,7 +205,7 @@
throw new QueryParseException("cannot negate: " + r);
}
- private static Query exactQuery(IndexPredicate<ChangeData> p) {
+ private Query exactQuery(IndexPredicate<ChangeData> p) {
if (p instanceof RegexPredicate<?>) {
return regexQuery(p);
} else {
@@ -206,7 +213,7 @@
}
}
- private static Query regexQuery(IndexPredicate<ChangeData> p) {
+ private Query regexQuery(IndexPredicate<ChangeData> p) {
String re = p.getValue();
if (re.startsWith("^")) {
re = re.substring(1);
@@ -217,12 +224,12 @@
return new RegexpQuery(new Term(p.getField().getName(), re));
}
- private static Query prefixQuery(IndexPredicate<ChangeData> p) {
+ private Query prefixQuery(IndexPredicate<ChangeData> p) {
return new PrefixQuery(new Term(p.getField().getName(), p.getValue()));
}
- private static Query fullTextQuery(IndexPredicate<ChangeData> p) {
- return new FuzzyQuery(new Term(p.getField().getName(), p.getValue()));
+ private Query fullTextQuery(IndexPredicate<ChangeData> p) {
+ return queryBuilder.createPhraseQuery(p.getField().getName(), p.getValue());
}
public static int toIndexTime(Timestamp ts) {
@@ -232,7 +239,4 @@
public static IllegalArgumentException badFieldType(FieldType<?> t) {
return new IllegalArgumentException("unknown index field type " + t);
}
-
- private QueryBuilder() {
- }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
index 6481d5f..a42827a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
@@ -107,7 +107,8 @@
db.rollback();
}
- CheckedFuture<?, IOException> indexFuture = indexer.indexAsync(change);
+ CheckedFuture<?, IOException> indexFuture =
+ indexer.indexAsync(change.getId());
try {
ReplyToChangeSender cm = abandonedSenderFactory.create(change);
cm.setFrom(caller.getAccountId());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityChecker.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityChecker.java
index 6c1202f..bf9f65d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityChecker.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityChecker.java
@@ -185,7 +185,8 @@
* reindexes the change (whether the mergeability flag was updated or
* not)
*/
- public CheckedFuture<?, IOException> updateAndIndexAsync(final Change change) {
+ public CheckedFuture<?, IOException> updateAndIndexAsync(Change change) {
+ final Change.Id id = change.getId();
return Futures.makeChecked(
Futures.transform(updateAsync(change),
new AsyncFunction<Boolean, Object>() {
@@ -194,7 +195,7 @@
public ListenableFuture<Object> apply(Boolean indexUpdated)
throws Exception {
if (!indexUpdated) {
- return (ListenableFuture<Object>) indexer.indexAsync(change);
+ return (ListenableFuture<Object>) indexer.indexAsync(id);
}
return Futures.immediateFuture(null);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
index 70936af..0203e3b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
@@ -139,7 +139,7 @@
CheckedFuture<?, IOException> indexWrite;
if (dirty) {
- indexWrite = indexer.indexAsync(change);
+ indexWrite = indexer.indexAsync(change.getId());
} else {
indexWrite = Futures.<Void, IOException> immediateCheckedFuture(null);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
index 1d6dfe7..46893df 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
@@ -228,7 +228,8 @@
db.rollback();
}
- CheckedFuture<?, IOException> indexFuture = indexer.indexAsync(rsrc.getChange());
+ CheckedFuture<?, IOException> indexFuture =
+ indexer.indexAsync(rsrc.getChange().getId());
result.reviewers = Lists.newArrayListWithCapacity(added.size());
for (PatchSetApproval psa : added) {
result.reviewers.add(json.format(
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Publish.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Publish.java
index d49527d..f522de5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Publish.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Publish.java
@@ -77,7 +77,7 @@
if (!updatedPatchSet.isDraft()
|| updatedChange.getStatus() == Change.Status.NEW) {
CheckedFuture<?, IOException> indexFuture =
- indexer.indexAsync(updatedChange);
+ indexer.indexAsync(updatedChange.getId());
hooks.doDraftPublishedHook(updatedChange, updatedPatchSet, dbProvider.get());
sender.send(rsrc.getChange().getStatus() == Change.Status.DRAFT,
rsrc.getUser(), updatedChange, updatedPatchSet,
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java
index 84ace98..a9534a5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java
@@ -115,7 +115,8 @@
} finally {
db.rollback();
}
- CheckedFuture<?, IOException> indexFuture = indexer.indexAsync(change);
+ CheckedFuture<?, IOException> indexFuture =
+ indexer.indexAsync(change.getId());
hooks.doTopicChangedHook(change, currentUser.getAccount(),
oldTopicName, db);
indexFuture.checkedGet();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java
index b763079..b3641e5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java
@@ -108,7 +108,8 @@
db.rollback();
}
- CheckedFuture<?, IOException> indexFuture = indexer.indexAsync(change);
+ CheckedFuture<?, IOException> indexFuture =
+ indexer.indexAsync(change.getId());
try {
ReplyToChangeSender cm = restoredSenderFactory.create(change);
cm.setFrom(caller.getAccountId());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/contact/ContactStoreProvider.java b/gerrit-server/src/main/java/com/google/gerrit/server/contact/ContactStoreProvider.java
index d5cd11a..835dc40 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/contact/ContactStoreProvider.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/contact/ContactStoreProvider.java
@@ -85,21 +85,10 @@
Class.forName(PGPPublicKey.class.getName());
addBouncyCastleProvider();
return true;
- } catch (NoClassDefFoundError noBouncyCastle) {
- return false;
- } catch (ClassNotFoundException noBouncyCastle) {
- return false;
- } catch (SecurityException noBouncyCastle) {
- return false;
- } catch (NoSuchMethodException noBouncyCastle) {
- return false;
- } catch (InstantiationException noBouncyCastle) {
- return false;
- } catch (IllegalAccessException noBouncyCastle) {
- return false;
- } catch (InvocationTargetException noBouncyCastle) {
- return false;
- } catch (ClassCastException noBouncyCastle) {
+ } catch (NoClassDefFoundError | ClassNotFoundException | SecurityException
+ | NoSuchMethodException | InstantiationException
+ | IllegalAccessException | InvocationTargetException
+ | ClassCastException noBouncyCastle) {
return false;
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
index 3dae94f..e4d0de8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
@@ -1043,7 +1043,7 @@
CheckedFuture<?, IOException> indexFuture;
if (change != null) {
- indexFuture = indexer.indexAsync(change);
+ indexFuture = indexer.indexAsync(change.getId());
} else {
indexFuture = null;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java
index 36208c3..9fa46fb 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.index;
+import com.google.common.base.Splitter;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.gerrit.reviewdb.client.Account;
@@ -160,10 +161,11 @@
}
};
- /** List of filenames modified in the current patch set. */
- public static final FieldDef<ChangeData, Iterable<String>> FILE =
+ /** List of full file paths modified in the current patch set. */
+ public static final FieldDef<ChangeData, Iterable<String>> PATH =
new FieldDef.Repeatable<ChangeData, String>(
- ChangeQueryBuilder.FIELD_FILE, FieldType.EXACT, false) {
+ // Named for backwards compatibility.
+ "file", FieldType.EXACT, false) {
@Override
public Iterable<String> get(ChangeData input, FillArgs args)
throws OrmException {
@@ -171,6 +173,28 @@
}
};
+ public static Set<String> getFileParts(ChangeData cd) throws OrmException {
+ Splitter s = Splitter.on('/').omitEmptyStrings();
+ Set<String> r = Sets.newHashSet();
+ for (String path : cd.currentFilePaths()) {
+ for (String part : s.split(path)) {
+ r.add(part);
+ }
+ }
+ return r;
+ }
+
+ /** Components of each file path modified in the current patch set. */
+ public static final FieldDef<ChangeData, Iterable<String>> FILE_PART =
+ new FieldDef.Repeatable<ChangeData, String>(
+ "filepart", FieldType.EXACT, false) {
+ @Override
+ public Iterable<String> get(ChangeData input, FillArgs args)
+ throws OrmException {
+ return getFileParts(input);
+ }
+ };
+
/** Owner/creator of the change. */
public static final FieldDef<ChangeData, Integer> OWNER =
new FieldDef.Single<ChangeData, Integer>(
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndexer.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndexer.java
index 612df9a..b500e8a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndexer.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndexer.java
@@ -112,12 +112,12 @@
/**
* Start indexing a change.
*
- * @param change change to index.
+ * @param id change to index.
* @return future for the indexing task.
*/
- public CheckedFuture<?, IOException> indexAsync(Change change) {
+ public CheckedFuture<?, IOException> indexAsync(Change.Id id) {
return executor != null
- ? submit(new Task(change, false))
+ ? submit(new Task(id, false))
: Futures.<Object, IOException> immediateCheckedFuture(null);
}
@@ -145,12 +145,12 @@
/**
* Start deleting a change.
*
- * @param change change to delete.
+ * @param id change to delete.
* @return future for the deleting task.
*/
- public CheckedFuture<?, IOException> deleteAsync(Change change) {
+ public CheckedFuture<?, IOException> deleteAsync(Change.Id id) {
return executor != null
- ? submit(new Task(change, true))
+ ? submit(new Task(id, true))
: Futures.<Object, IOException> immediateCheckedFuture(null);
}
@@ -186,11 +186,11 @@
}
private class Task implements Callable<Void> {
- private final Change change;
+ private final Change.Id id;
private final boolean delete;
- private Task(Change change, boolean delete) {
- this.change = change;
+ private Task(Change.Id id, boolean delete) {
+ this.id = id;
this.delete = delete;
}
@@ -225,7 +225,7 @@
RequestContext oldCtx = context.setContext(newCtx);
try {
ChangeData cd = changeDataFactory.create(
- newCtx.getReviewDbProvider().get(), change);
+ newCtx.getReviewDbProvider().get(), id);
if (delete) {
for (ChangeIndex i : getWriteIndexes()) {
i.delete(cd);
@@ -244,16 +244,14 @@
}
}
} catch (Exception e) {
- log.error(String.format(
- "Failed to index change %d in %s",
- change.getId().get(), change.getProject().get()), e);
+ log.error(String.format("Failed to index change %d", id), e);
throw e;
}
}
@Override
public String toString() {
- return "index-change-" + change.getId().get();
+ return "index-change-" + id.get();
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeSchemas.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeSchemas.java
index eb2c527..475e7d1 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeSchemas.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeSchemas.java
@@ -40,7 +40,7 @@
ChangeField.TOPIC,
ChangeField.UPDATED,
ChangeField.LEGACY_SORTKEY,
- ChangeField.FILE,
+ ChangeField.PATH,
ChangeField.OWNER,
ChangeField.REVIEWER,
ChangeField.COMMIT,
@@ -60,7 +60,7 @@
ChangeField.TOPIC,
ChangeField.UPDATED,
ChangeField.LEGACY_SORTKEY,
- ChangeField.FILE,
+ ChangeField.PATH,
ChangeField.OWNER,
ChangeField.REVIEWER,
ChangeField.COMMIT,
@@ -81,7 +81,7 @@
ChangeField.TOPIC,
ChangeField.UPDATED,
ChangeField.SORTKEY,
- ChangeField.FILE,
+ ChangeField.PATH,
ChangeField.OWNER,
ChangeField.REVIEWER,
ChangeField.COMMIT,
@@ -105,7 +105,7 @@
ChangeField.TOPIC,
ChangeField.UPDATED,
ChangeField.SORTKEY,
- ChangeField.FILE,
+ ChangeField.PATH,
ChangeField.OWNER,
ChangeField.REVIEWER,
ChangeField.COMMIT,
@@ -121,6 +121,30 @@
// For upgrade to Lucene 4.6.0 index format only.
static final Schema<ChangeData> V6 = release(V5.getFields().values());
+ static final Schema<ChangeData> V7 = release(
+ ChangeField.LEGACY_ID,
+ ChangeField.ID,
+ ChangeField.STATUS,
+ ChangeField.PROJECT,
+ ChangeField.REF,
+ ChangeField.TOPIC,
+ ChangeField.UPDATED,
+ ChangeField.SORTKEY,
+ ChangeField.FILE_PART,
+ ChangeField.PATH,
+ ChangeField.OWNER,
+ ChangeField.REVIEWER,
+ ChangeField.COMMIT,
+ ChangeField.TR,
+ ChangeField.LABEL,
+ ChangeField.REVIEWED,
+ ChangeField.COMMIT_MESSAGE,
+ ChangeField.COMMENT,
+ ChangeField.CHANGE,
+ ChangeField.APPROVAL,
+ ChangeField.MERGEABLE);
+
+
private static Schema<ChangeData> release(Collection<FieldDef<ChangeData, ?>> fields) {
return new Schema<ChangeData>(true, fields);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/RegexPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/RegexPredicate.java
index 198c7b0..b73674d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/RegexPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/RegexPredicate.java
@@ -18,4 +18,8 @@
protected RegexPredicate(FieldDef<I, ?> def, String value) {
super(def, value);
}
+
+ protected RegexPredicate(FieldDef<I, ?> def, String name, String value) {
+ super(def, name, value);
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java
index aa120e1..32c4ef8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java
@@ -45,19 +45,19 @@
@RequiresCapability(GlobalCapability.CREATE_PROJECT)
public class CreateProject implements RestModifyView<TopLevelResource, Input> {
public static class Input {
- String name;
- String parent;
- String description;
- boolean permissionsOnly;
- boolean createEmptyCommit;
- SubmitType submitType;
- List<String> branches;
- List<String> owners;
- InheritableBoolean useContributorAgreements;
- InheritableBoolean useSignedOffBy;
- InheritableBoolean useContentMerge;
- InheritableBoolean requireChangeId;
- String maxObjectSizeLimit;
+ public String name;
+ public String parent;
+ public String description;
+ public boolean permissionsOnly;
+ public boolean createEmptyCommit;
+ public SubmitType submitType;
+ public List<String> branches;
+ public List<String> owners;
+ public InheritableBoolean useContributorAgreements;
+ public InheritableBoolean useSignedOffBy;
+ public InheritableBoolean useContentMerge;
+ public InheritableBoolean requireChangeId;
+ public String maxObjectSizeLimit;
}
public static interface Factory {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
index 2e29c80..8f8cb70 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -16,7 +16,6 @@
import static com.google.gerrit.server.ApprovalsUtil.sortApprovals;
-import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Objects;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableList;
@@ -157,7 +156,6 @@
private ChangedLines changedLines;
private boolean patchesLoaded;
- @VisibleForTesting
@AssistedInject
public ChangeData(
GitRepositoryManager repoManager,
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 99dd2e7..c0b8a63 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -99,6 +99,7 @@
public static final String FIELD_OWNER = "owner";
public static final String FIELD_OWNERIN = "ownerin";
public static final String FIELD_PARENTPROJECT = "parentproject";
+ public static final String FIELD_PATH = "path";
public static final String FIELD_PROJECT = "project";
public static final String FIELD_REF = "ref";
public static final String FIELD_REVIEWER = "reviewer";
@@ -384,9 +385,18 @@
@Operator
public Predicate<ChangeData> file(String file) throws QueryParseException {
if (file.startsWith("^")) {
- return new RegexFilePredicate(file);
+ return new RegexPathPredicate(FIELD_FILE, file);
} else {
- return new EqualsFilePredicate(file);
+ return EqualsFilePredicate.create(args, file);
+ }
+ }
+
+ @Operator
+ public Predicate<ChangeData> path(String path) throws QueryParseException {
+ if (path.startsWith("^")) {
+ return new RegexPathPredicate(FIELD_PATH, path);
+ } else {
+ return new EqualsPathPredicate(FIELD_PATH, path);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
index 2102541..3c8b2c0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
@@ -70,7 +70,8 @@
List<Predicate<ChangeData>> filePredicates =
Lists.newArrayListWithCapacity(files.size());
for (String file : files) {
- filePredicates.add(new EqualsFilePredicate(file));
+ filePredicates.add(
+ new EqualsPathPredicate(ChangeQueryBuilder.FIELD_PATH, file));
}
List<Predicate<ChangeData>> predicatesForOneChange =
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsFilePredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsFilePredicate.java
index 13bd08b..e5fc51d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsFilePredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsFilePredicate.java
@@ -16,31 +16,31 @@
import com.google.gerrit.server.index.ChangeField;
import com.google.gerrit.server.index.IndexPredicate;
+import com.google.gerrit.server.query.Predicate;
+import com.google.gerrit.server.query.change.ChangeQueryBuilder.Arguments;
import com.google.gwtorm.server.OrmException;
-import java.util.Collections;
-import java.util.List;
-
class EqualsFilePredicate extends IndexPredicate<ChangeData> {
+ static Predicate<ChangeData> create(Arguments args, String value) {
+ Predicate<ChangeData> eqPath =
+ new EqualsPathPredicate(ChangeQueryBuilder.FIELD_FILE, value);
+ if (!args.indexes.getSearchIndex().getSchema().getFields().containsKey(
+ ChangeField.FILE_PART.getName())) {
+ return eqPath;
+ }
+ return Predicate.or(eqPath, new EqualsFilePredicate(value));
+ }
+
private final String value;
- EqualsFilePredicate(String value) {
- super(ChangeField.FILE, value);
+ private EqualsFilePredicate(String value) {
+ super(ChangeField.FILE_PART, ChangeQueryBuilder.FIELD_FILE, value);
this.value = value;
}
@Override
public boolean match(ChangeData object) throws OrmException {
- List<String> files = object.currentFilePaths();
- if (files != null) {
- return Collections.binarySearch(files, value) >= 0;
- } else {
- // The ChangeData can't do expensive lookups right now. Bypass
- // them and include the result anyway. We might be able to do
- // a narrow later on to a smaller set.
- //
- return true;
- }
+ return ChangeField.getFileParts(object).contains(value);
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsPathPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsPathPredicate.java
new file mode 100644
index 0000000..055b6d5
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsPathPredicate.java
@@ -0,0 +1,42 @@
+// Copyright (C) 2013 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.query.change;
+
+import com.google.gerrit.server.index.ChangeField;
+import com.google.gerrit.server.index.IndexPredicate;
+import com.google.gwtorm.server.OrmException;
+
+import java.util.Collections;
+import java.util.List;
+
+class EqualsPathPredicate extends IndexPredicate<ChangeData> {
+ private final String value;
+
+ EqualsPathPredicate(String fieldName, String value) {
+ super(ChangeField.PATH, fieldName, value);
+ this.value = value;
+ }
+
+ @Override
+ public boolean match(ChangeData object) throws OrmException {
+ List<String> files = object.currentFilePaths();
+ return files != null && Collections.binarySearch(files, value) >= 0;
+ }
+
+ @Override
+ public int getCost() {
+ return 1;
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/RegexFilePredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/RegexPathPredicate.java
similarity index 94%
rename from gerrit-server/src/main/java/com/google/gerrit/server/query/change/RegexFilePredicate.java
rename to gerrit-server/src/main/java/com/google/gerrit/server/query/change/RegexPathPredicate.java
index f39dc14..d073002 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/RegexFilePredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/RegexPathPredicate.java
@@ -25,7 +25,7 @@
import java.util.Collections;
import java.util.List;
-class RegexFilePredicate extends RegexPredicate<ChangeData> {
+class RegexPathPredicate extends RegexPredicate<ChangeData> {
private final RunAutomaton pattern;
private final String prefixBegin;
@@ -33,8 +33,8 @@
private final int prefixLen;
private final boolean prefixOnly;
- RegexFilePredicate(String re) {
- super(ChangeField.FILE, re);
+ RegexPathPredicate(String fieldName, String re) {
+ super(ChangeField.PATH, re);
if (re.startsWith("^")) {
re = re.substring(1);
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeIndex.java b/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeIndex.java
index d8a0f65..339a1bb 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeIndex.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeIndex.java
@@ -30,7 +30,7 @@
static Schema<ChangeData> V2 = new Schema<ChangeData>(2, false,
ImmutableList.of(
ChangeField.STATUS,
- ChangeField.FILE,
+ ChangeField.PATH,
ChangeField.SORTKEY));
private static class Source implements ChangeDataSource {
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index ec485bb..0e772f2 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -349,12 +349,27 @@
RevCommit commit2 = repo.parseBody(repo.commit().message("two").create());
Change change2 = newChange(repo, commit2, null, null, null).insert();
- assertTrue(query("topic:foo").isEmpty());
+ assertTrue(query("message:foo").isEmpty());
assertResultEquals(change1, queryOne("message:one"));
assertResultEquals(change2, queryOne("message:two"));
}
@Test
+ public void fullTextWithNumbers() throws Exception {
+ TestRepository<InMemoryRepository> repo = createProject("repo");
+ RevCommit commit1 =
+ repo.parseBody(repo.commit().message("12345 67890").create());
+ Change change1 = newChange(repo, commit1, null, null, null).insert();
+ RevCommit commit2 =
+ repo.parseBody(repo.commit().message("12346 67891").create());
+ Change change2 = newChange(repo, commit2, null, null, null).insert();
+
+ assertTrue(query("message:1234").isEmpty());
+ assertResultEquals(change1, queryOne("message:12345"));
+ assertResultEquals(change2, queryOne("message:12346"));
+ }
+
+ @Test
public void byLabel() throws Exception {
accountManager.authenticate(AuthRequest.forUser("anotheruser"));
TestRepository<InMemoryRepository> repo = createProject("repo");
@@ -574,13 +589,16 @@
TestRepository<InMemoryRepository> repo = createProject("repo");
RevCommit commit = repo.parseBody(
repo.commit().message("one")
- .add("file1", "contents1").add("file2", "contents2")
+ .add("dir/file1", "contents1").add("dir/file2", "contents2")
.create());
Change change = newChange(repo, commit, null, null, null).insert();
assertTrue(query("file:file").isEmpty());
+ assertResultEquals(change, queryOne("file:dir"));
assertResultEquals(change, queryOne("file:file1"));
assertResultEquals(change, queryOne("file:file2"));
+ assertResultEquals(change, queryOne("file:dir/file1"));
+ assertResultEquals(change, queryOne("file:dir/file2"));
}
@Test
@@ -588,12 +606,43 @@
TestRepository<InMemoryRepository> repo = createProject("repo");
RevCommit commit = repo.parseBody(
repo.commit().message("one")
- .add("file1", "contents1").add("file2", "contents2")
+ .add("dir/file1", "contents1").add("dir/file2", "contents2")
.create());
Change change = newChange(repo, commit, null, null, null).insert();
- assertTrue(query("file:file.*").isEmpty());
- assertResultEquals(change, queryOne("file:^file.*"));
+ assertTrue(query("file:.*file.*").isEmpty());
+ assertTrue(query("file:^file.*").isEmpty()); // Whole path only.
+ assertResultEquals(change, queryOne("file:^dir.file.*"));
+ }
+
+ @Test
+ public void byPathExact() throws Exception {
+ TestRepository<InMemoryRepository> repo = createProject("repo");
+ RevCommit commit = repo.parseBody(
+ repo.commit().message("one")
+ .add("dir/file1", "contents1").add("dir/file2", "contents2")
+ .create());
+ Change change = newChange(repo, commit, null, null, null).insert();
+
+ assertTrue(query("path:file").isEmpty());
+ assertTrue(query("path:dir").isEmpty());
+ assertTrue(query("path:file1").isEmpty());
+ assertTrue(query("path:file2").isEmpty());
+ assertResultEquals(change, queryOne("path:dir/file1"));
+ assertResultEquals(change, queryOne("path:dir/file2"));
+ }
+
+ @Test
+ public void byPathRegex() throws Exception {
+ TestRepository<InMemoryRepository> repo = createProject("repo");
+ RevCommit commit = repo.parseBody(
+ repo.commit().message("one")
+ .add("dir/file1", "contents1").add("dir/file2", "contents2")
+ .create());
+ Change change = newChange(repo, commit, null, null, null).insert();
+
+ assertTrue(query("path:.*file.*").isEmpty());
+ assertResultEquals(change, queryOne("path:^dir.file.*"));
}
@Test
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/RegexFilePredicateTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/RegexPathPredicateTest.java
similarity index 85%
rename from gerrit-server/src/test/java/com/google/gerrit/server/query/change/RegexFilePredicateTest.java
rename to gerrit-server/src/test/java/com/google/gerrit/server/query/change/RegexPathPredicateTest.java
index 363fa26..4e5dcdd 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/RegexFilePredicateTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/RegexPathPredicateTest.java
@@ -23,10 +23,10 @@
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
-public class RegexFilePredicateTest {
+public class RegexPathPredicateTest {
@Test
public void testPrefixOnlyOptimization() throws OrmException {
- RegexFilePredicate p = predicate("^a/b/.*");
+ RegexPathPredicate p = predicate("^a/b/.*");
assertTrue(p.match(change("a/b/source.c")));
assertFalse(p.match(change("source.c")));
@@ -36,7 +36,7 @@
@Test
public void testPrefixReducesSearchSpace() throws OrmException {
- RegexFilePredicate p = predicate("^a/b/.*\\.[ch]");
+ RegexPathPredicate p = predicate("^a/b/.*\\.[ch]");
assertTrue(p.match(change("a/b/source.c")));
assertFalse(p.match(change("a/b/source.res")));
assertFalse(p.match(change("source.res")));
@@ -46,7 +46,7 @@
@Test
public void testFileExtension_Constant() throws OrmException {
- RegexFilePredicate p = predicate("^.*\\.res");
+ RegexPathPredicate p = predicate("^.*\\.res");
assertTrue(p.match(change("test.res")));
assertTrue(p.match(change("foo/bar/test.res")));
assertFalse(p.match(change("test.res.bar")));
@@ -54,7 +54,7 @@
@Test
public void testFileExtension_CharacterGroup() throws OrmException {
- RegexFilePredicate p = predicate("^.*\\.[ch]");
+ RegexPathPredicate p = predicate("^.*\\.[ch]");
assertTrue(p.match(change("test.c")));
assertTrue(p.match(change("test.h")));
assertFalse(p.match(change("test.C")));
@@ -71,14 +71,14 @@
@Test
public void testExactMatch() throws OrmException {
- RegexFilePredicate p = predicate("^foo.c");
+ RegexPathPredicate p = predicate("^foo.c");
assertTrue(p.match(change("foo.c")));
assertFalse(p.match(change("foo.cc")));
assertFalse(p.match(change("bar.c")));
}
- private static RegexFilePredicate predicate(String pattern) {
- return new RegexFilePredicate(pattern);
+ private static RegexPathPredicate predicate(String pattern) {
+ return new RegexPathPredicate(ChangeQueryBuilder.FIELD_PATH, pattern);
}
private static ChangeData change(String... files) {
diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java
index 40c79b0..df2de80 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java
@@ -191,26 +191,12 @@
Constructor<?> c =
clazz.getConstructor(Integer.class, int.class, String.class);
return (Module) c.newInstance(version, 0, null);
- } catch (ClassNotFoundException e) {
- throw newProvisionException(e);
- } catch (SecurityException e) {
- throw newProvisionException(e);
- } catch (NoSuchMethodException e) {
- throw newProvisionException(e);
- } catch (IllegalArgumentException e) {
- throw newProvisionException(e);
- } catch (InstantiationException e) {
- throw newProvisionException(e);
- } catch (IllegalAccessException e) {
- throw newProvisionException(e);
- } catch (InvocationTargetException e) {
- throw newProvisionException(e);
+ } catch (ClassNotFoundException | SecurityException | NoSuchMethodException
+ | IllegalArgumentException | InstantiationException
+ | IllegalAccessException | InvocationTargetException e) {
+ ProvisionException pe = new ProvisionException(e.getMessage());
+ pe.initCause(e);
+ throw pe;
}
}
-
- private static ProvisionException newProvisionException(Throwable cause) {
- ProvisionException pe = new ProvisionException(cause.getMessage());
- pe.initCause(cause);
- return pe;
- }
}
diff --git a/gerrit-solr/BUCK b/gerrit-solr/BUCK
index c072830..ec3c728 100644
--- a/gerrit-solr/BUCK
+++ b/gerrit-solr/BUCK
@@ -12,6 +12,7 @@
'//lib/guice:guice',
'//lib/jgit:jgit',
'//lib/log:api',
+ '//lib/lucene:analyzers-common',
'//lib/lucene:core',
'//lib/solr:solrj',
],
diff --git a/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrChangeIndex.java b/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrChangeIndex.java
index 73dc07e..7101a4d 100644
--- a/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrChangeIndex.java
+++ b/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrChangeIndex.java
@@ -45,7 +45,10 @@
import com.google.gwtorm.server.ResultSet;
import com.google.inject.Provider;
+import org.apache.lucene.analysis.standard.StandardAnalyzer;
+import org.apache.lucene.analysis.util.CharArraySet;
import org.apache.lucene.search.Query;
+import org.apache.lucene.util.Version;
import org.apache.solr.client.solrj.SolrQuery;
import org.apache.solr.client.solrj.SolrServer;
import org.apache.solr.client.solrj.SolrServerException;
@@ -79,6 +82,7 @@
private final CloudSolrServer openIndex;
private final CloudSolrServer closedIndex;
private final Schema<ChangeData> schema;
+ private final QueryBuilder queryBuilder;
SolrChangeIndex(
@GerritServerConfig Config cfg,
@@ -101,6 +105,14 @@
throw new IllegalStateException("index.solr.url must be supplied");
}
+ // Version is only used to determine the list of stop words used by the
+ // analyzer, so use the latest version rather than trying to match the Solr
+ // server version.
+ @SuppressWarnings("deprecation")
+ Version v = Version.LUCENE_CURRENT;
+ queryBuilder = new QueryBuilder(
+ schema, new StandardAnalyzer(v, CharArraySet.EMPTY_SET));
+
base = Strings.nullToEmpty(base);
openIndex = new CloudSolrServer(url);
openIndex.setDefaultCollection(base + CHANGES_OPEN);
@@ -208,7 +220,7 @@
if (!Sets.intersection(statuses, CLOSED_STATUSES).isEmpty()) {
indexes.add(closedIndex);
}
- return new QuerySource(indexes, QueryBuilder.toQuery(schema, p), limit,
+ return new QuerySource(indexes, queryBuilder.toQuery(p), limit,
ChangeQueryBuilder.hasNonTrivialSortKeyAfter(schema, p));
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/CreateProjectCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/CreateProjectCommand.java
index 90c5e94..90c1f2c 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/CreateProjectCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/CreateProjectCommand.java
@@ -14,24 +14,31 @@
package com.google.gerrit.sshd.commands;
+import com.google.common.base.Function;
+import com.google.common.collect.Lists;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.common.errors.ProjectCreationFailedException;
import com.google.gerrit.extensions.annotations.RequiresCapability;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.Project.InheritableBoolean;
import com.google.gerrit.reviewdb.client.Project.SubmitType;
-import com.google.gerrit.server.project.PerformCreateProject;
-import com.google.gerrit.server.project.CreateProjectArgs;
+import com.google.gerrit.server.project.CreateProject;
+import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.SuggestParentCandidates;
import com.google.gerrit.sshd.CommandMetaData;
import com.google.gerrit.sshd.SshCommand;
+import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import org.kohsuke.args4j.Argument;
import org.kohsuke.args4j.Option;
+import java.io.IOException;
import java.util.List;
/** Create a new project. **/
@@ -120,35 +127,46 @@
}
@Inject
- private PerformCreateProject.Factory factory;
+ private Provider<CreateProject.Factory> createProjectFactory;
@Inject
private SuggestParentCandidates.Factory suggestParentCandidatesFactory;
@Override
- protected void run() throws Exception {
+ protected void run() throws UnloggedFailure {
try {
if (!suggestParent) {
if (projectName == null) {
throw new UnloggedFailure(1, "fatal: Project name is required.");
}
- final CreateProjectArgs args = new CreateProjectArgs();
- args.setProjectName(projectName);
- args.ownerIds = ownerIds;
- args.newParent = newParent;
- args.permissionsOnly = permissionsOnly;
- args.projectDescription = projectDescription;
- args.submitType = submitType;
- args.contributorAgreements = contributorAgreements;
- args.signedOffBy = signedOffBy;
- args.contentMerge = contentMerge;
- args.changeIdRequired = requireChangeID;
- args.branch = branch;
- args.createEmptyCommit = createEmptyCommit;
- args.maxObjectSizeLimit = maxObjectSizeLimit;
- final PerformCreateProject createProject = factory.create(args);
- createProject.createProject();
+ CreateProject.Input input = new CreateProject.Input();
+ input.name = projectName;
+ if (ownerIds != null) {
+ input.owners = Lists.transform(ownerIds,
+ new Function<AccountGroup.UUID, String>() {
+ @Override
+ public String apply(AccountGroup.UUID uuid) {
+ return uuid.get();
+ }
+ });
+ }
+ if (newParent != null) {
+ input.parent = newParent.getProject().getName();
+ }
+ input.permissionsOnly = permissionsOnly;
+ input.description = projectDescription;
+ input.submitType = submitType;
+ input.useContributorAgreements = contributorAgreements;
+ input.useSignedOffBy = signedOffBy;
+ input.useContentMerge = contentMerge;
+ input.requireChangeId = requireChangeID;
+ input.branches = branch;
+ input.createEmptyCommit = createEmptyCommit;
+ input.maxObjectSizeLimit = maxObjectSizeLimit;
+
+ createProjectFactory.get().create(projectName)
+ .apply(TopLevelResource.INSTANCE, input);
} else {
List<Project.NameKey> parentCandidates =
suggestParentCandidatesFactory.create().getNameKeys();
@@ -157,7 +175,8 @@
stdout.print(parent + "\n");
}
}
- } catch (ProjectCreationFailedException err) {
+ } catch (RestApiException | ProjectCreationFailedException | IOException
+ | NoSuchProjectException | OrmException err) {
throw new UnloggedFailure(1, "fatal: " + err.getMessage(), err);
}
}
diff --git a/lib/asciidoctor/java/DocIndexer.java b/lib/asciidoctor/java/DocIndexer.java
index c8a3a55..6cb9d62 100644
--- a/lib/asciidoctor/java/DocIndexer.java
+++ b/lib/asciidoctor/java/DocIndexer.java
@@ -39,10 +39,13 @@
import java.io.InputStreamReader;
import java.util.ArrayList;
import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import java.util.zip.ZipOutputStream;
public class DocIndexer {
private static final Version LUCENE_VERSION = Version.LUCENE_46;
+ private static final Pattern SECTION_HEADER = Pattern.compile("^=+ (.*)");
@Option(name = "-z", usage = "output zip file")
private String zipFile;
@@ -94,6 +97,10 @@
title = titleReader.readLine();
}
titleReader.close();
+ Matcher matcher = SECTION_HEADER.matcher(title);
+ if (matcher.matches()) {
+ title = matcher.group(1);
+ }
String outputFile = AsciiDoctor.mapInFileToOutFile(
inputFile, inExt, outExt);
