Merge "Add workaround to renderSkipLine() for the first line"

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

@@ -55,7 +55,6 @@ import net.codemirror.lib.KeyMap;
 import net.codemirror.lib.LineCharacter;
 import net.codemirror.lib.LineWidget;
 import net.codemirror.lib.ModeInjector;
-import net.codemirror.lib.TextMarker;
 
 import java.util.ArrayList;
 import java.util.Collections;
@@ -291,8 +290,8 @@ public class CodeMirrorDemo extends Screen {
         int aLength = currentA.length();
         int bLength = currentB.length();
         String color = currentA == EMPTY || currentB == EMPTY
-            ? diffTable.style.diff()
+            ? DiffTable.style.diff()
-            : diffTable.style.intralineBg();
+            : DiffTable.style.intralineBg();
         colorLines(cmA, color, origLineA, aLength);
         colorLines(cmB, color, origLineB, bLength);
         mapper.appendCommon(Math.min(aLength, bLength));
@@ -359,7 +358,7 @@ public class CodeMirrorDemo extends Screen {
     } else {
       // Estimated height at 21px, fixed by deferring after display
       manager = new PaddingManager(
-          addPaddingWidget(cm, diffTable.style.padding(), line, 21, Unit.PX, 0));
+          addPaddingWidget(cm, DiffTable.style.padding(), line, 21, Unit.PX, 0));
       linePaddingManagerMap.put(handle, manager);
     }
     int lineToPad = mapper.lineOnOther(mySide, line).getLine();
@@ -368,7 +367,7 @@ public class CodeMirrorDemo extends Screen {
       PaddingManager.link(manager, linePaddingManagerMap.get(otherHandle));
     } else {
       PaddingManager otherManager = new PaddingManager(
-          addPaddingWidget(other, diffTable.style.padding(), lineToPad, 21, Unit.PX, 0));
+          addPaddingWidget(other, DiffTable.style.padding(), lineToPad, 21, Unit.PX, 0));
       linePaddingManagerMap.put(otherHandle, otherManager);
       PaddingManager.link(manager, otherManager);
     }
@@ -486,25 +485,26 @@ public class CodeMirrorDemo extends Screen {
     hiddenSkipMap.put(cm.getLineHandle(markEnd), size);
     SkipBar bar = new SkipBar(cm, hiddenSkipMap);
     diffTable.add(bar);
-    TextMarker marker = cm.markText(
-        CodeMirror.pos(markStart),
-        CodeMirror.pos(markEnd),
-        Configuration.create().set("collapsed", true));
     /**
-     * TODO: Due to CodeMirror limitation, there's no way to make the first
+     * Due to CodeMirror limitation, there's no way to make the first
-     * line disappear completely. The current approach leaves an empty line
+     * line disappear completely, and CodeMirror doesn't like manually
-     * with line number "1" still showing, and CodeMirror doesn't like manually
+     * setting the display of a line to "none". The workaround here uses
-     * setting the display of a line to "none". A workaround may be to use
      * inline widget for the first line and regular line widgets for others.
      */
-    boolean isZero = markStart == -1;
+    Configuration markerConfig;
-    Configuration config = Configuration.create()
+    if (markStart == -1) {
-      .set("coverGutter", true)
+      markerConfig = Configuration.create()
-      .set("above", isZero);
+        .set("inclusiveLeft", true)
-    LineWidget widget = cm.addLineWidget(
+        .set("inclusiveRight", true)
-        isZero ? markEnd + 1 : markStart, bar.getElement(), config);
+        .set("replacedWith", bar.getElement());
-    bar.setWidget(widget);
+      cm.addLineClass(0, LineClassWhere.WRAP, DiffTable.style.hideNumber());
-    bar.setMarker(marker, size);
+    } else {
+      markerConfig = Configuration.create().set("collapsed", true);
+      Configuration config = Configuration.create().set("coverGutter", true);
+      bar.setWidget(cm.addLineWidget(markStart, bar.getElement(), config));
+    }
+    bar.setMarker(cm.markText(CodeMirror.pos(markStart),
+        CodeMirror.pos(markEnd), markerConfig), size);
     return bar;
   }
 
