ChangeScreen2: Show inline comments in history

When a history comment is expanded include the inline comments written
on that revision by the author at the same timestamp.  This makes it
easier to follow a change's evolution over time, especially if the
reader is new to the change and does not have the entire email thread
in their inbox.

Bug: issue 93
Change-Id: I57fe29fea2b3651306ff0a58746cefcfe1669267

gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.java

@@ -71,7 +71,6 @@ import com.google.gwt.user.client.EventListener;
 import com.google.gwt.user.client.rpc.AsyncCallback;
 import com.google.gwt.user.client.ui.Anchor;
 import com.google.gwt.user.client.ui.Button;
-import com.google.gwt.user.client.ui.FlowPanel;
 import com.google.gwt.user.client.ui.HTMLPanel;
 import com.google.gwt.user.client.ui.Image;
 import com.google.gwt.user.client.ui.ToggleButton;
@@ -150,7 +149,7 @@ public class ChangeScreen2 extends Screen {
   @UiField CommitBox commit;
   @UiField RelatedChanges related;
   @UiField FileTable files;
-  @UiField FlowPanel history;
+  @UiField History history;
 
   @UiField Button includedIn;
   @UiField Button revisions;
@@ -520,6 +519,7 @@ public class ChangeScreen2 extends Screen {
 
   private List<NativeMap<JsArray<CommentInfo>>> loadComments(
       RevisionInfo rev, CallbackGroup group) {
+    final int id = rev._number();
     final List<NativeMap<JsArray<CommentInfo>>> r =
         new ArrayList<NativeMap<JsArray<CommentInfo>>>(1);
     ChangeApi.revision(changeId.get(), rev.name())
@@ -528,6 +528,7 @@ public class ChangeScreen2 extends Screen {
         @Override
         public void onSuccess(NativeMap<JsArray<CommentInfo>> result) {
           r.add(result);
+          history.addComments(id, result);
         }
 
         @Override
@@ -657,7 +658,6 @@ public class ChangeScreen2 extends Screen {
     renderCommitSubject(info);
     renderOwner(info);
     renderActionTextDate(info);
-    renderHistory(info);
     initIncludedInAction(info);
     initRevisionsAction(info, revision);
     initDownloadAction(info, revision);
@@ -673,6 +673,7 @@ public class ChangeScreen2 extends Screen {
     related.set(info, revision);
     reviewers.set(info);
     quickApprove.set(info, revision);
+    history.set(commentLinkProcessor, changeId, info);
 
     if (Gerrit.isSignedIn()) {
       initEditMessageAction(info, revision);
@@ -736,15 +737,6 @@ public class ChangeScreen2 extends Screen {
     actionDate.setInnerText(FormatUtil.relativeFormat(info.updated()));
   }
 
-  private void renderHistory(ChangeInfo info) {
-    JsArray<MessageInfo> messages = info.messages();
-    if (messages != null) {
-      for (int i = 0; i < messages.length(); i++) {
-        history.add(new Message(commentLinkProcessor, messages.get(i)));
-      }
-    }
-  }
-
   void showUpdates(ChangeInfo newInfo) {
     if (!isAttached() || newInfo.updated().equals(lastDisplayedUpdate)) {
       return;

gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.ui.xml

@@ -432,6 +432,6 @@ limitations under the License.
         </g:Button>
       </div>
     </div>
-    <g:FlowPanel ui:field='history'/>
+    <c:History ui:field='history'/>
   </g:HTMLPanel>
 </ui:UiBinder>

gerrit-gwtui/src/main/java/com/google/gerrit/client/change/FileComments.java

@@ -0,0 +1,83 @@
+// Copyright (C) 2013 The Android Open Source Project
+//
+// 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.change;
+
+import com.google.gerrit.client.Gerrit;
+import com.google.gerrit.client.changes.CommentInfo;
+import com.google.gerrit.client.ui.CommentLinkProcessor;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gwt.core.client.GWT;
+import com.google.gwt.event.dom.client.ClickEvent;
+import com.google.gwt.uibinder.client.UiBinder;
+import com.google.gwt.uibinder.client.UiField;
+import com.google.gwt.uibinder.client.UiHandler;
+import com.google.gwt.user.client.ui.Anchor;
+import com.google.gwt.user.client.ui.Composite;
+import com.google.gwt.user.client.ui.FlowPanel;
+import com.google.gwt.user.client.ui.HTMLPanel;
+import com.google.gwtorm.client.KeyUtil;
+
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
+
+class FileComments extends Composite {
+  interface Binder extends UiBinder<HTMLPanel, FileComments> {}
+  private static final Binder uiBinder = GWT.create(Binder.class);
+
+  private final String url;
+  @UiField Anchor path;
+  @UiField FlowPanel comments;
+
+  FileComments(CommentLinkProcessor clp,
+      PatchSet.Id ps,
+      String title,
+      List<CommentInfo> list) {
+    initWidget(uiBinder.createAndBindUi(this));
+
+    url = url(ps, list.get(0));
+    path.setHref("#" + url);
+    path.setText(title);
+
+    Collections.sort(list, new Comparator<CommentInfo>() {
+      @Override
+      public int compare(CommentInfo a, CommentInfo b) {
+        return a.line() - b.line();
+      }
+    });
+    for (CommentInfo c : list) {
+      comments.add(new LineComment(clp, c));
+    }
+  }
+
+  @UiHandler("path")
+  void onClick(ClickEvent e) {
+    e.preventDefault();
+    e.stopPropagation();
+    Gerrit.display(url);
+  }
+
+  private static String url(PatchSet.Id ps, CommentInfo info) {
+    // TODO(sop): Switch to Dispatcher.toPatchSideBySide.
+    Change.Id c = ps.getParentKey();
+    return new StringBuilder()
+      .append("/c/").append(c.get()).append('/')
+      .append(ps.get()).append('/')
+      .append(KeyUtil.encode(info.path()))
+      .append(",cm")
+      .toString();
+  }
+}

gerrit-gwtui/src/main/java/com/google/gerrit/client/change/FileComments.ui.xml

@@ -0,0 +1,37 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+Copyright (C) 2013 The Android Open Source Project
+
+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.
+-->
+<ui:UiBinder
+    xmlns:ui='urn:ui:com.google.gwt.uibinder'
+    xmlns:c='urn:import:com.google.gerrit.client'
+    xmlns:g='urn:import:com.google.gwt.user.client.ui'>
+  <ui:style>
+    .box {
+    }
+    .path {
+      display: block;
+      white-space: nowrap;
+    }
+    .comments {
+      margin-left: 1em;
+    }
+  </ui:style>
+
+  <g:HTMLPanel styleName='{style.box}'>
+    <g:Anchor styleName='{style.path}' ui:field='path'/>
+    <g:FlowPanel styleName='{style.comments}' ui:field='comments'/>
+  </g:HTMLPanel>
+</ui:UiBinder>

gerrit-gwtui/src/main/java/com/google/gerrit/client/change/History.java

@@ -0,0 +1,172 @@
+// Copyright (C) 2013 The Android Open Source Project
+//
+// 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.change;
+
+import com.google.gerrit.client.account.AccountInfo;
+import com.google.gerrit.client.changes.ChangeApi;
+import com.google.gerrit.client.changes.ChangeInfo;
+import com.google.gerrit.client.changes.ChangeInfo.MessageInfo;
+import com.google.gerrit.client.changes.CommentInfo;
+import com.google.gerrit.client.rpc.NativeMap;
+import com.google.gerrit.client.rpc.Natives;
+import com.google.gerrit.client.ui.CommentLinkProcessor;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gwt.core.client.JsArray;
+import com.google.gwt.user.client.rpc.AsyncCallback;
+import com.google.gwt.user.client.ui.FlowPanel;
+import com.google.gwt.user.client.ui.Widget;
+
+import java.sql.Timestamp;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+class History extends FlowPanel {
+  private CommentLinkProcessor clp;
+  private Change.Id changeId;
+
+  private final Set<Integer> loaded = new HashSet<Integer>();
+  private final Map<AuthorRevision, List<CommentInfo>> byAuthor =
+      new HashMap<AuthorRevision, List<CommentInfo>>();
+
+  void set(CommentLinkProcessor clp, Change.Id id, ChangeInfo info) {
+    this.clp = clp;
+    this.changeId = id;
+
+    JsArray<MessageInfo> messages = info.messages();
+    if (messages != null) {
+      for (MessageInfo msg : Natives.asList(messages)) {
+        Message ui = new Message(this, msg);
+        if (loaded.contains(msg._revisionNumber())) {
+          ui.addComments(comments(msg));
+        }
+        add(ui);
+      }
+    }
+  }
+
+  CommentLinkProcessor getCommentLinkProcessor() {
+    return clp;
+  }
+
+  Change.Id getChangeId() {
+    return changeId;
+  }
+
+  void addComments(int id, NativeMap<JsArray<CommentInfo>> map) {
+    loaded.add(id);
+
+    for (String path : map.keySet()) {
+      for (CommentInfo c : Natives.asList(map.get(path))) {
+        c.setPath(path);
+        if (c.author() != null) {
+          AuthorRevision k = new AuthorRevision(c.author(), id);
+          List<CommentInfo> l = byAuthor.get(k);
+          if (l == null) {
+            l = new ArrayList<CommentInfo>();
+            byAuthor.put(k, l);
+          }
+          l.add(c);
+        }
+      }
+    }
+  }
+
+  void load(final int revisionNumber) {
+    if (revisionNumber > 0 && loaded.add(revisionNumber)) {
+      ChangeApi.revision(new PatchSet.Id(changeId, revisionNumber))
+        .view("comments")
+        .get(new AsyncCallback<NativeMap<JsArray<CommentInfo>>>() {
+          @Override
+          public void onSuccess(NativeMap<JsArray<CommentInfo>> result) {
+            addComments(revisionNumber, result);
+            update(revisionNumber);
+          }
+
+          @Override
+          public void onFailure(Throwable caught) {
+          }
+        });
+    }
+  }
+
+  private void update(int revisionNumber) {
+    for (Widget child : getChildren()) {
+      Message ui = (Message) child;
+      MessageInfo info = ui.getMessageInfo();
+      if (info._revisionNumber() == revisionNumber) {
+        ui.addComments(comments(info));
+      }
+    }
+  }
+
+  private List<CommentInfo> comments(MessageInfo msg) {
+    if (msg.author() == null) {
+      return Collections.emptyList();
+    }
+
+    AuthorRevision k = new AuthorRevision(msg.author(), msg._revisionNumber());
+    List<CommentInfo> list = byAuthor.get(k);
+    if (list == null) {
+      return Collections.emptyList();
+    }
+
+    Timestamp when = msg.date();
+    List<CommentInfo> match = new ArrayList<CommentInfo>();
+    List<CommentInfo> other = new ArrayList<CommentInfo>();
+    for (int i = 0; i < list.size(); i++) {
+      CommentInfo c = list.get(i);
+      if (c.updated().compareTo(when) <= 0) {
+        match.add(c);
+      } else {
+        other.add(c);
+      }
+    }
+    if (match.isEmpty()) {
+      return Collections.emptyList();
+    } else if (other.isEmpty()) {
+      byAuthor.remove(k);
+    } else {
+      byAuthor.put(k, other);
+    }
+    return match;
+  }
+
+  private static final class AuthorRevision {
+    final int author;
+    final int revision;
+
+    AuthorRevision(AccountInfo author, int revision) {
+      this.author = author._account_id();
+      this.revision = revision;
+    }
+
+    @Override
+    public int hashCode() {
+      return author * 31 + revision;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+      AuthorRevision b = (AuthorRevision) o;
+      return author == b.author && revision == b.revision;
+    }
+  }
+}

gerrit-gwtui/src/main/java/com/google/gerrit/client/change/LineComment.java

@@ -0,0 +1,46 @@
+// Copyright (C) 2013 The Android Open Source Project
+//
+// 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.change;
+
+import com.google.gerrit.client.changes.CommentInfo;
+import com.google.gerrit.client.changes.Util;
+import com.google.gerrit.client.ui.CommentLinkProcessor;
+import com.google.gwt.core.client.GWT;
+import com.google.gwt.dom.client.Element;
+import com.google.gwt.uibinder.client.UiBinder;
+import com.google.gwt.uibinder.client.UiField;
+import com.google.gwt.user.client.ui.Composite;
+import com.google.gwt.user.client.ui.HTMLPanel;
+import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder;
+
+class LineComment extends Composite {
+  interface Binder extends UiBinder<HTMLPanel, LineComment> {}
+  private static final Binder uiBinder = GWT.create(Binder.class);
+
+  @UiField Element location;
+  @UiField Element message;
+
+  LineComment(CommentLinkProcessor clp, CommentInfo info) {
+    initWidget(uiBinder.createAndBindUi(this));
+
+    location.setInnerText(info.has_line()
+        ? Util.M.lineHeader(info.line())
+        : Util.C.fileCommentHeader());
+    if (info.message() != null) {
+      message.setInnerSafeHtml(clp.apply(new SafeHtmlBuilder()
+          .append(info.message().trim()).wikify()));
+    }
+  }
+}

gerrit-gwtui/src/main/java/com/google/gerrit/client/change/LineComment.ui.xml

@@ -0,0 +1,40 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+Copyright (C) 2013 The Android Open Source Project
+
+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.
+-->
+<ui:UiBinder
+    xmlns:ui='urn:ui:com.google.gwt.uibinder'
+    xmlns:c='urn:import:com.google.gerrit.client'
+    xmlns:g='urn:import:com.google.gwt.user.client.ui'>
+  <ui:style>
+    .box {
+      position: relative;
+    }
+    .location {
+      position: absolute;
+      top: 0;
+      left: 0;
+      font-weight: bold;
+    }
+    .message {
+      margin-left: 100px;
+    }
+  </ui:style>
+
+  <g:HTMLPanel styleName='{style.box}'>
+    <div class='{style.location}' ui:field='location'/>
+    <div class='{style.message}' ui:field='message'/>
+  </g:HTMLPanel>
+</ui:UiBinder>

gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Message.java

@@ -18,8 +18,11 @@ import com.google.gerrit.client.AvatarImage;
 import com.google.gerrit.client.FormatUtil;
 import com.google.gerrit.client.Gerrit;
 import com.google.gerrit.client.changes.ChangeInfo.MessageInfo;
+import com.google.gerrit.client.changes.CommentInfo;
 import com.google.gerrit.client.changes.Util;
 import com.google.gerrit.client.ui.CommentLinkProcessor;
+import com.google.gerrit.reviewdb.client.Patch;
+import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gwt.core.client.GWT;
 import com.google.gwt.dom.client.Element;
 import com.google.gwt.event.dom.client.ClickEvent;
@@ -28,10 +31,17 @@ import com.google.gwt.resources.client.CssResource;
 import com.google.gwt.uibinder.client.UiBinder;
 import com.google.gwt.uibinder.client.UiField;
 import com.google.gwt.user.client.ui.Composite;
+import com.google.gwt.user.client.ui.FlowPanel;
 import com.google.gwt.user.client.ui.HTMLPanel;
 import com.google.gwt.user.client.ui.UIObject;
 import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder;
 
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.TreeMap;
+
 class Message extends Composite {
   interface Binder extends UiBinder<HTMLPanel, Message> {}
   private static final Binder uiBinder = GWT.create(Binder.class);
@@ -46,11 +56,16 @@ class Message extends Composite {
   @UiField Element summary;
   @UiField Element date;
   @UiField Element message;
+  @UiField FlowPanel comments;
+
+  private final History history;
+  private final MessageInfo info;
+  private List<CommentInfo> commentList;
 
   @UiField(provided = true)
   AvatarImage avatar;
 
-  Message(CommentLinkProcessor clp, MessageInfo info) {
+  Message(History parent, MessageInfo info) {
     if (info.author() != null) {
       avatar = new AvatarImage(info.author());
       avatar.setSize("", "");
@@ -66,23 +81,40 @@ class Message extends Composite {
       }
     }, ClickEvent.getType());
 
+    this.history = parent;
+    this.info = info;
+
     name.setInnerText(authorName(info));
     date.setInnerText(FormatUtil.shortFormatDayTime(info.date()));
     if (info.message() != null) {
       String msg = info.message().trim();
       summary.setInnerText(msg);
-      message.setInnerSafeHtml(clp.apply(
+      message.setInnerSafeHtml(history.getCommentLinkProcessor()
-          new SafeHtmlBuilder().append(msg).wikify()));
+        .apply(new SafeHtmlBuilder().append(msg).wikify()));
     }
   }
 
+  MessageInfo getMessageInfo() {
+    return info;
+  }
+
   private boolean isOpen() {
     return UIObject.isVisible(message);
   }
 
   void setOpen(boolean open) {
+    if (open && info._revisionNumber() > 0) {
+      if (commentList == null) {
+        history.load(info._revisionNumber());
+      } else if (!commentList.isEmpty()) {
+        renderComments(commentList);
+        commentList = Collections.emptyList();
+      }
+    }
+
     UIObject.setVisible(summary, !open);
     UIObject.setVisible(message, open);
+    comments.setVisible(open && comments.getWidgetCount() > 0);
     if (open) {
       removeStyleName(style.closed());
     } else {
@@ -90,6 +122,46 @@ class Message extends Composite {
     }
   }
 
+  void addComments(List<CommentInfo> list) {
+    if (isOpen()) {
+      renderComments(list);
+      comments.setVisible(comments.getWidgetCount() > 0);
+      commentList = Collections.emptyList();
+    } else {
+      commentList = list;
+    }
+  }
+
+  private void renderComments(List<CommentInfo> list) {
+    CommentLinkProcessor clp = history.getCommentLinkProcessor();
+    PatchSet.Id ps = new PatchSet.Id(
+        history.getChangeId(),
+        info._revisionNumber());
+    TreeMap<String, List<CommentInfo>> m = byPath(list);
+    List<CommentInfo> l = m.remove(Patch.COMMIT_MSG);
+    if (l != null) {
+      comments.add(new FileComments(clp, ps, Util.C.commitMessage(), l));
+    }
+    for (Map.Entry<String, List<CommentInfo>> e : m.entrySet()) {
+      comments.add(new FileComments(clp, ps, e.getKey(), e.getValue()));
+    }
+  }
+
+  private static TreeMap<String, List<CommentInfo>>
+  byPath(List<CommentInfo> list) {
+    TreeMap<String, List<CommentInfo>> m =
+        new TreeMap<String, List<CommentInfo>>();
+    for (CommentInfo c : list) {
+      List<CommentInfo> l = m.get(c.path());
+      if (l == null) {
+        l = new ArrayList<CommentInfo>();
+        m.put(c.path(), l);
+      }
+      l.add(c);
+    }
+    return m;
+  }
+
   static String authorName(MessageInfo info) {
     if (info.author() != null) {
       if (info.author().name() != null) {

gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Message.ui.xml

@@ -93,6 +93,7 @@ limitations under the License.
       <div ui:field='message'
            aria-hidden='true'
            style='display: NONE'/>
+      <g:FlowPanel ui:field='comments' visible='false'/>
     </div>
   </g:HTMLPanel>
 </ui:UiBinder>

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java

@@ -267,6 +267,7 @@ public class ChangeInfo extends JavaScriptObject {
   public static class MessageInfo extends JavaScriptObject {
     public final native AccountInfo author() /*-{ return this.author; }-*/;
     public final native String message() /*-{ return this.message; }-*/;
+    public final native int _revisionNumber() /*-{ return this._revision_number || 0; }-*/;
     private final native String dateRaw() /*-{ return this.date; }-*/;
 
     public final Timestamp date() {

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentInfo.java

@@ -42,7 +42,7 @@ public class CommentInfo extends JavaScriptObject {
   }
 
   private final native void setId(String id) /*-{ this.id = id; }-*/;
-  private final native void setPath(String path) /*-{ this.path = path; }-*/;
+  public final native void setPath(String path) /*-{ this.path = path; }-*/;
 
   private final void setSide(Side side) {
     setSideRaw(side.toString());
@@ -68,7 +68,7 @@ public class CommentInfo extends JavaScriptObject {
   }
   private final native String sideRaw() /*-{ return this.side }-*/;
 
-  public final native int line() /*-{ return this.line; }-*/;
+  public final native int line() /*-{ return this.line || 0; }-*/;
   public final native String in_reply_to() /*-{ return this.in_reply_to; }-*/;
   public final native String message() /*-{ return this.message; }-*/;