opendev/gerrit
Code Issues Proposed changes

SideBySide2: Support linking directly to a line of code or comment

Browse Source 
Using "@nnn" as a suffix on the URL token will open the file at
the given line on the B (right) side of the file. "@aNNN" can be
used to open the file relative to the line NNN on the A (left) side.

In the History section of ChangeScreen2 hyperlink each line number
to the line that comment appears on.

Bug: issue 348
Change-Id: I37de37d63d5deb6dc38f040e7e9b2bf5b09ef909
This commit is contained in:
Shawn Pearce
2013-12-18 16:54:49 -08:00
parent 9b28aca43a
commit f8e1c97c00
6 changed files with 83 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java
View File

@@ -70,6 +70,7 @@ import com.google.gerrit.client.changes.PublishCommentScreen;
import com.google.gerrit.client.changes.QueryScreen;
import com.google.gerrit.client.dashboards.DashboardInfo;
import com.google.gerrit.client.dashboards.DashboardList;
import com.google.gerrit.client.diff.DisplaySide;
import com.google.gerrit.client.diff.SideBySide2;
import com.google.gerrit.client.documentation.DocScreen;
import com.google.gerrit.client.groups.GroupApi;
@@ -109,12 +110,17 @@ public class Dispatcher {
public static String toSideBySide(PatchSet.Id diffBase,
PatchSet.Id revision, String fileName) {
return toPatch("", diffBase, revision, fileName);
return toPatch("", diffBase, revision, fileName, null, 0);
}
public static String toSideBySide(PatchSet.Id diffBase,
PatchSet.Id revision, String fileName, DisplaySide side, int line) {
return toPatch("", diffBase, revision, fileName, side, line);
}
public static String toUnified(PatchSet.Id diffBase,
PatchSet.Id revision, String fileName) {
return toPatch("unified", diffBase, revision, fileName);
return toPatch("unified", diffBase, revision, fileName, null, 0);
}
public static String toPatchUnified(final Patch.Key id) {
@@ -126,11 +132,11 @@ public class Dispatcher {
}
private static String toPatch(String type, PatchSet.Id diffBase, Patch.Key id) {
return toPatch(type, diffBase, id.getParentKey(), id.get());
return toPatch(type, diffBase, id.getParentKey(), id.get(), null, 0);
}
private static String toPatch(String type, PatchSet.Id diffBase,
PatchSet.Id revision, String fileName) {
PatchSet.Id revision, String fileName, DisplaySide side, int line) {
Change.Id c = revision.getParentKey();
StringBuilder p = new StringBuilder();
p.append("/c/").append(c).append("/");
@@ -141,6 +147,11 @@ public class Dispatcher {
if (type != null && !type.isEmpty()) {
p.append(",").append(type);
}
if (side == DisplaySide.A && line > 0) {
p.append("@a").append(line);
} else if (line > 0) {
p.append("@").append(line);
}
return p.toString();
}
@@ -521,8 +532,21 @@ public class Dispatcher {
}
if (!rest.isEmpty()) {
DisplaySide side = DisplaySide.B;
int line = 0;
int at = rest.lastIndexOf('@');
if (at > 0) {
String l = rest.substring(at+1);
if (l.startsWith("a")) {
side = DisplaySide.A;
l = l.substring(1);
}
line = Integer.parseInt(l);
rest = rest.substring(0, at);
}
Patch.Key p = new Patch.Key(ps, KeyUtil.decode(rest));
patch(token, base, p, 0, null, null, panel);
patch(token, base, p, side, line, 0,
null, null, null, panel);
} else {
if (panel == null) {
Gerrit.display(token, isChangeScreen2()
@@ -580,17 +604,12 @@ public class Dispatcher {
public static void patch(String token, PatchSet.Id base, Patch.Key id,
int patchIndex, PatchSetDetail patchSetDetail,
PatchTable patchTable, PatchScreen.TopView topView) {
patch(token, base, id, patchIndex, patchSetDetail, patchTable, topView, null);
}
public static void patch(String token, PatchSet.Id base, Patch.Key id,
int patchIndex, PatchSetDetail patchSetDetail,
PatchTable patchTable, String panelType) {
patch(token, base, id, patchIndex, patchSetDetail, patchTable,
null, panelType);
patch(token, base, id, null, 0, patchIndex,
patchSetDetail, patchTable, topView, null);
}
public static void patch(String token, final PatchSet.Id baseId, final Patch.Key id,
final DisplaySide side, final int line,
final int patchIndex, final PatchSetDetail patchSetDetail,
final PatchTable patchTable, final PatchScreen.TopView topView,
final String panelType) {
@@ -633,7 +652,8 @@ public class Dispatcher {
baseId //
);
}
return new SideBySide2(baseId, id.getParentKey(), id.get());
return new SideBySide2(baseId, id.getParentKey(), id.get(),
side, line);
} else if ("".equals(panel) || "sidebyside".equals(panel)) {
return new PatchScreen.SideBySide(//
id, //

2
gerrit-gwtui/src/main/java/com/google/gerrit/client/change/FileComments.java
View File

@@ -53,7 +53,7 @@ class FileComments extends Composite {
}
});
for (CommentInfo c : list) {
comments.add(new LineComment(clp, c));
comments.add(new LineComment(clp, ps, c));
}
}

34
gerrit-gwtui/src/main/java/com/google/gerrit/client/change/LineComment.java
View File

@@ -14,9 +14,13 @@
package com.google.gerrit.client.change;
import com.google.gerrit.client.Dispatcher;
import com.google.gerrit.client.changes.CommentInfo;
import com.google.gerrit.client.changes.Util;
import com.google.gerrit.client.diff.DisplaySide;
import com.google.gerrit.client.ui.CommentLinkProcessor;
import com.google.gerrit.client.ui.InlineHyperlink;
import com.google.gerrit.common.changes.Side;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gwt.core.client.GWT;
import com.google.gwt.dom.client.Element;
import com.google.gwt.uibinder.client.UiBinder;
@@ -29,18 +33,36 @@ class LineComment extends Composite {
interface Binder extends UiBinder<HTMLPanel, LineComment> {}
private static final Binder uiBinder = GWT.create(Binder.class);
@UiField Element location;
@UiField Element fileLoc;
@UiField Element lineLoc;
@UiField InlineHyperlink line;
@UiField Element message;
LineComment(CommentLinkProcessor clp, CommentInfo info) {
LineComment(CommentLinkProcessor clp, PatchSet.Id ps, CommentInfo info) {
initWidget(uiBinder.createAndBindUi(this));
location.setInnerText(info.has_line()
? Util.M.lineHeader(info.line())
: Util.C.fileCommentHeader());
if (info.has_line()) {
fileLoc.removeFromParent();
fileLoc = null;
line.setTargetHistoryToken(url(ps, info));
line.setText(Integer.toString(info.line()));
} else {
lineLoc.removeFromParent();
lineLoc = null;
line = null;
}
if (info.message() != null) {
message.setInnerSafeHtml(clp.apply(new SafeHtmlBuilder()
.append(info.message().trim()).wikify()));
}
}
private static String url(PatchSet.Id ps, CommentInfo info) {
return Dispatcher.toSideBySide(null, ps, info.path(),
info.side() == Side.PARENT ? DisplaySide.A : DisplaySide.B,
info.line());
}
}

5
gerrit-gwtui/src/main/java/com/google/gerrit/client/change/LineComment.ui.xml
View File

@@ -16,7 +16,7 @@ limitations under the License.
-->
<ui:UiBinder
xmlns:ui='urn:ui:com.google.gwt.uibinder'
xmlns:c='urn:import:com.google.gerrit.client'
xmlns:c='urn:import:com.google.gerrit.client.ui'
xmlns:g='urn:import:com.google.gwt.user.client.ui'>
<ui:style>
.box {
@@ -34,7 +34,8 @@ limitations under the License.
</ui:style>
<g:HTMLPanel styleName='{style.box}'>
<div class='{style.location}' ui:field='location'/>
<div class='{style.location}' ui:field='fileLoc'><ui:msg>File Comment</ui:msg></div>
<div class='{style.location}' ui:field='lineLoc'><ui:msg>Line <c:InlineHyperlink ui:field='line'/>:</ui:msg></div>
<div class='{style.message}' ui:field='message'/>
</g:HTMLPanel>
</ui:UiBinder>

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

@@ -15,6 +15,6 @@
package com.google.gerrit.client.diff;
/** Enum representing the side on a side-by-side view */
enum DisplaySide {
public enum DisplaySide {
A, B
}

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

@@ -119,6 +119,8 @@ public class SideBySide2 extends Screen {
private final PatchSet.Id base;
private final PatchSet.Id revision;
private final String path;
private final DisplaySide startSide;
private final int startLine;
private DiffPreferences prefs;
private CodeMirror cmA;
@@ -155,11 +157,15 @@ public class SideBySide2 extends Screen {
public SideBySide2(
PatchSet.Id base,
PatchSet.Id revision,
String path) {
String path,
DisplaySide startSide,
int startLine) {
this.base = base;
this.revision = revision;
this.changeId = revision.getParentKey();
this.path = path;
this.startSide = startSide;
this.startLine = startLine;
prefs = DiffPreferences.create(Gerrit.getAccountDiffPreference());
unsaved = new HashSet<DraftBox>();
@@ -267,7 +273,15 @@ public class SideBySide2 extends Screen {
});
diffTable.sidePanel.adjustGutters(cmB);
if (diff.meta_b() != null) {
if (startSide != null && startLine > 0) {
int line = startLine - 1;
CodeMirror cm = getCmFromSide(startSide);
if (cm.lineAtHeight(height - 20) < line) {
cm.scrollToY(cm.heightAtLine(line, "local") - 0.5 * height);
}
cm.setCursor(LineCharacter.create(line));
cm.focus();
} else if (diff.meta_b() != null) {
int line = 0;
if (!diffChunks.isEmpty()) {
DiffChunkInfo d = diffChunks.get(0);