Make DiffChunkInfo implement Comparable to simplify code The start line on the other side of a DiffChunkInfo is always known at chunk creation. Pass it to the constructor and make DiffChunkInfo implement Comparable. This both simplifies code and avoids unnecessary LineMapper lookups. Change the fields in DiffChunkInfo to be final and remove the getters. This simplifies code a bit more. Change-Id: I556c94f9c2a68dda5b81bf303f42f73d981dc465
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 91533ab..092009d 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
@@ -25,7 +25,6 @@ import net.codemirror.lib.TextMarker; import java.util.ArrayList; -import java.util.Comparator; import java.util.List; /** Colors modified regions for {@link SideBySide} and {@link Unified}. */ @@ -110,7 +109,7 @@ DiffChunkInfo lookUp = chunks.get(res); // If edit, skip the deletion chunk and set focus on the insertion one. - if (lookUp.isEdit() && lookUp.getSide() == A) { + if (lookUp.edit && lookUp.side == A) { res = res + (dir == Direction.PREV ? -1 : 1); if (res < 0 || chunks.size() <= res) { return; @@ -118,8 +117,8 @@ } DiffChunkInfo target = chunks.get(res); - CodeMirror targetCm = host.getCmFromSide(target.getSide()); - int cmLine = getCmLine(target.getStart(), target.getSide()); + CodeMirror targetCm = host.getCmFromSide(target.side); + int cmLine = getCmLine(target.start, target.side); targetCm.setCursor(Pos.create(cmLine)); targetCm.focus(); targetCm.scrollToY( @@ -127,28 +126,5 @@ - 0.5 * targetCm.scrollbarV().getClientHeight()); } - Comparator<DiffChunkInfo> getDiffChunkComparator() { - // Chunks are ordered by their starting line. If it's a deletion, - // use its corresponding line on the revision side for comparison. - // In the edit case, put the deletion chunk right before the - // insertion chunk. This placement guarantees well-ordering. - return new Comparator<DiffChunkInfo>() { - @Override - public int compare(DiffChunkInfo a, DiffChunkInfo b) { - if (a.getSide() == b.getSide()) { - return a.getStart() - b.getStart(); - } else if (a.getSide() == A) { - int comp = mapper.lineOnOther(a.getSide(), a.getStart()) - .getLine() - b.getStart(); - return comp == 0 ? -1 : comp; - } else { - int comp = a.getStart() - - mapper.lineOnOther(b.getSide(), b.getStart()).getLine(); - return comp == 0 ? 1 : comp; - } - } - }; - } - abstract int getCmLine(int line, DisplaySide side); } \ No newline at end of file
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffChunkInfo.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffChunkInfo.java index 1e5c5e5..4725b1e 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffChunkInfo.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffChunkInfo.java
@@ -15,32 +15,39 @@ package com.google.gerrit.client.diff; /** Object recording the position of a diff chunk and whether it's an edit */ -class DiffChunkInfo { - private DisplaySide side; - private int start; - private int end; - private boolean edit; +class DiffChunkInfo implements Comparable<DiffChunkInfo> { + final DisplaySide side; + final int start; + final int end; + final boolean edit; - DiffChunkInfo(DisplaySide side, int start, int end, boolean edit) { + private final int startOnOther; + + DiffChunkInfo(DisplaySide side, int start, int startOnOther, int end, + boolean edit) { this.side = side; this.start = start; + this.startOnOther = startOnOther; this.end = end; this.edit = edit; } - DisplaySide getSide() { - return side; - } - - int getStart() { - return start; - } - - int getEnd() { - return end; - } - - boolean isEdit() { - return edit; + /** + * Chunks are ordered by their starting line. If it's a deletion, use its + * corresponding line on the revision side for comparison. In the edit case, + * put the deletion chunk right before the insertion chunk. This placement + * guarantees well-ordering. + */ + @Override + public int compareTo(DiffChunkInfo o) { + if (side == o.side) { + return start - o.start; + } else if (side == DisplaySide.A) { + int comp = startOnOther - o.start; + return comp == 0 ? -1 : comp; + } else { + int comp = start - o.startOnOther; + return comp == 0 ? 1 : comp; + } } } \ No newline at end of file
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide.java index a7689e9..34121b97 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide.java
@@ -117,12 +117,12 @@ if (getStartLine() == 0) { DiffChunkInfo d = chunkManager.getFirst(); if (d != null) { - if (d.isEdit() && d.getSide() == DisplaySide.A) { + if (d.edit && d.side == DisplaySide.A) { setStartSide(DisplaySide.B); - setStartLine(lineOnOther(d.getSide(), d.getStart()).getLine() + 1); + setStartLine(lineOnOther(d.side, d.start).getLine() + 1); } else { - setStartSide(d.getSide()); - setStartLine(d.getStart() + 1); + setStartSide(d.side); + setStartLine(d.start + 1); } } }
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySideChunkManager.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySideChunkManager.java index cbbafad..0b76521 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySideChunkManager.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySideChunkManager.java
@@ -174,10 +174,10 @@ int endA = mapper.getLineA() - 1; int endB = mapper.getLineB() - 1; if (aLen > 0) { - addDiffChunk(cmB, endA, aLen, bLen > 0); + addDiffChunk(cmB, endB, endA, aLen, bLen > 0); } if (bLen > 0) { - addDiffChunk(cmA, endB, bLen, aLen > 0); + addDiffChunk(cmA, endA, endB, bLen, aLen > 0); } } @@ -250,10 +250,10 @@ } } - private void addDiffChunk(CodeMirror cmToPad, int lineOnOther, + private void addDiffChunk(CodeMirror cmToPad, int line, int lineOnOther, int chunkSize, boolean edit) { chunks.add(new DiffChunkInfo(host.otherCm(cmToPad).side(), - lineOnOther - chunkSize + 1, lineOnOther, edit)); + lineOnOther - chunkSize + 1, line - chunkSize + 1, lineOnOther, edit)); } @Override @@ -266,8 +266,7 @@ : 0; int res = Collections.binarySearch( chunks, - new DiffChunkInfo(cm.side(), line, 0, false), - getDiffChunkComparator()); + new DiffChunkInfo(cm.side(), line, 0, 0, false)); diffChunkNavHelper(chunks, host, res, dir); } };
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Unified.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Unified.java index 96e6794..9b04d6f 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Unified.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Unified.java
@@ -119,12 +119,12 @@ if (getStartLine() == 0) { DiffChunkInfo d = chunkManager.getFirst(); if (d != null) { - if (d.isEdit() && d.getSide() == DisplaySide.A) { + if (d.edit && d.side == DisplaySide.A) { setStartSide(DisplaySide.B); } else { - setStartSide(d.getSide()); + setStartSide(d.side); } - setStartLine(chunkManager.getCmLine(d.getStart(), d.getSide()) + 1); + setStartLine(chunkManager.getCmLine(d.start, d.side) + 1); } } if (getStartSide() != null && getStartLine() > 0) {
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/UnifiedChunkManager.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/UnifiedChunkManager.java index 1e6ed0c..1107ece 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/UnifiedChunkManager.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/UnifiedChunkManager.java
@@ -142,13 +142,13 @@ int endA = mapper.getLineA() - 1; int endB = mapper.getLineB() - 1; if (aLen > 0) { - addDiffChunk(DisplaySide.A, endA, aLen, cmLine, bLen > 0); + addDiffChunk(DisplaySide.A, endA, endB, aLen, cmLine, bLen > 0); for (int j = 0; j < aLen; j++) { host.setLineNumber(DisplaySide.A, cmLine + j, startA + j + 1); } } if (bLen > 0) { - addDiffChunk(DisplaySide.B, endB, bLen, cmLine + aLen, aLen > 0); + addDiffChunk(DisplaySide.B, endB, endA, bLen, cmLine + aLen, aLen > 0); for (int j = 0; j < bLen; j++) { host.setLineNumber(DisplaySide.B, cmLine + aLen + j, startB + j + 1); } @@ -209,10 +209,10 @@ : UnifiedTable.style.diffInsert(); } - private void addDiffChunk(DisplaySide side, int chunkEnd, int chunkSize, - int cmLine, boolean edit) { - chunks.add(new UnifiedDiffChunkInfo(side, chunkEnd - chunkSize + 1, chunkEnd, - cmLine, edit)); + private void addDiffChunk(DisplaySide side, int chunkEnd, int otherChunkEnd, + int chunkSize, int cmLine, boolean edit) { + chunks.add(new UnifiedDiffChunkInfo(side, chunkEnd - chunkSize + 1, + otherChunkEnd - chunkSize + 1, chunkEnd, cmLine, edit)); } @Override @@ -225,7 +225,7 @@ : 0; int res = Collections.binarySearch( chunks, - new UnifiedDiffChunkInfo(cm.side(), 0, 0, line, false), + new UnifiedDiffChunkInfo(cm.side(), 0, 0, 0, line, false), getDiffChunkComparatorCmLine()); diffChunkNavHelper(chunks, host, res, dir); } @@ -237,7 +237,7 @@ return new Comparator<UnifiedDiffChunkInfo>() { @Override public int compare(UnifiedDiffChunkInfo o1, UnifiedDiffChunkInfo o2) { - return o1.getCmLine() - o2.getCmLine(); + return o1.cmLine - o2.cmLine; } }; } @@ -247,31 +247,30 @@ int res = Collections.binarySearch(chunks, new UnifiedDiffChunkInfo( - side, line, 0, 0, false), // Dummy DiffChunkInfo - getDiffChunkComparator()); + side, line, 0, 0, 0, false)); // Dummy DiffChunkInfo if (res >= 0) { - return chunks.get(res).getCmLine(); + return chunks.get(res).cmLine; } else { // The line might be within a DiffChunk res = -res - 1; if (res > 0) { UnifiedDiffChunkInfo info = chunks.get(res - 1); - if (side == DisplaySide.A && info.isEdit() - && info.getSide() == DisplaySide.B) { + if (side == DisplaySide.A && info.edit + && info.side == DisplaySide.B) { // Need to use the start and cmLine of the deletion chunk UnifiedDiffChunkInfo delete = chunks.get(res - 2); - if (line <= delete.getEnd()) { - return delete.getCmLine() + line - delete.getStart(); + if (line <= delete.end) { + return delete.cmLine + line - delete.start; } else { // Need to add the length of the insertion chunk - return delete.getCmLine() + line - delete.getStart() - + info.getEnd() - info.getStart() + 1; + return delete.cmLine + line - delete.start + + info.end - info.start + 1; } - } else if (side == info.getSide()) { - return info.getCmLine() + line - info.getStart(); + } else if (side == info.side) { + return info.cmLine + line - info.start; } else { - return info.getCmLine() + return info.cmLine + getLineMapper().lineOnOther(side, line).getLine() - - info.getStart(); + - info.start; } } else { return line; @@ -283,19 +282,19 @@ int res = Collections.binarySearch(chunks, new UnifiedDiffChunkInfo( - DisplaySide.A, 0, 0, cmLine, false), // Dummy DiffChunkInfo + DisplaySide.A, 0, 0, 0, cmLine, false), // Dummy DiffChunkInfo getDiffChunkComparatorCmLine()); if (res >= 0) { // The line is right at the start of a diff chunk. UnifiedDiffChunkInfo info = chunks.get(res); return new LineRegionInfo( - info.getStart(), displaySideToRegionType(info.getSide())); + info.start, displaySideToRegionType(info.side)); } else { // The line might be within or after a diff chunk. res = -res - 1; if (res > 0) { UnifiedDiffChunkInfo info = chunks.get(res - 1); - int lineOnInfoSide = info.getStart() + cmLine - info.getCmLine(); - if (lineOnInfoSide > info.getEnd()) { // After a diff chunk - if (info.getSide() == DisplaySide.A) { + int lineOnInfoSide = info.start + cmLine - info.cmLine; + if (lineOnInfoSide > info.end) { // After a diff chunk + if (info.side == DisplaySide.A) { // For the common region after a deletion chunk, associate the line // on side B with a common region. return new LineRegionInfo( @@ -306,7 +305,7 @@ } } else { // Within a diff chunk return new LineRegionInfo( - lineOnInfoSide, displaySideToRegionType(info.getSide())); + lineOnInfoSide, displaySideToRegionType(info.side)); } } else { // The line is before any diff chunk, so it always equals cmLine and
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/UnifiedDiffChunkInfo.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/UnifiedDiffChunkInfo.java index 844be78..542a153 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/UnifiedDiffChunkInfo.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/UnifiedDiffChunkInfo.java
@@ -16,15 +16,11 @@ public class UnifiedDiffChunkInfo extends DiffChunkInfo { - private int cmLine; + final int cmLine; - UnifiedDiffChunkInfo(DisplaySide side, - int start, int end, int cmLine, boolean edit) { - super(side, start, end, edit); + UnifiedDiffChunkInfo(DisplaySide side, int start, int startOnOther, int end, + int cmLine, boolean edit) { + super(side, start, startOnOther, end, edit); this.cmLine = cmLine; } - - int getCmLine() { - return cmLine; - } }