opendev/gerrit
Code Issues Proposed changes

Display the size of a patch (lines added/removed)

Browse Source 
In a patch table we now display the number of new lines added or the
number of lines deleted for each file, assuming it is not binary.
Added files only show the total number of lines, while deleted
files show nothing at all.

A new row is added at the bottom of the table that shows the overall
size of the delta.  This can be useful if a project has rules about
how big a patch can be before additional types of review are required
(e.g. Eclipse based projects).

Bug: issue 499
Change-Id: I961d8fac3f5d82a5d24f0a4d0b0a9ddf39182a50
Signed-off-by: Shawn O. Pearce <sop@google.com>
This commit is contained in:
Shawn O. Pearce
2010-08-21 19:04:56 -07:00
parent b6adfc74c3
commit d4b2787ed3
13 changed files with 169 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritCss.java
View File

@@ -145,6 +145,7 @@ public interface GerritCss extends CssResource {
String patchSetLink();
String patchSetRevision();
String patchSetUserIdentity();
String patchSizeCell();
String permalink();
String posscore();
String projectAdminApprovalCategoryRangeLine();

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.java
View File

@@ -54,6 +54,7 @@ public interface ChangeConstants extends Constants {
String patchTableColumnName();
String patchTableColumnComments();
String patchTableColumnSize();
String patchTableColumnDiff();
String patchTableDiffSideBySide();
String patchTableDiffUnified();

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties
View File

@@ -34,6 +34,7 @@ keyPublishComments = Review and publish comments
patchTableColumnName = File Path
patchTableColumnComments = Comments
patchTableColumnSize = Size
patchTableColumnDiff = Diff
patchTableDiffSideBySide = Side-by-Side
patchTableDiffUnified = Unified

2
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java
View File

@@ -31,6 +31,8 @@ public interface ChangeMessages extends Messages {
String patchTableComments(@PluralCount int count);
String patchTableDrafts(@PluralCount int count);
String patchTableSize_Modify(int insertions, int deletions);
String patchTableSize_Lines(@PluralCount int insertions);
String removeReviewer(String fullName);
String messageWrittenOn(String date);

2
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties
View File

@@ -12,6 +12,8 @@ submitPatchSet = Submit Patch Set {0}
patchTableComments = {0} comments
patchTableDrafts = {0} drafts
patchTableSize_Modify = +{0}, -{1}
patchTableSize_Lines = {0} lines
removeReviewer = Remove reviewer {0}
messageWrittenOn = on {0}

3
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages_en.properties
View File

@@ -3,3 +3,6 @@ patchTableComments = {0} comments
patchTableDrafts[one] = 1 draft
patchTableDrafts = {0} drafts
patchTableSize_Lines[one] = 1 line
patchTableSize_Lines = {0} lines

77
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchTable.java
View File

@@ -22,6 +22,7 @@ import com.google.gerrit.client.ui.PatchLink;
import com.google.gerrit.common.data.PatchSetDetail;
import com.google.gerrit.reviewdb.Patch;
import com.google.gerrit.reviewdb.Patch.Key;
import com.google.gerrit.reviewdb.Patch.PatchType;
import com.google.gwt.core.client.GWT;
import com.google.gwt.event.dom.client.ClickEvent;
import com.google.gwt.event.dom.client.ClickHandler;
@@ -242,7 +243,8 @@ public class PatchTable extends Composite {
private class MyTable extends NavigationTable<Patch> {
private static final int C_PATH = 2;
private static final int C_DRAFT = 3;
private static final int C_SIDEBYSIDE = 4;
private static final int C_SIZE = 4;
private static final int C_SIDEBYSIDE = 5;
private int activeRow = -1;
MyTable() {
@@ -404,6 +406,12 @@ public class PatchTable extends Composite {
m.append(Util.C.patchTableColumnComments());
m.closeTd();
// "Size"
m.openTd();
m.setStyleName(Gerrit.RESOURCES.css().dataHeader());
m.append(Util.C.patchTableColumnSize());
m.closeTd();
// "Diff"
m.openTd();
m.setStyleName(Gerrit.RESOURCES.css().dataHeader());
@@ -452,6 +460,12 @@ public class PatchTable extends Composite {
appendCommentCount(m, p);
m.closeTd();
m.openTd();
m.addStyleName(Gerrit.RESOURCES.css().dataCell());
m.addStyleName(Gerrit.RESOURCES.css().patchSizeCell());
appendSize(m, p);
m.closeTd();
switch (p.getPatchType()) {
case UNIFIED:
openlink(m, 2);
@@ -513,6 +527,29 @@ public class PatchTable extends Composite {
m.closeTr();
}
void appendTotals(final SafeHtmlBuilder m, int ins, int dels) {
m.openTr();
m.openTd();
m.addStyleName(Gerrit.RESOURCES.css().iconCell());
m.addStyleName(Gerrit.RESOURCES.css().noborder());
m.nbsp();
m.closeTd();
m.openTd();
m.setAttribute("colspan", C_SIZE - 1);
m.closeTd();
m.openTd();
m.addStyleName(Gerrit.RESOURCES.css().dataCell());
m.addStyleName(Gerrit.RESOURCES.css().patchSizeCell());
m.addStyleName(Gerrit.RESOURCES.css().leftMostCell());
m.append(Util.M.patchTableSize_Modify(ins, dels));
m.closeTd();
m.closeTr();
}
void appendCommentCount(final SafeHtmlBuilder m, final Patch p) {
if (p.getCommentCount() > 0) {
m.append(Util.M.patchTableComments(p.getCommentCount()));
@@ -528,6 +565,34 @@ public class PatchTable extends Composite {
}
}
void appendSize(final SafeHtmlBuilder m, final Patch p) {
if (Patch.COMMIT_MSG.equals(p.getFileName())) {
m.nbsp();
return;
}
if (p.getPatchType() == PatchType.UNIFIED) {
int ins = p.getInsertions();
int dels = p.getDeletions();
switch (p.getChangeType()) {
case ADDED:
m.append(Util.M.patchTableSize_Lines(ins));
break;
case DELETED:
m.nbsp();
break;
case MODIFIED:
case COPIED:
case RENAMED:
m.append(Util.M.patchTableSize_Modify(ins, dels));
break;
}
} else {
m.nbsp();
}
}
private void openlink(final SafeHtmlBuilder m, final int colspan) {
m.openTd();
m.addStyleName(Gerrit.RESOURCES.css().dataCell());
@@ -599,6 +664,9 @@ public class PatchTable extends Composite {
private double start;
private ProgressBar meter;
private int insertions;
private int deletions;
private DisplayCommand(final List<Patch> list) {
this.table = new MyTable();
this.list = list;
@@ -627,14 +695,19 @@ public class PatchTable extends Composite {
case 0:
if (row == 0) {
table.appendHeader(nc);
table.appendRow(nc, list.get(row++));
}
while (row < list.size()) {
table.appendRow(nc, list.get(row));
Patch p = list.get(row);
insertions += p.getInsertions();
deletions += p.getDeletions();
table.appendRow(nc, p);
if ((++row % 10) == 0 && longRunning()) {
updateMeter();
return true;
}
}
table.appendTotals(nc, insertions, deletions);
table.resetHtml(nc);
nc = null;
stage = 1;

8
gerrit-gwtui/src/main/java/com/google/gerrit/client/gerrit.css
View File

@@ -415,6 +415,14 @@
color: #ff5555;
}
.changeTable .patchSizeCell {
text-align: right;
white-space: nowrap;
}
.changeTable td.noborder {
border: none;
}
.changeTable .filePathCell {
white-space: nowrap;
}

2
gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/NavigationTable.java
View File

@@ -118,7 +118,7 @@ public abstract class NavigationTable<RowItem> extends FancyFlexTable<RowItem> {
row++;
} else if (sEnd < cTop) {
row--;
} else if (getRowItem(row) != null) {
} else {
break;
}
}

22
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/Patch.java
View File

@@ -194,6 +194,12 @@ public final class Patch {
/** Number of drafts by the current user; not persisted in the datastore. */
protected int nbrDrafts;
/** Number of lines added to the file. */
protected int insertions;
/** Number of lines deleted from the file. */
protected int deletions;
/**
* Original if {@link #changeType} is {@link ChangeType#COPIED} or
* {@link ChangeType#RENAMED}.
@@ -232,6 +238,22 @@ public final class Patch {
nbrDrafts = n;
}
public int getInsertions() {
return insertions;
}
public void setInsertions(int n) {
insertions = n;
}
public int getDeletions() {
return deletions;
}
public void setDeletions(int n) {
deletions = n;
}
public ChangeType getChangeType() {
return ChangeType.forCode(changeType);
}

20
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchList.java
View File

@@ -61,6 +61,8 @@ public class PatchList implements Serializable {
private transient ObjectId newId;
private transient boolean intralineDifference;
private transient boolean againstParent;
private transient int insertions;
private transient int deletions;
private transient PatchListEntry[] patches;
PatchList(@Nullable final AnyObjectId oldId, final AnyObjectId newId,
@@ -75,6 +77,10 @@ public class PatchList implements Serializable {
if (patches.length > 1) {
Arrays.sort(patches, 1, patches.length, PATCH_CMP);
}
for (int i = 1; i < patches.length; i++) {
insertions += patches[i].getInsertions();
deletions += patches[i].getDeletions();
}
this.patches = patches;
}
@@ -105,6 +111,16 @@ public class PatchList implements Serializable {
return againstParent;
}
/** @return total number of new lines added. */
public int getInsertions() {
return insertions;
}
/** @return total number of lines removed. */
public int getDeletions() {
return deletions;
}
/**
* Get a sorted, modifiable list of all files in this list.
* <p>
@@ -157,6 +173,8 @@ public class PatchList implements Serializable {
writeNotNull(out, newId);
writeVarInt32(out, intralineDifference ? 1 : 0);
writeVarInt32(out, againstParent ? 1 : 0);
writeVarInt32(out, insertions);
writeVarInt32(out, deletions);
writeVarInt32(out, patches.length);
for (PatchListEntry p : patches) {
p.writeTo(out);
@@ -175,6 +193,8 @@ public class PatchList implements Serializable {
newId = readNotNull(in);
intralineDifference = readVarInt32(in) != 0;
againstParent = readVarInt32(in) != 0;
insertions = readVarInt32(in);
deletions = readVarInt32(in);
final int cnt = readVarInt32(in);
final PatchListEntry[] all = new PatchListEntry[cnt];
for (int i = 0; i < all.length; i++) {

36
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListEntry.java
View File

@@ -50,7 +50,7 @@ public class PatchListEntry {
static PatchListEntry empty(final String fileName) {
return new PatchListEntry(ChangeType.MODIFIED, PatchType.UNIFIED, null,
fileName, EMPTY_HEADER, Collections.<Edit> emptyList());
fileName, EMPTY_HEADER, Collections.<Edit> emptyList(), 0, 0);
}
private final ChangeType changeType;
@@ -59,6 +59,8 @@ public class PatchListEntry {
private final String newName;
private final byte[] header;
private final List<Edit> edits;
private final int insertions;
private final int deletions;
PatchListEntry(final FileHeader hdr, List<Edit> editList) {
changeType = toChangeType(hdr);
@@ -96,17 +98,29 @@ public class PatchListEntry {
} else {
edits = Collections.unmodifiableList(editList);
}
int ins = 0;
int del = 0;
for (Edit e : editList) {
del += e.getEndA() - e.getBeginA();
ins += e.getEndB() - e.getBeginB();
}
insertions = ins;
deletions = del;
}
private PatchListEntry(final ChangeType changeType,
final PatchType patchType, final String oldName, final String newName,
final byte[] header, final List<Edit> edits) {
final byte[] header, final List<Edit> edits, final int insertions,
final int deletions) {
this.changeType = changeType;
this.patchType = patchType;
this.oldName = oldName;
this.newName = newName;
this.header = header;
this.edits = edits;
this.insertions = insertions;
this.deletions = deletions;
}
public ChangeType getChangeType() {
@@ -129,6 +143,14 @@ public class PatchListEntry {
return edits;
}
public int getInsertions() {
return insertions;
}
public int getDeletions() {
return deletions;
}
public List<String> getHeaderLines() {
final IntList m = RawParseUtils.lineMap(header, 0, header.length);
final List<String> headerLines = new ArrayList<String>(m.size() - 1);
@@ -145,6 +167,8 @@ public class PatchListEntry {
p.setChangeType(getChangeType());
p.setPatchType(getPatchType());
p.setSourceFileName(getOldName());
p.setInsertions(insertions);
p.setDeletions(deletions);
return p;
}
@@ -154,6 +178,8 @@ public class PatchListEntry {
writeString(out, oldName);
writeString(out, newName);
writeBytes(out, header);
writeVarInt32(out, insertions);
writeVarInt32(out, deletions);
writeVarInt32(out, edits.size());
for (final Edit e : edits) {
@@ -184,6 +210,8 @@ public class PatchListEntry {
final String oldName = readString(in);
final String newName = readString(in);
final byte[] hdr = readBytes(in);
final int ins = readVarInt32(in);
final int del = readVarInt32(in);
final int editCount = readVarInt32(in);
final Edit[] editArray = new Edit[editCount];
@@ -201,7 +229,7 @@ public class PatchListEntry {
}
return new PatchListEntry(changeType, patchType, oldName, newName, hdr,
toList(editArray));
toList(editArray), ins, del);
}
private static List<Edit> toList(Edit[] l) {
@@ -213,7 +241,7 @@ public class PatchListEntry {
final int endA = readVarInt32(in);
final int beginB = readVarInt32(in);
final int endB = readVarInt32(in);
return new Edit(beginA, endA, beginB, endB);
return new Edit(beginA, endA, beginB, endB);
}
private static byte[] compact(final FileHeader h) {

2
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListKey.java
View File

@@ -35,7 +35,7 @@ import java.io.Serializable;
import javax.annotation.Nullable;
public class PatchListKey implements Serializable {
static final long serialVersionUID = 14L;
static final long serialVersionUID = 15L;
private transient ObjectId oldId;
private transient ObjectId newId;