Merge "Revert "Make DiffChunkInfo implement Comparable to simplify code""

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ChunkManager.java

@@ -25,6 +25,7 @@ import net.codemirror.lib.Pos;
 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}. */
@@ -101,7 +102,7 @@ abstract class ChunkManager {
 
     DiffChunkInfo lookUp = chunks.get(res);
     // If edit, skip the deletion chunk and set focus on the insertion one.
-    if (lookUp.edit && lookUp.side == A) {
+    if (lookUp.isEdit() && lookUp.getSide() == A) {
       res = res + (dir == Direction.PREV ? -1 : 1);
       if (res < 0 || chunks.size() <= res) {
         return;
@@ -109,8 +110,8 @@ abstract class ChunkManager {
     }
 
     DiffChunkInfo target = chunks.get(res);
-    CodeMirror targetCm = host.getCmFromSide(target.side);
+    CodeMirror targetCm = host.getCmFromSide(target.getSide());
-    int cmLine = getCmLine(target.start, target.side);
+    int cmLine = getCmLine(target.getStart(), target.getSide());
     targetCm.setCursor(Pos.create(cmLine));
     targetCm.focus();
     targetCm.scrollToY(
@@ -118,5 +119,28 @@ abstract class ChunkManager {
         - 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 = lineMapper.lineOnOther(a.getSide(), a.getStart())
+              .getLine() - b.getStart();
+          return comp == 0 ? -1 : comp;
+        } else {
+          int comp = a.getStart() -
+              lineMapper.lineOnOther(b.getSide(), b.getStart()).getLine();
+          return comp == 0 ? 1 : comp;
+        }
+      }
+    };
+  }
+
   abstract int getCmLine(int line, DisplaySide side);
 }

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffChunkInfo.java

@@ -15,39 +15,32 @@
 package com.google.gerrit.client.diff;
 
 /** Object recording the position of a diff chunk and whether it's an edit */
-class DiffChunkInfo implements Comparable<DiffChunkInfo> {
+class DiffChunkInfo {
-  final DisplaySide side;
+  private DisplaySide side;
-  final int start;
+  private int start;
-  final int end;
+  private int end;
-  final boolean edit;
+  private boolean edit;
 
-  private final int startOnOther;
+  DiffChunkInfo(DisplaySide side, int start, int end, boolean edit) {
-
-  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() {
-   * Chunks are ordered by their starting line. If it's a deletion, use its
+    return side;
-   * 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.
+  int getStart() {
-   */
+    return start;
-  @Override
+  }
-  public int compareTo(DiffChunkInfo o) {
+
-    if (side == o.side) {
+  int getEnd() {
-      return start - o.start;
+    return end;
-    } else if (side == DisplaySide.A) {
+  }
-      int comp = startOnOther - o.start;
+
-      return comp == 0 ? -1 : comp;
+  boolean isEdit() {
-    } else {
+    return edit;
-      int comp = start - o.startOnOther;
-      return comp == 0 ? 1 : comp;
-    }
   }
 }

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide.java

@@ -118,12 +118,12 @@ public class SideBySide extends DiffScreen {
     if (getStartLine() == 0) {
       DiffChunkInfo d = chunkManager.getFirst();
       if (d != null) {
-        if (d.edit && d.side == DisplaySide.A) {
+        if (d.isEdit() && d.getSide() == DisplaySide.A) {
           setStartSide(DisplaySide.B);
-          setStartLine(lineOnOther(d.side, d.start).getLine() + 1);
+          setStartLine(lineOnOther(d.getSide(), d.getStart()).getLine() + 1);
         } else {
-          setStartSide(d.side);
+          setStartSide(d.getSide());
-          setStartLine(d.start + 1);
+          setStartLine(d.getStart() + 1);
         }
       }
     }

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySideChunkManager.java

@@ -170,10 +170,10 @@ class SideBySideChunkManager extends ChunkManager {
     int endA = lineMapper.getLineA() - 1;
     int endB = lineMapper.getLineB() - 1;
     if (aLen > 0) {
-      addDiffChunk(cmB, endB, endA, aLen, bLen > 0);
+      addDiffChunk(cmB, endA, aLen, bLen > 0);
     }
     if (bLen > 0) {
-      addDiffChunk(cmA, endA, endB, bLen, aLen > 0);
+      addDiffChunk(cmA, endB, bLen, aLen > 0);
     }
   }
 
@@ -245,10 +245,10 @@ class SideBySideChunkManager extends ChunkManager {
     }
   }
 
-  private void addDiffChunk(CodeMirror cmToPad, int line, int lineOnOther,
+  private void addDiffChunk(CodeMirror cmToPad, int lineOnOther,
       int chunkSize, boolean edit) {
     chunks.add(new DiffChunkInfo(host.otherCm(cmToPad).side(),
-        lineOnOther - chunkSize + 1, line - chunkSize + 1, lineOnOther, edit));
+        lineOnOther - chunkSize + 1, lineOnOther, edit));
   }
 
   @Override
@@ -261,7 +261,8 @@ class SideBySideChunkManager extends ChunkManager {
             : 0;
         int res = Collections.binarySearch(
                 chunks,
-                new DiffChunkInfo(cm.side(), line, 0, 0, false));
+                new DiffChunkInfo(cm.side(), line, 0, false),
+                getDiffChunkComparator());
         diffChunkNavHelper(chunks, host, res, dir);
       }
     };

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Unified.java

@@ -116,12 +116,12 @@ public class Unified extends DiffScreen {
     if (getStartLine() == 0) {
       DiffChunkInfo d = chunkManager.getFirst();
       if (d != null) {
-        if (d.edit && d.side == DisplaySide.A) {
+        if (d.isEdit() && d.getSide() == DisplaySide.A) {
           setStartSide(DisplaySide.B);
         } else {
-          setStartSide(d.side);
+          setStartSide(d.getSide());
         }
-        setStartLine(chunkManager.getCmLine(d.start, d.side) + 1);
+        setStartLine(chunkManager.getCmLine(d.getStart(), d.getSide()) + 1);
       }
     }
     if (getStartSide() != null && getStartLine() > 0) {

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/UnifiedChunkManager.java

@@ -140,14 +140,14 @@ class UnifiedChunkManager extends ChunkManager {
     int endA = lineMapper.getLineA() - 1;
     int endB = lineMapper.getLineB() - 1;
     if (aLen > 0) {
-      addDiffChunk(DisplaySide.A, endA, endB, aLen, cmLine, bLen > 0);
+      addDiffChunk(DisplaySide.A, endA, aLen, cmLine, bLen > 0);
       for (int j = 0; j < aLen; j++) {
         host.setLineNumber(DisplaySide.A, cmLine + j, startA + j + 1);
         host.setLineNumberEmpty(DisplaySide.B, cmLine + j);
       }
     }
     if (bLen > 0) {
-      addDiffChunk(DisplaySide.B, endB, endA, bLen, cmLine + aLen, aLen > 0);
+      addDiffChunk(DisplaySide.B, endB, bLen, cmLine + aLen, aLen > 0);
       for (int j = 0; j < bLen; j++) {
         host.setLineNumberEmpty(DisplaySide.A, cmLine + aLen + j);
         host.setLineNumber(DisplaySide.B, cmLine + aLen + j, startB + j + 1);
@@ -208,10 +208,10 @@ class UnifiedChunkManager extends ChunkManager {
         : UnifiedTable.style.diffInsert();
   }
 
-  private void addDiffChunk(DisplaySide side, int chunkEnd, int otherChunkEnd,
+  private void addDiffChunk(DisplaySide side, int chunkEnd, int chunkSize,
-      int chunkSize, int cmLine, boolean edit) {
+      int cmLine, boolean edit) {
-    chunks.add(new UnifiedDiffChunkInfo(side, chunkEnd - chunkSize + 1,
+    chunks.add(new UnifiedDiffChunkInfo(side, chunkEnd - chunkSize + 1, chunkEnd,
-        otherChunkEnd - chunkSize + 1, chunkEnd, cmLine, edit));
+        cmLine, edit));
   }
 
   @Override
@@ -224,7 +224,7 @@ class UnifiedChunkManager extends ChunkManager {
             : 0;
         int res = Collections.binarySearch(
                 chunks,
-                new UnifiedDiffChunkInfo(cm.side(), 0, 0, 0, line, false),
+                new UnifiedDiffChunkInfo(cm.side(), 0, 0, line, false),
                 getDiffChunkComparatorCmLine());
         diffChunkNavHelper(chunks, host, res, dir);
       }
@@ -236,7 +236,7 @@ class UnifiedChunkManager extends ChunkManager {
     return new Comparator<UnifiedDiffChunkInfo>() {
       @Override
       public int compare(UnifiedDiffChunkInfo o1, UnifiedDiffChunkInfo o2) {
-        return o1.cmLine - o2.cmLine;
+        return o1.getCmLine() - o2.getCmLine();
       }
     };
   }
@@ -246,30 +246,31 @@ class UnifiedChunkManager extends ChunkManager {
     int res =
         Collections.binarySearch(chunks,
             new UnifiedDiffChunkInfo(
-                side, line, 0, 0, 0, false)); // Dummy DiffChunkInfo
+                side, line, 0, 0, false), // Dummy DiffChunkInfo
+            getDiffChunkComparator());
     if (res >= 0) {
-      return chunks.get(res).cmLine;
+      return chunks.get(res).getCmLine();
     } 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.edit
+        if (side == DisplaySide.A && info.isEdit()
-            && info.side == DisplaySide.B) {
+            && info.getSide() == DisplaySide.B) {
           // Need to use the start and cmLine of the deletion chunk
           UnifiedDiffChunkInfo delete = chunks.get(res - 2);
-          if (line <= delete.end) {
+          if (line <= delete.getEnd()) {
-            return delete.cmLine + line - delete.start;
+            return delete.getCmLine() + line - delete.getStart();
           } else {
             // Need to add the length of the insertion chunk
-            return delete.cmLine + line - delete.start
+            return delete.getCmLine() + line - delete.getStart()
-                + info.end - info.start + 1;
+                + info.getEnd() - info.getStart() + 1;
           }
-        } else if (side == info.side) {
+        } else if (side == info.getSide()) {
-          return info.cmLine + line - info.start;
+          return info.getCmLine() + line - info.getStart();
         } else {
-          return info.cmLine
+          return info.getCmLine()
               + lineMapper.lineOnOther(side, line).getLine()
-              - info.start;
+              - info.getStart();
         }
       } else {
         return line;
@@ -281,19 +282,19 @@ class UnifiedChunkManager extends ChunkManager {
     int res =
         Collections.binarySearch(chunks,
             new UnifiedDiffChunkInfo(
-                DisplaySide.A, 0, 0, 0, cmLine, false), // Dummy DiffChunkInfo
+                DisplaySide.A, 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.start, displaySideToRegionType(info.side));
+          info.getStart(), displaySideToRegionType(info.getSide()));
     } 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.start + cmLine - info.cmLine;
+        int lineOnInfoSide = info.getStart() + cmLine - info.getCmLine();
-        if (lineOnInfoSide > info.end) { // After a diff chunk
+        if (lineOnInfoSide > info.getEnd()) { // After a diff chunk
-          if (info.side == DisplaySide.A) {
+          if (info.getSide() == DisplaySide.A) {
             // For the common region after a deletion chunk, associate the line
             // on side B with a common region.
             return new LineRegionInfo(
@@ -304,7 +305,7 @@ class UnifiedChunkManager extends ChunkManager {
           }
         } else { // Within a diff chunk
           return new LineRegionInfo(
-              lineOnInfoSide, displaySideToRegionType(info.side));
+              lineOnInfoSide, displaySideToRegionType(info.getSide()));
         }
       } else {
         // The line is before any diff chunk, so it always equals cmLine and

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/UnifiedDiffChunkInfo.java

@@ -16,11 +16,15 @@ package com.google.gerrit.client.diff;
 
 public class UnifiedDiffChunkInfo extends DiffChunkInfo {
 
-  final int cmLine;
+  private int cmLine;
 
-  UnifiedDiffChunkInfo(DisplaySide side, int start, int startOnOther, int end,
+  UnifiedDiffChunkInfo(DisplaySide side,
-      int cmLine, boolean edit) {
+      int start, int end, int cmLine, boolean edit) {
-    super(side, start, startOnOther, end, edit);
+    super(side, start, end, edit);
     this.cmLine = cmLine;
   }
+
+  int getCmLine() {
+    return cmLine;
+  }
 }