@@ -523,10 +523,10 @@ public class CodeMirrorDemo extends Screen {
     }
     EditIterator iter = new EditIterator(lines, startLine);
     Configuration intralineBgOpt = Configuration.create()
-        .set("className", diffTable.style.intralineBg())
+        .set("className", DiffTable.style.intralineBg())
         .set("readOnly", true);
     Configuration diffOpt = Configuration.create()
-        .set("className", diffTable.style.diff())
+        .set("className", DiffTable.style.diff())
         .set("readOnly", true);
     LineCharacter last = CodeMirror.pos(0, 0);
     for (int i = 0; i < edits.length(); i++) {
@@ -543,7 +543,7 @@ public class CodeMirrorDemo extends Screen {
       last = to;
       for (int line = fromLine; line < to.getLine(); line++) {
         cm.addLineClass(line, LineClassWhere.BACKGROUND,
-            diffTable.style.diff());
+            DiffTable.style.diff());
       }
     }
   }
@@ -556,7 +556,7 @@ public class CodeMirrorDemo extends Screen {
 
   private void insertEmptyLines(CodeMirror cm, int nextLine, int cnt) {
     // -1 to compensate for the line we went past when this method is called.
-    addPaddingWidget(cm, diffTable.style.padding(), nextLine - 1,
+    addPaddingWidget(cm, DiffTable.style.padding(), nextLine - 1,
         cnt, Unit.EM, null);
   }
 
@@ -590,19 +590,23 @@ public class CodeMirrorDemo extends Screen {
       public void run() {
         if (cm.hasActiveLine()) {
           cm.removeLineClass(cm.getActiveLine(),
-              LineClassWhere.WRAP, diffTable.style.activeLine());
+              LineClassWhere.WRAP, DiffTable.style.activeLine());
           cm.removeLineClass(cm.getActiveLine(),
-              LineClassWhere.BACKGROUND, diffTable.style.activeLineBg());
+              LineClassWhere.BACKGROUND, DiffTable.style.activeLineBg());
         }
         if (other.hasActiveLine()) {
           other.removeLineClass(other.getActiveLine(),
-              LineClassWhere.WRAP, diffTable.style.activeLine());
+              LineClassWhere.WRAP, DiffTable.style.activeLine());
           other.removeLineClass(other.getActiveLine(),
-              LineClassWhere.BACKGROUND, diffTable.style.activeLineBg());
+              LineClassWhere.BACKGROUND, DiffTable.style.activeLineBg());
         }
         int line = cm.getCursor("head").getLine();
         LineHandle handle = cm.getLineHandle(line);
-        // Ugly workaround because CodeMirror never hides lines completely.
+        /**
+         * Ugly workaround because CodeMirror never hides lines completely.
+         * TODO: Change to use CodeMirror's official workaround after
+         * updating the library to latest HEAD.
+         */
         if (hiddenSkipMap.containsKey(handle)) {
           line -= hiddenSkipMap.get(handle);
           handle = cm.getLineHandle(line);
@@ -611,17 +615,17 @@ public class CodeMirrorDemo extends Screen {
         if (cm.somethingSelected()) {
           return;
         }
-        cm.addLineClass(line, LineClassWhere.WRAP, diffTable.style.activeLine());
+        cm.addLineClass(line, LineClassWhere.WRAP, DiffTable.style.activeLine());
-        cm.addLineClass(line, LineClassWhere.BACKGROUND, diffTable.style.activeLineBg());
+        cm.addLineClass(line, LineClassWhere.BACKGROUND, DiffTable.style.activeLineBg());
         LineOnOtherInfo info =
             mapper.lineOnOther(cm == cmA ? Side.PARENT : Side.REVISION, line);
         int oLine = info.getLine();
         if (info.isAligned()) {
           other.setActiveLine(other.getLineHandle(oLine));
           other.addLineClass(oLine, LineClassWhere.WRAP,
-              diffTable.style.activeLine());
+              DiffTable.style.activeLine());
           other.addLineClass(oLine, LineClassWhere.BACKGROUND,
-              diffTable.style.activeLineBg());
+              DiffTable.style.activeLineBg());
         }
       }
     };

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

