Use raw HTML to create UnifiedDiffTable, like SideBySideTable

This makes the unified display quicker, but also makes it easier to
abstract out some code that is common to the two classes and pull
that up into a common base class.

Signed-off-by: Shawn O. Pearce <sop@google.com>

3 files changed, 112 insertions, 158 deletions

diff --git a/webapp/src/com/google/gerrit/client/patches/AbstractPatchContentTable.java b/webapp/src/com/google/gerrit/client/patches/AbstractPatchContentTable.java
new file mode 100644
index 0000000000..d7affc33d7
--- /dev/null
+++ b/webapp/src/com/google/gerrit/client/patches/AbstractPatchContentTable.java
@@ -0,0 +1,88 @@
+// Copyright 2008 Google Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.client.patches;
+
+import com.google.gerrit.client.FormatUtil;
+import com.google.gerrit.client.changes.Util;
+import com.google.gerrit.client.data.AccountInfoCache;
+import com.google.gerrit.client.reviewdb.PatchLineComment;
+import com.google.gerrit.client.ui.ComplexDisclosurePanel;
+import com.google.gerrit.client.ui.FancyFlexTable;
+import com.google.gwt.user.client.ui.InlineLabel;
+import com.google.gwt.user.client.ui.FlexTable.FlexCellFormatter;
+
+import java.sql.Timestamp;
+
+public abstract class AbstractPatchContentTable extends FancyFlexTable<Object> {
+  private static final long AGE = 7 * 24 * 60 * 60 * 1000L;
+  protected AccountInfoCache accountCache = AccountInfoCache.empty();
+  private final Timestamp aged =
+      new Timestamp(System.currentTimeMillis() - AGE);
+
+  @Override
+  protected Object getRowItemKey(final Object item) {
+    return null;
+  }
+
+  @Override
+  protected void onOpenItem(final Object item) {
+    if (item instanceof PatchLineComment) {
+      final ComplexDisclosurePanel p =
+          (ComplexDisclosurePanel) table.getWidget(getCurrentRow(), 1);
+      p.setOpen(!p.isOpen());
+    }
+  }
+
+  public void setAccountInfoCache(final AccountInfoCache aic) {
+    assert aic != null;
+    accountCache = aic;
+  }
+
+  protected void bindComment(final int row, final int col,
+      final PatchLineComment line, final boolean isLast) {
+    final LineCommentPanel mp = new LineCommentPanel(line);
+    String panelHeader;
+    final ComplexDisclosurePanel panel;
+
+    if (line.getAuthor() != null) {
+      panelHeader = FormatUtil.nameEmail(accountCache.get(line.getAuthor()));
+    } else {
+      panelHeader = Util.C.messageNoAuthor();
+    }
+
+    if (isLast) {
+      mp.isRecent = true;
+    } else {
+      // TODO Instead of opening messages by strict age, do it by "unread"?
+      mp.isRecent = line.getWrittenOn().after(aged);
+    }
+
+    panel = new ComplexDisclosurePanel(panelHeader, mp.isRecent);
+    panel.getHeader().add(
+        new InlineLabel(Util.M.messageWrittenOn(FormatUtil.mediumFormat(line
+            .getWrittenOn()))));
+    if (line.getStatus() == PatchLineComment.Status.DRAFT) {
+      final InlineLabel d = new InlineLabel(PatchUtil.C.draft());
+      d.setStyleName("CommentIsDraftFlag");
+      panel.getHeader().add(d);
+    }
+    panel.setContent(mp);
+    table.setWidget(row, col, panel);
+
+    final FlexCellFormatter fmt = table.getFlexCellFormatter();
+    fmt.setStyleName(row, col, "Comment");
+    setRowItem(row, line);
+  }
+}
diff --git a/webapp/src/com/google/gerrit/client/patches/SideBySideTable.java b/webapp/src/com/google/gerrit/client/patches/SideBySideTable.java
index 5cce68d390..910454bc19 100644
--- a/webapp/src/com/google/gerrit/client/patches/SideBySideTable.java
+++ b/webapp/src/com/google/gerrit/client/patches/SideBySideTable.java
@@ -14,25 +14,17 @@
 
 package com.google.gerrit.client.patches;
 
-import com.google.gerrit.client.FormatUtil;
-import com.google.gerrit.client.changes.Util;
-import com.google.gerrit.client.data.AccountInfoCache;
 import com.google.gerrit.client.data.SideBySideLine;
 import com.google.gerrit.client.data.SideBySidePatchDetail;
 import com.google.gerrit.client.reviewdb.PatchLineComment;
-import com.google.gerrit.client.ui.ComplexDisclosurePanel;
 import com.google.gerrit.client.ui.DomUtil;
