Display the size of a patch (lines added/removed)
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>
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritCss.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritCss.java
index c556770..dd68479 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritCss.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritCss.java
@@ -145,6 +145,7 @@
String patchSetLink();
String patchSetRevision();
String patchSetUserIdentity();
+ String patchSizeCell();
String permalink();
String posscore();
String projectAdminApprovalCategoryRangeLine();
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.java
index e339d26..abdaa15 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.java
@@ -54,6 +54,7 @@
String patchTableColumnName();
String patchTableColumnComments();
+ String patchTableColumnSize();
String patchTableColumnDiff();
String patchTableDiffSideBySide();
String patchTableDiffUnified();
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties
index ed6be47..7cd0208 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties
@@ -34,6 +34,7 @@
patchTableColumnName = File Path
patchTableColumnComments = Comments
+patchTableColumnSize = Size
patchTableColumnDiff = Diff
patchTableDiffSideBySide = Side-by-Side
patchTableDiffUnified = Unified
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java
index 51e2bc5..6c68ba9 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java
@@ -31,6 +31,8 @@
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);
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties
index 5d2d3a5..a4a1c31 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties
@@ -12,6 +12,8 @@
patchTableComments = {0} comments
patchTableDrafts = {0} drafts
+patchTableSize_Modify = +{0}, -{1}
+patchTableSize_Lines = {0} lines
removeReviewer = Remove reviewer {0}
messageWrittenOn = on {0}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages_en.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages_en.properties
index a5e9426..d9c7059 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages_en.properties
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages_en.properties
@@ -3,3 +3,6 @@
patchTableDrafts[one] = 1 draft
patchTableDrafts = {0} drafts
+
+patchTableSize_Lines[one] = 1 line
+patchTableSize_Lines = {0} lines
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchTable.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchTable.java
index ce95178..34695dd 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchTable.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchTable.java
@@ -22,6 +22,7 @@
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 @@
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 @@
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 @@
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 @@
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 @@
}
}
+ 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 @@
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 @@
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;
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/gerrit.css b/gerrit-gwtui/src/main/java/com/google/gerrit/client/gerrit.css
index d44aa78..b184064 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/gerrit.css
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/gerrit.css
@@ -415,6 +415,14 @@
color: #ff5555;
}
+.changeTable .patchSizeCell {
+ text-align: right;
+ white-space: nowrap;
+}
+.changeTable td.noborder {
+ border: none;
+}
+
.changeTable .filePathCell {
white-space: nowrap;
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/NavigationTable.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/NavigationTable.java
index e5cc0ea..0d45529 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/NavigationTable.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/NavigationTable.java
@@ -118,7 +118,7 @@
row++;
} else if (sEnd < cTop) {
row--;
- } else if (getRowItem(row) != null) {
+ } else {
break;
}
}
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/Patch.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/Patch.java
index 28bf53c..3a922fb 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/Patch.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/Patch.java
@@ -194,6 +194,12 @@
/** 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 @@
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);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchList.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchList.java
index a8e3cf8..6e68f27 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchList.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchList.java
@@ -61,6 +61,8 @@
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 @@
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 @@
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 @@
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 @@
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++) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListEntry.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListEntry.java
index 50d6120c..fd7da08 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListEntry.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListEntry.java
@@ -50,7 +50,7 @@
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 @@
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 @@
} 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 @@
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 @@
p.setChangeType(getChangeType());
p.setPatchType(getPatchType());
p.setSourceFileName(getOldName());
+ p.setInsertions(insertions);
+ p.setDeletions(deletions);
return p;
}
@@ -154,6 +178,8 @@
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 @@
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 @@
}
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 @@
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) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListKey.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListKey.java
index ffc9ac4..715b90a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListKey.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListKey.java
@@ -35,7 +35,7 @@
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;
