Remove duplication in SideBySideCommentManager and UnifiedCommentManager
In the change that added the new unified diff screen [1],
SideBySideCommentManager and UnifiedCommentManager had a lot of
duplicate code. Extract common code to reduce duplication.
[1]: https://gerrit-review.googlesource.com/#/c/49171/
Change-Id: I3396d72b9146ddd4944a8226c9860022b0879c95
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 4fe2914..70ef947 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
@@ -57,7 +57,7 @@
CommentBox(CommentGroup group, CommentRange range) {
this.group = group;
if (range != null) {
- DiffScreen screen = group.getManager().getDiffScreen();
+ DiffScreen screen = group.getManager().host;
int startCmLine =
screen.getCmLine(range.startLine() - 1, group.getSide());
int endCmLine = screen.getCmLine(range.endLine() - 1, group.getSide());
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 7f4e446..a26b1ce 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
@@ -14,6 +14,7 @@
package com.google.gerrit.client.diff;
+import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.changes.CommentInfo;
import com.google.gerrit.client.patches.SkippedLine;
import com.google.gerrit.client.rpc.CallbackGroup;
@@ -27,11 +28,15 @@
import net.codemirror.lib.Pos;
import net.codemirror.lib.TextMarker.FromTo;
+import java.util.ArrayList;
+import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
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 DiffScreen}. */
abstract class CommentManager {
@@ -39,19 +44,23 @@
private final PatchSet.Id revision;
private final String path;
private final CommentLinkProcessor commentLinkProcessor;
-
+ final SortedMap<Integer, CommentGroup> sideA;
+ final SortedMap<Integer, CommentGroup> sideB;
private final Map<String, PublishedBox> published;
private final Set<DraftBox> unsavedDrafts;
+ final DiffScreen host;
private boolean attached;
private boolean expandAll;
private boolean open;
CommentManager(
+ DiffScreen host,
PatchSet.Id base,
PatchSet.Id revision,
String path,
CommentLinkProcessor clp,
boolean open) {
+ this.host = host;
this.base = base;
this.revision = revision;
this.path = path;
@@ -60,6 +69,8 @@
published = new HashMap<>();
unsavedDrafts = new HashSet<>();
+ sideA = new TreeMap<>();
+ sideB = new TreeMap<>();
}
void setAttached(boolean attached) {
@@ -142,27 +153,290 @@
return fromTo;
}
- abstract void insertNewDraft(DisplaySide side, int line);
+ abstract CommentGroup group(DisplaySide side, int cmLinePlusOne);
- abstract Runnable newDraftCallback(final CodeMirror cm);
+ /**
+ * 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.skipManager.ensureFirstLineIsVisible();
+ }
- abstract DraftBox addDraftBox(DisplaySide side, CommentInfo info);
+ 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(
+ getPath(),
+ getStoredSideFromDisplaySide(side),
+ line,
+ null)).setEdit(true);
+ }
+ }
- abstract void setExpandAllComments(boolean b);
+ abstract String getTokenSuffixForActiveLine(CodeMirror cm);
- abstract Runnable commentNav(CodeMirror src, Direction dir);
+ Runnable signInCallback(final CodeMirror cm) {
+ return new Runnable() {
+ @Override
+ public void run() {
+ String token = host.getToken();
+ if (cm.extras().hasActiveLine()) {
+ token += "@" + getTokenSuffixForActiveLine(cm);
+ }
+ Gerrit.doSignIn(token);
+ }
+ };
+ }
- abstract void clearLine(DisplaySide side, int line, CommentGroup group);
+ abstract void newDraft(CodeMirror cm);
- abstract void renderPublished(DisplaySide forSide, JsArray<CommentInfo> in);
+ Runnable newDraftCallback(final CodeMirror cm) {
+ if (!Gerrit.isSignedIn()) {
+ return signInCallback(cm);
+ }
- abstract List<SkippedLine> splitSkips(int context, List<SkippedLine> skips);
+ return new Runnable() {
+ @Override
+ public void run() {
+ if (cm.extras().hasActiveLine()) {
+ newDraft(cm);
+ }
+ }
+ };
+ }
- abstract void newDraftOnGutterClick(CodeMirror cm, String gutterClass, int line);
+ DraftBox addDraftBox(DisplaySide side, CommentInfo info) {
+ int cmLinePlusOne = host.getCmLine(info.line() - 1, side) + 1;
+ CommentGroup group = group(side, cmLinePlusOne);
+ DraftBox box = new DraftBox(
+ group,
+ getCommentLinkProcessor(),
+ getPatchSetIdFromSide(side),
+ info,
+ isExpandAll());
- abstract Runnable toggleOpenBox(final CodeMirror cm);
+ if (info.inReplyTo() != null) {
+ PublishedBox r = getPublished().get(info.inReplyTo());
+ if (r != null) {
+ r.setReplyBox(box);
+ }
+ }
- abstract Runnable openCloseAll(final CodeMirror cm);
+ group.add(box);
+ box.setAnnotation(host.getDiffTable().scrollbar.draft(
+ host.getCmFromSide(side),
+ Math.max(0, cmLinePlusOne - 1)));
+ return box;
+ }
- abstract DiffScreen getDiffScreen();
+ void setExpandAllComments(boolean b) {
+ setExpandAll(b);
+ for (CommentGroup g : sideA.values()) {
+ g.setOpenAll(b);
+ }
+ for (CommentGroup g : sideB.values()) {
+ g.setOpenAll(b);
+ }
+ }
+
+ abstract SortedMap<Integer, CommentGroup> getMapForNav(DisplaySide side);
+
+ 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 = getMapForNav(src.side());
+ int line = src.extras().hasActiveLine()
+ ? src.getLineNumber(src.extras().activeLine()) + 1
+ : 0;
+
+ CommentGroup g;
+ if (dir == Direction.NEXT) {
+ map = map.tailMap(line + 1);
+ if (map.isEmpty()) {
+ return;
+ }
+ g = map.get(map.firstKey());
+ while (g.getBoxCount() == 0) {
+ map = map.tailMap(map.firstKey() + 1);
+ if (map.isEmpty()) {
+ return;
+ }
+ g = map.get(map.firstKey());
+ }
+ } else {
+ map = map.headMap(line);
+ if (map.isEmpty()) {
+ return;
+ }
+ g = map.get(map.lastKey());
+ while (g.getBoxCount() == 0) {
+ map = map.headMap(map.lastKey());
+ if (map.isEmpty()) {
+ return;
+ }
+ g = map.get(map.lastKey());
+ }
+ }
+
+ CodeMirror cm = g.getCm();
+ double y = cm.heightAtLine(g.getLine() - 1, "local");
+ cm.setCursor(Pos.create(g.getLine() - 1));
+ cm.scrollToY(y - 0.5 * cm.scrollbarV().getClientHeight());
+ cm.focus();
+ }
+ };
+ }
+
+ void clearLine(DisplaySide side, int line, CommentGroup group) {
+ SortedMap<Integer, CommentGroup> map = map(side);
+ if (map.get(line) == group) {
+ map.remove(line);
+ }
+ }
+
+ void render(CommentsCollections in, boolean expandAll) {
+ if (in.publishedBase != null) {
+ renderPublished(DisplaySide.A, in.publishedBase);
+ }
+ if (in.publishedRevision != null) {
+ renderPublished(DisplaySide.B, in.publishedRevision);
+ }
+ if (in.draftsBase != null) {
+ renderDrafts(DisplaySide.A, in.draftsBase);
+ }
+ if (in.draftsRevision != null) {
+ renderDrafts(DisplaySide.B, in.draftsRevision);
+ }
+ if (expandAll) {
+ setExpandAllComments(true);
+ }
+ for (CommentGroup g : sideA.values()) {
+ g.init(host.getDiffTable());
+ }
+ for (CommentGroup g : sideB.values()) {
+ g.init(host.getDiffTable());
+ g.handleRedraw();
+ }
+ setAttached(true);
+ }
+
+ void renderPublished(DisplaySide forSide, JsArray<CommentInfo> in) {
+ for (CommentInfo info : Natives.asList(in)) {
+ DisplaySide side = displaySide(info, forSide);
+ if (side != null) {
+ int cmLinePlusOne = host.getCmLine(info.line() - 1, side) + 1;
+ CommentGroup group = group(side, cmLinePlusOne);
+ PublishedBox box = new PublishedBox(
+ group,
+ getCommentLinkProcessor(),
+ getPatchSetIdFromSide(side),
+ info,
+ side,
+ isOpen());
+ group.add(box);
+ box.setAnnotation(host.getDiffTable().scrollbar.comment(
+ host.getCmFromSide(side),
+ cmLinePlusOne - 1));
+ getPublished().put(info.id(), box);
+ }
+ }
+ }
+
+ abstract Collection<Integer> getLinesWithCommentGroups();
+
+ private static void checkAndAddSkip(List<SkippedLine> out, SkippedLine s) {
+ if (s.getSize() > 1) {
+ out.add(s);
+ }
+ }
+
+ List<SkippedLine> splitSkips(int context, List<SkippedLine> skips) {
+ if (sideA.containsKey(0) || sideB.containsKey(0)) {
+ // Special case of file comment; cannot skip first line.
+ for (SkippedLine skip : skips) {
+ if (skip.getStartA() == 0) {
+ skip.incrementStart(1);
+ break;
+ }
+ }
+ }
+
+ for (int boxLine : getLinesWithCommentGroups()) {
+ List<SkippedLine> temp = new ArrayList<>(skips.size() + 2);
+ for (SkippedLine skip : skips) {
+ int startLine = host.getCmLine(skip.getStartB(), DisplaySide.B);
+ int deltaBefore = boxLine - startLine;
+ int deltaAfter = startLine + skip.getSize() - boxLine;
+ if (deltaBefore < -context || deltaAfter < -context) {
+ temp.add(skip); // Size guaranteed to be greater than 1
+ } else if (deltaBefore > context && deltaAfter > context) {
+ SkippedLine before = new SkippedLine(
+ skip.getStartA(), skip.getStartB(),
+ skip.getSize() - deltaAfter - context);
+ skip.incrementStart(deltaBefore + context);
+ checkAndAddSkip(temp, before);
+ checkAndAddSkip(temp, skip);
+ } else if (deltaAfter > context) {
+ skip.incrementStart(deltaBefore + context);
+ checkAndAddSkip(temp, skip);
+ } else if (deltaBefore > context) {
+ skip.reduceSize(deltaAfter + context);
+ checkAndAddSkip(temp, skip);
+ }
+ }
+ if (temp.isEmpty()) {
+ return temp;
+ }
+ skips = temp;
+ }
+ return skips;
+ }
+
+ abstract void newDraftOnGutterClick(CodeMirror cm, String gutterClass,
+ int line);
+
+ abstract CommentGroup getCommentGroupOnActiveLine(CodeMirror cm);
+
+ Runnable toggleOpenBox(final CodeMirror cm) {
+ return new Runnable() {
+ @Override
+ public void run() {
+ CommentGroup group = getCommentGroupOnActiveLine(cm);
+ if (group != null) {
+ group.openCloseLast();
+ }
+ }
+ };
+ }
+
+ Runnable openCloseAll(final CodeMirror cm) {
+ return new Runnable() {
+ @Override
+ public void run() {
+ CommentGroup group = getCommentGroupOnActiveLine(cm);
+ if (group != null) {
+ group.openCloseAll();
+ }
+ }
+ };
+ }
+
+ SortedMap<Integer, CommentGroup> map(DisplaySide side) {
+ return side == DisplaySide.A ? sideA : sideB;
+ }
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DisplaySide.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DisplaySide.java
index 4dfcd8c..c7ee678 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DisplaySide.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DisplaySide.java
@@ -16,5 +16,9 @@
/** Enum representing the side on a side-by-side view */
public enum DisplaySide {
- A, B
+ A, B;
+
+ DisplaySide otherSide() {
+ return this == A ? B : A;
+ }
}
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 e497e8b..dc2e3a2 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
@@ -161,7 +161,7 @@
void doReply() {
if (!Gerrit.isSignedIn()) {
- Gerrit.doSignIn(getCommentManager().getDiffScreen().getToken());
+ Gerrit.doSignIn(getCommentManager().host.getToken());
} else if (replyBox == null) {
addReplyBox(false);
} else {
@@ -179,7 +179,7 @@
void onQuote(ClickEvent e) {
e.stopPropagation();
if (!Gerrit.isSignedIn()) {
- Gerrit.doSignIn(getCommentManager().getDiffScreen().getToken());
+ Gerrit.doSignIn(getCommentManager().host.getToken());
}
addReplyBox(true);
}
@@ -188,7 +188,7 @@
void onReplyDone(ClickEvent e) {
e.stopPropagation();
if (!Gerrit.isSignedIn()) {
- Gerrit.doSignIn(getCommentManager().getDiffScreen().getToken());
+ Gerrit.doSignIn(getCommentManager().host.getToken());
} else if (replyBox == null) {
done.setEnabled(false);
CommentInfo input = CommentInfo.createReply(comment);
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySideCommentManager.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySideCommentManager.java
index e18f7a5..a2af3a1c 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySideCommentManager.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySideCommentManager.java
@@ -16,321 +16,67 @@
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.changes.CommentInfo;
-import com.google.gerrit.client.patches.SkippedLine;
-import com.google.gerrit.client.rpc.Natives;
import com.google.gerrit.client.ui.CommentLinkProcessor;
import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gwt.core.client.JsArray;
import net.codemirror.lib.CodeMirror;
-import net.codemirror.lib.CodeMirror.LineHandle;
-import net.codemirror.lib.Pos;
import net.codemirror.lib.TextMarker.FromTo;
-import java.util.ArrayList;
-import java.util.List;
+import java.util.Collection;
import java.util.SortedMap;
-import java.util.TreeMap;
/** Tracks comment widgets for {@link SideBySide}. */
class SideBySideCommentManager extends CommentManager {
- private final SideBySide host;
- private final SortedMap<Integer, SideBySideCommentGroup> sideA;
- private final SortedMap<Integer, SideBySideCommentGroup> sideB;
-
SideBySideCommentManager(SideBySide host,
PatchSet.Id base, PatchSet.Id revision,
String path,
CommentLinkProcessor clp,
boolean open) {
- super(base, revision, path, clp, open);
-
- this.host = host;
- sideA = new TreeMap<>();
- sideB = new TreeMap<>();
+ super(host, base, revision, path, clp, open);
}
@Override
- SideBySide getDiffScreen() {
- return host;
- }
-
- @Override
- void setExpandAllComments(boolean b) {
- setExpandAll(b);
- for (SideBySideCommentGroup g : sideA.values()) {
- g.setOpenAll(b);
- }
- for (SideBySideCommentGroup g : sideB.values()) {
- g.setOpenAll(b);
- }
- }
-
- @Override
- 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, SideBySideCommentGroup> map = map(src.side());
- int line = src.extras().hasActiveLine()
- ? src.getLineNumber(src.extras().activeLine()) + 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();
- }
-
- SideBySideCommentGroup 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(Pos.create(g.getLine() - 1));
- cm.scrollToY(y - 0.5 * cm.scrollbarV().getClientHeight());
- cm.focus();
- }
- };
- }
-
- void render(CommentsCollections in, boolean expandAll) {
- if (in.publishedBase != null) {
- renderPublished(DisplaySide.A, in.publishedBase);
- }
- if (in.publishedRevision != null) {
- renderPublished(DisplaySide.B, in.publishedRevision);
- }
- if (in.draftsBase != null) {
- renderDrafts(DisplaySide.A, in.draftsBase);
- }
- if (in.draftsRevision != null) {
- renderDrafts(DisplaySide.B, in.draftsRevision);
- }
- if (expandAll) {
- setExpandAllComments(true);
- }
- for (SideBySideCommentGroup g : sideA.values()) {
- g.init(host.getDiffTable());
- }
- for (SideBySideCommentGroup g : sideB.values()) {
- g.init(host.getDiffTable());
- g.handleRedraw();
- }
- setAttached(true);
- }
-
- @Override
- void renderPublished(DisplaySide forSide, JsArray<CommentInfo> in) {
- for (CommentInfo info : Natives.asList(in)) {
- DisplaySide side = displaySide(info, forSide);
- if (side != null) {
- SideBySideCommentGroup group = group(side, info.line());
- PublishedBox box = new PublishedBox(
- group,
- getCommentLinkProcessor(),
- getPatchSetIdFromSide(side),
- info,
- side,
- isOpen());
- group.add(box);
- box.setAnnotation(host.getDiffTable().scrollbar.comment(
- host.getCmFromSide(side),
- Math.max(0, info.line() - 1)));
- getPublished().put(info.id(), box);
- }
- }
- }
-
- @Override
- void newDraftOnGutterClick(CodeMirror cm, String gutterClass, int line) {
- insertNewDraft(cm.side(), line);
- }
-
- /**
- * 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.
- */
- @Override
- void insertNewDraft(DisplaySide side, int line) {
- if (line == 0) {
- host.skipManager.ensureFirstLineIsVisible();
- }
-
- SideBySideCommentGroup 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(
- getPath(),
- getStoredSideFromDisplaySide(side),
- line,
- null)).setEdit(true);
- }
- }
-
- @Override
- DraftBox addDraftBox(DisplaySide side, CommentInfo info) {
- SideBySideCommentGroup group = group(side, info.line());
- DraftBox box = new DraftBox(
- group,
- getCommentLinkProcessor(),
- getPatchSetIdFromSide(side),
- info,
- isExpandAll());
-
- if (info.inReplyTo() != null) {
- PublishedBox r = getPublished().get(info.inReplyTo());
- if (r != null) {
- r.setReplyBox(box);
- }
- }
-
- group.add(box);
- box.setAnnotation(host.getDiffTable().scrollbar.draft(
- host.getCmFromSide(side),
- Math.max(0, info.line() - 1)));
- return box;
- }
-
- @Override
- 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 (int boxLine : sideB.tailMap(1).keySet()) {
- List<SkippedLine> temp = new ArrayList<>(skips.size() + 2);
- for (SkippedLine skip : skips) {
- int startLine = skip.getStartB();
- int deltaBefore = boxLine - startLine;
- int deltaAfter = startLine + skip.getSize() - boxLine;
- if (deltaBefore < -context || deltaAfter < -context) {
- temp.add(skip); // Size guaranteed to be greater than 1
- } else if (deltaBefore > context && deltaAfter > context) {
- SkippedLine before = new SkippedLine(
- skip.getStartA(), skip.getStartB(),
- skip.getSize() - deltaAfter - context);
- skip.incrementStart(deltaBefore + context);
- checkAndAddSkip(temp, before);
- checkAndAddSkip(temp, skip);
- } else if (deltaAfter > context) {
- skip.incrementStart(deltaBefore + context);
- checkAndAddSkip(temp, skip);
- } else if (deltaBefore > context) {
- skip.reduceSize(deltaAfter + context);
- checkAndAddSkip(temp, skip);
- }
- }
- if (temp.isEmpty()) {
- return temp;
- }
- skips = temp;
- }
- return skips;
- }
-
- private static void checkAndAddSkip(List<SkippedLine> out, SkippedLine s) {
- if (s.getSize() > 1) {
- out.add(s);
- }
+ SortedMap<Integer, CommentGroup> getMapForNav(DisplaySide side) {
+ return map(side);
}
@Override
void clearLine(DisplaySide side, int line, CommentGroup group) {
- SortedMap<Integer, SideBySideCommentGroup> map = map(side);
- if (map.get(line) == group) {
- map.remove(line);
- }
+ super.clearLine(side, line, group);
}
@Override
- Runnable toggleOpenBox(final CodeMirror cm) {
- return new Runnable() {
- @Override
- public void run() {
- if (cm.extras().hasActiveLine()) {
- SideBySideCommentGroup w = map(cm.side()).get(
- cm.getLineNumber(cm.extras().activeLine()) + 1);
- if (w != null) {
- w.openCloseLast();
- }
- }
- }
- };
- }
-
- @Override
- Runnable openCloseAll(final CodeMirror cm) {
- return new Runnable() {
- @Override
- public void run() {
- if (cm.extras().hasActiveLine()) {
- SideBySideCommentGroup w = map(cm.side()).get(
- cm.getLineNumber(cm.extras().activeLine()) + 1);
- if (w != null) {
- w.openCloseAll();
- }
- }
- }
- };
- }
-
- @Override
- Runnable newDraftCallback(final CodeMirror cm) {
+ void newDraftOnGutterClick(CodeMirror cm, String gutterClass, int line) {
if (!Gerrit.isSignedIn()) {
- return new Runnable() {
- @Override
- public void run() {
- String token = host.getToken();
- if (cm.extras().hasActiveLine()) {
- LineHandle handle = cm.extras().activeLine();
- int line = cm.getLineNumber(handle) + 1;
- token += "@" + (cm.side() == DisplaySide.A ? "a" : "") + line;
- }
- Gerrit.doSignIn(token);
- }
- };
+ signInCallback(cm).run();
+ } else {
+ insertNewDraft(cm.side(), line);
}
-
- return new Runnable() {
- @Override
- public void run() {
- if (cm.extras().hasActiveLine()) {
- newDraft(cm);
- }
- }
- };
}
- private void newDraft(CodeMirror cm) {
+ @Override
+ CommentGroup getCommentGroupOnActiveLine(CodeMirror cm) {
+ CommentGroup group = null;
+ if (cm.extras().hasActiveLine()) {
+ group = map(cm.side())
+ .get(cm.getLineNumber(cm.extras().activeLine()) + 1);
+ }
+ return group;
+ }
+
+ @Override
+ Collection<Integer> getLinesWithCommentGroups() {
+ return sideB.tailMap(1).keySet();
+ }
+
+ @Override
+ String getTokenSuffixForActiveLine(CodeMirror cm) {
+ return (cm.side() == DisplaySide.A ? "a" : "")
+ + (cm.getLineNumber(cm.extras().activeLine()) + 1);
+ }
+
+ @Override
+ void newDraft(CodeMirror cm) {
int line = cm.getLineNumber(cm.extras().activeLine()) + 1;
if (cm.somethingSelected()) {
FromTo fromTo = adjustSelection(cm);
@@ -346,8 +92,9 @@
}
}
- private SideBySideCommentGroup group(DisplaySide side, int line) {
- SideBySideCommentGroup w = map(side).get(line);
+ @Override
+ CommentGroup group(DisplaySide side, int line) {
+ SideBySideCommentGroup w = (SideBySideCommentGroup) map(side).get(line);
if (w != null) {
return w;
}
@@ -382,8 +129,4 @@
private SideBySideCommentGroup newGroup(DisplaySide side, int line) {
return new SideBySideCommentGroup(this, host.getCmFromSide(side), side, line);
}
-
- private SortedMap<Integer, SideBySideCommentGroup> map(DisplaySide side) {
- return side == DisplaySide.A ? sideA : sideB;
- }
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/UnifiedCommentManager.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/UnifiedCommentManager.java
index 9fae92c..8968bc7 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/UnifiedCommentManager.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/UnifiedCommentManager.java
@@ -19,334 +19,116 @@
import com.google.gerrit.client.diff.LineMapper.LineOnOtherInfo;
import com.google.gerrit.client.diff.UnifiedChunkManager.LineRegionInfo;
import com.google.gerrit.client.diff.UnifiedChunkManager.RegionType;
-import com.google.gerrit.client.patches.SkippedLine;
-import com.google.gerrit.client.rpc.Natives;
import com.google.gerrit.client.ui.CommentLinkProcessor;
import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gwt.core.client.JsArray;
import net.codemirror.lib.CodeMirror;
-import net.codemirror.lib.CodeMirror.LineHandle;
import net.codemirror.lib.Pos;
import net.codemirror.lib.TextMarker.FromTo;
-import java.util.ArrayList;
-import java.util.List;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
import java.util.SortedMap;
import java.util.TreeMap;
-import java.util.TreeSet;
/** Tracks comment widgets for {@link Unified}. */
class UnifiedCommentManager extends CommentManager {
- private final Unified host;
- private final SortedMap<Integer, UnifiedCommentGroup> sideA;
- private final SortedMap<Integer, UnifiedCommentGroup> sideB;
+
+ private final SortedMap<Integer, CommentGroup> mergedMap;
+
+ // In Unified, a CodeMirror line can have up to two CommentGroups - one for
+ // the base side and one for the revision, so we need to keep track of the
+ // duplicates and replace the entries in mergedMap on draft removal.
+ private final Map<Integer, CommentGroup> duplicates;
UnifiedCommentManager(Unified host,
PatchSet.Id base, PatchSet.Id revision,
String path,
CommentLinkProcessor clp,
boolean open) {
- super(base, revision, path, clp, open);
-
- this.host = host;
- sideA = new TreeMap<>();
- sideB = new TreeMap<>();
+ super(host, base, revision, path, clp, open);
+ mergedMap = new TreeMap<>();
+ duplicates = new HashMap<>();
}
@Override
- Unified getDiffScreen() {
- return host;
+ SortedMap<Integer, CommentGroup> getMapForNav(DisplaySide side) {
+ return mergedMap;
}
@Override
- void setExpandAllComments(boolean b) {
- setExpandAll(b);
- for (UnifiedCommentGroup g : sideA.values()) {
- g.setOpenAll(b);
- }
- for (UnifiedCommentGroup g : sideB.values()) {
- g.setOpenAll(b);
- }
- }
+ void clearLine(DisplaySide side, int line, CommentGroup group) {
+ super.clearLine(side, line, group);
- @Override
- Runnable commentNav(final CodeMirror src, final Direction dir) {
- return new Runnable() {
- @Override
- public void run() {
- SortedMap<Integer, UnifiedCommentGroup> map = map(src.side());
- int line = src.extras().hasActiveLine()
- ? src.getLineNumber(src.extras().activeLine()) + 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();
- }
-
- UnifiedCommentGroup g = map.get(line);
- CodeMirror cm = g.getCm();
- double y = cm.heightAtLine(g.getLine() - 1, "local");
- cm.setCursor(Pos.create(g.getLine() - 1));
- cm.scrollToY(y - 0.5 * cm.scrollbarV().getClientHeight());
- cm.focus();
- }
- };
- }
-
- void render(CommentsCollections in, boolean expandAll) {
- if (in.publishedBase != null) {
- renderPublished(DisplaySide.A, in.publishedBase);
- }
- if (in.publishedRevision != null) {
- renderPublished(DisplaySide.B, in.publishedRevision);
- }
- if (in.draftsBase != null) {
- renderDrafts(DisplaySide.A, in.draftsBase);
- }
- if (in.draftsRevision != null) {
- renderDrafts(DisplaySide.B, in.draftsRevision);
- }
- if (expandAll) {
- setExpandAllComments(true);
- }
- for (CommentGroup g : sideA.values()) {
- g.init(host.getDiffTable());
- }
- for (CommentGroup g : sideB.values()) {
- g.init(host.getDiffTable());
- g.handleRedraw();
- }
- setAttached(true);
- }
-
- @Override
- void renderPublished(DisplaySide forSide, JsArray<CommentInfo> in) {
- for (CommentInfo info : Natives.asList(in)) {
- DisplaySide side = displaySide(info, forSide);
- if (side != null) {
- int cmLinePlusOne = host.getCmLine(info.line() - 1, side) + 1;
- UnifiedCommentGroup group = group(side, cmLinePlusOne);
- PublishedBox box = new PublishedBox(
- group,
- getCommentLinkProcessor(),
- getPatchSetIdFromSide(side),
- info,
- side,
- isOpen());
- group.add(box);
- box.setAnnotation(getDiffScreen().getDiffTable().scrollbar.comment(
- host.getCm(),
- cmLinePlusOne));
- getPublished().put(info.id(), box);
+ if (mergedMap.get(line) == group) {
+ mergedMap.remove(line);
+ if (duplicates.containsKey(line)) {
+ mergedMap.put(line, duplicates.remove(line));
}
}
}
@Override
- void newDraftOnGutterClick(CodeMirror cm, String gutterClass, int cmLinePlusOne) {
- DisplaySide side = gutterClass.equals(UnifiedTable.style.lineNumbersLeft())
- ? DisplaySide.A
- : DisplaySide.B;
- insertNewDraft(side, cmLinePlusOne);
- }
-
- /**
- * Create a new {@link DraftBox} at the specified line and focus it.
- *
- * @param side which side the draft will appear on.
- * @param cmLinePlusOne the line the draft will be at, plus one.
- * Lines are 1-based. Line 0 is a special case creating a file level comment.
- */
- @Override
- void insertNewDraft(DisplaySide side, int cmLinePlusOne) {
- if (cmLinePlusOne == 0) {
- getDiffScreen().skipManager.ensureFirstLineIsVisible();
- }
-
- CommentGroup group = group(side, cmLinePlusOne);
- if (0 < group.getBoxCount()) {
- CommentBox last = group.getCommentBox(group.getBoxCount() - 1);
- if (last instanceof DraftBox) {
- ((DraftBox)last).setEdit(true);
- } else {
- ((PublishedBox)last).doReply();
- }
+ void newDraftOnGutterClick(CodeMirror cm, String gutterClass,
+ int cmLinePlusOne) {
+ if (!Gerrit.isSignedIn()) {
+ signInCallback(cm).run();
} else {
- LineRegionInfo info = host.getLineRegionInfoFromCmLine(cmLinePlusOne - 1);
+ LineRegionInfo info =
+ ((Unified) host).getLineRegionInfoFromCmLine(cmLinePlusOne - 1);
+ DisplaySide side =
+ gutterClass.equals(UnifiedTable.style.lineNumbersLeft())
+ ? DisplaySide.A
+ : DisplaySide.B;
int line = info.line;
if (info.getSide() != side) {
line = host.lineOnOther(info.getSide(), line).getLine();
}
- addDraftBox(side, CommentInfo.create(
- getPath(),
- getStoredSideFromDisplaySide(side),
- line + 1,
- null)).setEdit(true);
+ insertNewDraft(side, line + 1);
}
}
@Override
- DraftBox addDraftBox(DisplaySide side, CommentInfo info) {
- int cmLinePlusOne = host.getCmLine(info.line() - 1, side) + 1;
- UnifiedCommentGroup group = group(side, cmLinePlusOne);
- DraftBox box = new DraftBox(
- group,
- getCommentLinkProcessor(),
- getPatchSetIdFromSide(side),
- info,
- isExpandAll());
-
- if (info.inReplyTo() != null) {
- PublishedBox r = getPublished().get(info.inReplyTo());
- if (r != null) {
- r.setReplyBox(box);
- }
+ CommentGroup getCommentGroupOnActiveLine(CodeMirror cm) {
+ CommentGroup group = null;
+ if (cm.extras().hasActiveLine()) {
+ int cmLinePlusOne = cm.getLineNumber(cm.extras().activeLine()) + 1;
+ LineRegionInfo info =
+ ((Unified) host).getLineRegionInfoFromCmLine(cmLinePlusOne - 1);
+ CommentGroup forSide = map(info.getSide()).get(cmLinePlusOne);
+ group = forSide == null
+ ? map(info.getSide().otherSide()).get(cmLinePlusOne)
+ : forSide;
}
-
- group.add(box);
- box.setAnnotation(getDiffScreen().getDiffTable().scrollbar.draft(
- host.getCm(),
- cmLinePlusOne));
- return box;
+ return group;
}
@Override
- List<SkippedLine> splitSkips(int context, List<SkippedLine> skips) {
- if (sideA.containsKey(0) || sideB.containsKey(0)) {
- // Special case of file comment; cannot skip first line.
- for (SkippedLine skip : skips) {
- if (skip.getStartA() == 0) {
- skip.incrementStart(1);
- }
- }
- }
-
- TreeSet<Integer> allBoxLines = new TreeSet<>(sideA.tailMap(1).keySet());
- allBoxLines.addAll(sideB.tailMap(1).keySet());
- for (int boxLine : allBoxLines) {
- List<SkippedLine> temp = new ArrayList<>(skips.size() + 2);
- for (SkippedLine skip : skips) {
- int startLine = host.getCmLine(skip.getStartA(), DisplaySide.A);
- int deltaBefore = boxLine - startLine;
- int deltaAfter = startLine + skip.getSize() - boxLine;
- if (deltaBefore < -context || deltaAfter < -context) {
- temp.add(skip); // Size guaranteed to be greater than 1
- } else if (deltaBefore > context && deltaAfter > context) {
- SkippedLine before = new SkippedLine(
- skip.getStartA(), skip.getStartB(),
- skip.getSize() - deltaAfter - context);
- skip.incrementStart(deltaBefore + context);
- checkAndAddSkip(temp, before);
- checkAndAddSkip(temp, skip);
- } else if (deltaAfter > context) {
- skip.incrementStart(deltaBefore + context);
- checkAndAddSkip(temp, skip);
- } else if (deltaBefore > context) {
- skip.reduceSize(deltaAfter + context);
- checkAndAddSkip(temp, skip);
- }
- }
- if (temp.isEmpty()) {
- return temp;
- }
- skips = temp;
- }
- return skips;
- }
-
- private static void checkAndAddSkip(List<SkippedLine> out, SkippedLine s) {
- if (s.getSize() > 1) {
- out.add(s);
- }
+ Collection<Integer> getLinesWithCommentGroups() {
+ return mergedMap.tailMap(1).keySet();
}
@Override
- void clearLine(DisplaySide side, int cmLinePlusOne, CommentGroup group) {
- SortedMap<Integer, UnifiedCommentGroup> map = map(side);
- if (map.get(cmLinePlusOne) == group) {
- map.remove(cmLinePlusOne);
- }
+ String getTokenSuffixForActiveLine(CodeMirror cm) {
+ int cmLinePlusOne = cm.getLineNumber(cm.extras().activeLine()) + 1;
+ LineRegionInfo info =
+ ((Unified) host).getLineRegionInfoFromCmLine(cmLinePlusOne - 1);
+ return (info.getSide() == DisplaySide.A ? "a" : "") + cmLinePlusOne;
}
@Override
- Runnable toggleOpenBox(final CodeMirror cm) {
- return new Runnable() {
- @Override
- public void run() {
- if (cm.extras().hasActiveLine()) {
- UnifiedCommentGroup w = map(cm.side()).get(
- cm.getLineNumber(cm.extras().activeLine()) + 1);
- if (w != null) {
- w.openCloseLast();
- }
- }
- }
- };
- }
-
- @Override
- Runnable openCloseAll(final CodeMirror cm) {
- return new Runnable() {
- @Override
- public void run() {
- if (cm.extras().hasActiveLine()) {
- CommentGroup w = map(cm.side()).get(
- cm.getLineNumber(cm.extras().activeLine()) + 1);
- if (w != null) {
- w.openCloseAll();
- }
- }
- }
- };
- }
-
- @Override
- Runnable newDraftCallback(final CodeMirror cm) {
- if (!Gerrit.isSignedIn()) {
- return new Runnable() {
- @Override
- public void run() {
- String token = host.getToken();
- if (cm.extras().hasActiveLine()) {
- LineHandle handle = cm.extras().activeLine();
- int line = cm.getLineNumber(handle) + 1;
- token += "@" + line;
- }
- Gerrit.doSignIn(token);
- }
- };
- }
-
- return new Runnable() {
- @Override
- public void run() {
- if (cm.extras().hasActiveLine()) {
- newDraft(cm);
- }
- }
- };
- }
-
- private void newDraft(CodeMirror cm) {
+ void newDraft(CodeMirror cm) {
if (cm.somethingSelected()) {
FromTo fromTo = adjustSelection(cm);
Pos from = fromTo.from();
Pos to = fromTo.to();
- UnifiedChunkManager manager = host.getChunkManager();
+ Unified unified = (Unified) host;
+ UnifiedChunkManager manager = unified.getChunkManager();
LineRegionInfo fromInfo =
- host.getLineRegionInfoFromCmLine(from.line());
+ unified.getLineRegionInfoFromCmLine(from.line());
LineRegionInfo toInfo =
- host.getLineRegionInfoFromCmLine(to.line());
+ unified.getLineRegionInfoFromCmLine(to.line());
DisplaySide side = toInfo.getSide();
// Handle special cases in selections that span multiple regions. Force
@@ -398,33 +180,33 @@
cm.setSelection(cm.getCursor());
} else {
int cmLine = cm.getLineNumber(cm.extras().activeLine());
- LineRegionInfo info = host.getLineRegionInfoFromCmLine(cmLine);
+ LineRegionInfo info =
+ ((Unified) host).getLineRegionInfoFromCmLine(cmLine);
insertNewDraft(info.getSide(), cmLine + 1);
}
}
- private UnifiedCommentGroup group(DisplaySide side, int cmLinePlusOne) {
- UnifiedCommentGroup w = map(side).get(cmLinePlusOne);
- if (w != null) {
- return w;
+ @Override
+ CommentGroup group(DisplaySide side, int cmLinePlusOne) {
+ Map<Integer, CommentGroup> map = map(side);
+ CommentGroup existing = map.get(cmLinePlusOne);
+ if (existing != null) {
+ return existing;
}
- UnifiedCommentGroup g = new UnifiedCommentGroup(this, host.getCm(), side, cmLinePlusOne);
- if (side == DisplaySide.A) {
- sideA.put(cmLinePlusOne, g);
- } else {
- sideB.put(cmLinePlusOne, g);
+ UnifiedCommentGroup g = new UnifiedCommentGroup(
+ this, host.getCmFromSide(side), side, cmLinePlusOne);
+ map.put(cmLinePlusOne, g);
+ if (mergedMap.containsKey(cmLinePlusOne)) {
+ duplicates.put(cmLinePlusOne, mergedMap.remove(cmLinePlusOne));
}
+ mergedMap.put(cmLinePlusOne, g);
if (isAttached()) {
- g.init(getDiffScreen().getDiffTable());
+ g.init(host.getDiffTable());
g.handleRedraw();
}
return g;
}
-
- private SortedMap<Integer, UnifiedCommentGroup> map(DisplaySide side) {
- return side == DisplaySide.A ? sideA : sideB;
- }
}