-import com.google.gerrit.client.ui.FancyFlexTable;
 import com.google.gwt.user.client.ui.FlowPanel;
 import com.google.gwt.user.client.ui.InlineLabel;
-import com.google.gwt.user.client.ui.FlexTable.FlexCellFormatter;
 
-import java.sql.Timestamp;
 import java.util.Iterator;
 import java.util.List;
 
-public class SideBySideTable extends FancyFlexTable<Object> {
-  private AccountInfoCache accountCache = AccountInfoCache.empty();
+public class SideBySideTable extends AbstractPatchContentTable {
   private int fileCnt;
   private int maxLineNumber;
 
@@ -40,27 +32,8 @@ public class SideBySideTable extends FancyFlexTable<Object> {
     table.setStyleName("gerrit-SideBySideTable");
   }
 
-  @Override
-  protected Object getRowItemKey(final Object item) {
-    return null;
-  }
-
-  @Override
-  protected void onOpenItem(final Object item) {
-    if (item instanceof PatchLineComment) {
-      final ComplexDisclosurePanel p =
-          (ComplexDisclosurePanel) table.getWidget(getCurrentRow(), 1);
-      p.setOpen(!p.isOpen());
-    }
-  }
-
-  public void setAccountInfoCache(final AccountInfoCache aic) {
-    assert aic != null;
-    accountCache = aic;
-  }
-
   public void display(final SideBySidePatchDetail detail) {
-    accountCache = detail.getAccounts();
+    setAccountInfoCache(detail.getAccounts());
     fileCnt = detail.getFileCount();
     maxLineNumber = detail.getLineCount();
 
@@ -257,43 +230,4 @@ public class SideBySideTable extends FancyFlexTable<Object> {
 
     nc.append("</tr>");
   }
-
-  private void bindComment(final int row, final int col,
-      final PatchLineComment line, final boolean isLast) {
-    final long AGE = 7 * 24 * 60 * 60 * 1000L;
-    final Timestamp aged = new Timestamp(System.currentTimeMillis() - AGE);
-
-    final LineCommentPanel mp = new LineCommentPanel(line);
-    String panelHeader;
-    final ComplexDisclosurePanel panel;
-
-    if (line.getAuthor() != null) {
-      panelHeader = FormatUtil.nameEmail(accountCache.get(line.getAuthor()));
-    } else {
-      panelHeader = Util.C.messageNoAuthor();
-    }
-
-    if (isLast) {
-      mp.isRecent = true;
-    } else {
-      // TODO Instead of opening messages by strict age, do it by "unread"?
-      mp.isRecent = line.getWrittenOn().after(aged);
-    }
-
-    panel = new ComplexDisclosurePanel(panelHeader, mp.isRecent);
-    panel.getHeader().add(
-        new InlineLabel(Util.M.messageWrittenOn(FormatUtil.mediumFormat(line
-            .getWrittenOn()))));
-    if (line.getStatus() == PatchLineComment.Status.DRAFT) {
-      final InlineLabel d = new InlineLabel(PatchUtil.C.draft());
-      d.setStyleName("CommentIsDraftFlag");
-      panel.getHeader().add(d);
-    }
-    panel.setContent(mp);
-    table.setWidget(row, col, panel);
-
-    final FlexCellFormatter fmt = table.getFlexCellFormatter();
-    fmt.setStyleName(row, col, "Comment");
-    setRowItem(row, line);
-  }
 }
diff --git a/webapp/src/com/google/gerrit/client/patches/UnifiedDiffTable.java b/webapp/src/com/google/gerrit/client/patches/UnifiedDiffTable.java
index 461084b2d0..310ca07975 100644
--- a/webapp/src/com/google/gerrit/client/patches/UnifiedDiffTable.java
+++ b/webapp/src/com/google/gerrit/client/patches/UnifiedDiffTable.java
@@ -14,123 +14,55 @@
 
 package com.google.gerrit.client.patches;
 
-import com.google.gerrit.client.FormatUtil;
-import com.google.gerrit.client.changes.Util;
-import com.google.gerrit.client.data.AccountInfoCache;
 import com.google.gerrit.client.data.PatchLine;
 import com.google.gerrit.client.reviewdb.PatchLineComment;
-import com.google.gerrit.client.ui.ComplexDisclosurePanel;
-import com.google.gerrit.client.ui.FancyFlexTable;
-import com.google.gwt.user.client.ui.InlineLabel;
-import com.google.gwt.user.client.ui.HTMLTable.CellFormatter;
 
-import java.sql.Timestamp;
 import java.util.Iterator;
 import java.util.List;
 
-public class UnifiedDiffTable extends FancyFlexTable<Object> {
-  private AccountInfoCache accountCache = AccountInfoCache.empty();
-
+public class UnifiedDiffTable extends AbstractPatchContentTable {
   public UnifiedDiffTable() {
     table.setStyleName("gerrit-UnifiedDiffTable");
   }
 
-  @Override
-  protected Object getRowItemKey(final Object item) {
-    return null;
-  }
-
-  @Override
-  protected void onOpenItem(final Object item) {
-    if (item instanceof PatchLineComment) {
-      final ComplexDisclosurePanel p =
-          (ComplexDisclosurePanel) table.getWidget(getCurrentRow(), 1);
-      p.setOpen(!p.isOpen());
-    }
-  }
-
-  public void setAccountInfoCache(final AccountInfoCache aic) {
-    assert aic != null;
-    accountCache = aic;
-  }
-
   public void display(final List<PatchLine> list) {
-    final int sz = list != null ? list.size() : 0;
-    int dataRows = table.getRowCount() - 1;
-    while (sz < dataRows) {
-      table.removeRow(dataRows);
-      dataRows--;
+    final StringBuilder nc = new StringBuilder();
+    for (final PatchLine pLine : list) {
+      appendLine(nc, pLine);
     }
+    resetHtml(nc.toString());
 
     int row = 0;
     for (final PatchLine pLine : list) {
-      if (dataRows <= row) {
-        table.insertRow(++dataRows);
-        applyDataRowStyle(row);
-      }
-      populate(row++, pLine);
+      setRowItem(row, pLine);
+      row++;
 
       final List<PatchLineComment> comments = pLine.getComments();
       if (comments != null) {
         for (final Iterator<PatchLineComment> ci = comments.iterator(); ci
             .hasNext();) {
           final PatchLineComment c = ci.next();
-          if (dataRows <= row) {
-            table.insertRow(++dataRows);
-            applyDataRowStyle(row);
-          }
-          populate(row++, c, !ci.hasNext());
+          table.insertRow(row);
+          bindComment(row, 1, c, !ci.hasNext());
+          row++;
         }
       }
     }
   }
 
-  private void populate(final int row, final PatchLine line) {
-    final CellFormatter fmt = table.getCellFormatter();
-    table.setWidget(row, C_ARROW, null);
-    table.setHTML(row, 1, PatchUtil.lineToHTML(line.getText()));
-    fmt.setStyleName(row, 1, "DiffText-" + line.getType().name());
-    fmt.addStyleName(row, 1, "DiffText");
-    setRowItem(row, line);
-  }
-
-  private void populate(final int row, final PatchLineComment line,
-      final boolean isLast) {
-    final long AGE = 7 * 24 * 60 * 60 * 1000L;
-    final Timestamp aged = new Timestamp(System.currentTimeMillis() - AGE);
+  private void appendLine(final StringBuilder nc, final PatchLine line) {
+    nc.append("<tr>");
+    nc.append("<td class=\"" + S_ICON_CELL + "\">&nbsp;</td>");
 
-    final LineCommentPanel mp = new LineCommentPanel(line);
-    String panelHeader;
-    final ComplexDisclosurePanel panel;
-
-    if (line.getAuthor() != null) {
-      panelHeader = FormatUtil.nameEmail(accountCache.get(line.getAuthor()));
-    } else {
-      panelHeader = Util.C.messageNoAuthor();
-    }
-
-    if (isLast) {
-      mp.isRecent = true;
-    } else {
-      // TODO Instead of opening messages by strict age, do it by "unread"?
-      mp.isRecent = line.getWrittenOn().after(aged);
-    }
-
-    panel = new ComplexDisclosurePanel(panelHeader, mp.isRecent);
-    panel.getHeader().add(
-        new InlineLabel(Util.M.messageWrittenOn(FormatUtil.mediumFormat(line
-            .getWrittenOn()))));
-    if (line.getStatus() == PatchLineComment.Status.DRAFT) {
-      final InlineLabel d = new InlineLabel(PatchUtil.C.draft());
-      d.setStyleName("CommentIsDraftFlag");
-      panel.getHeader().add(d);
-    }
-    panel.setContent(mp);
-    table.setWidget(row, C_ARROW, null);
-    table.setWidget(row, 1, panel);
+    nc.append("<td class=\"DiffText DiffText-");
+    nc.append(line.getType().name());
+    nc.append("\">");
+    if (!"".equals(line.getText()))
+      nc.append(PatchUtil.lineToHTML(line.getText()));
+    else
+      nc.append("&nbsp;");
+    nc.append("</td>");
 
-    final CellFormatter fmt = table.getCellFormatter();
-    fmt.setStyleName(row, 1, "Comment");
-    setRowItem(row, line);
+    nc.append("</tr>");
   }
 }