@@ -37,6 +37,7 @@ class DiffTable extends Composite {
     String padding();
     String activeLine();
     String activeLineBg();
+    String hideNumber();
   }
 
   @UiField
@@ -46,7 +47,7 @@ class DiffTable extends Composite {
   Element cmB;
 
   @UiField
-  LineStyle style;
+  static LineStyle style;
 
   DiffTable() {
     initWidget(uiBinder.createAndBindUi(this));

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.ui.xml

@@ -62,14 +62,20 @@ limitations under the License.
     }
     .activeLine .CodeMirror-linenumber,
     .activeLine .diff, .activeLine .intralineBg {
-      background-color: #D8EDF9 !important;
+      background-color: #FFFF99 !important;
     }
     .activeLineBg {
-      background-color: #D8EDF9 !important;
+      background-color: #FFFF99 !important;
     }
     .cm-trailingspace {
       background-color: red !important;
     }
+    .hideNumber .CodeMirror-linenumber {
+      color: transparent;
+      background-color: #def;
+      width: 23px !important;
+      height: 16px;
+    }
   </ui:style>
   <g:HTMLPanel styleName='{style.difftable}'>
     <table class='{style.table}'>

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

@@ -26,6 +26,7 @@ import com.google.gwt.user.client.ui.Composite;
 import com.google.gwt.user.client.ui.HTMLPanel;
 
 import net.codemirror.lib.CodeMirror;
+import net.codemirror.lib.CodeMirror.LineClassWhere;
 import net.codemirror.lib.CodeMirror.LineHandle;
 import net.codemirror.lib.Configuration;
 import net.codemirror.lib.LineWidget;
@@ -46,6 +47,7 @@ class SkipBar extends Composite {
   private LineWidget widget;
 
   interface SkipBarStyle extends CssResource {
+    String isLineWidget();
     String noExpand();
   }
 
@@ -78,6 +80,7 @@ class SkipBar extends Composite {
 
   void setWidget(LineWidget widget) {
     this.widget = widget;
+    addStyleName(style.isLineWidget());
   }
 
   void setMarker(TextMarker marker, int length) {
@@ -110,24 +113,31 @@ class SkipBar extends Composite {
     return true;
   }
 
+  private void clearMarkerAndWidget() {
+    marker.clear();
+    if (widget != null) {
+      widget.clear();
+    } else {
+      cm.removeLineClass(0, LineClassWhere.WRAP, DiffTable.style.hideNumber());
+    }
+  }
+
   private void expandAll() {
     hiddenSkipMap.remove(
         cm.getLineHandle(marker.find().getTo().getLine()));
-    marker.clear();
+    clearMarkerAndWidget();
-    widget.clear();
     removeFromParent();
   }
 
   private void expandBefore() {
     FromTo fromTo = marker.find();
-    marker.clear();
     int oldStart = fromTo.getFrom().getLine();
     int newStart = oldStart + NUM_ROWS_TO_EXPAND;
     int end = fromTo.getTo().getLine();
+    clearMarkerAndWidget();
     marker = cm.markText(CodeMirror.pos(newStart), CodeMirror.pos(end), COLLAPSED);
     Configuration config = Configuration.create().set("coverGutter", true);
     LineWidget newWidget = cm.addLineWidget(newStart, getElement(), config);
-    widget.clear();
     setWidget(newWidget);
     updateSkipNum();
     hiddenSkipMap.put(cm.getLineHandle(end), numSkipLines);
@@ -135,9 +145,9 @@ class SkipBar extends Composite {
 
   private void expandAfter() {
     FromTo fromTo = marker.find();
-    marker.clear();
     int oldEnd = fromTo.getTo().getLine();
     int newEnd = oldEnd - NUM_ROWS_TO_EXPAND;
+    marker.clear();
     marker = cm.markText(CodeMirror.pos(fromTo.getFrom().getLine()),
         CodeMirror.pos(newEnd),
         COLLAPSED);

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SkipBar.ui.xml

@@ -27,6 +27,9 @@ limitations under the License.
       font-style: italic;
       overflow: hidden;
     }
+    .isLineWidget .text {
+      padding-left: 31px;
+    }
     .anchor {
       color: inherit;
       text-decoration: none;