Merge pull request #1961 from gitbucket/improve-pullreq-comments

Improve pull request comments presentation

src/main/scala/gitbucket/core/controller/PullRequestsController.scala

@@ -1,6 +1,6 @@
 package gitbucket.core.controller
 
-import gitbucket.core.model.WebHook
+import gitbucket.core.model.{CommitComment, CommitComments, IssueComment, WebHook}
 import gitbucket.core.plugin.PluginRegistry
 import gitbucket.core.pulls.html
 import gitbucket.core.service.CommitStatusService
@@ -113,25 +113,19 @@ trait PullRequestsControllerBase extends ControllerBase {
         val name = repository.name
         getPullRequest(owner, name, issueId) map {
           case (issue, pullreq) =>
-            using(Git.open(getRepositoryDir(owner, name))) {
-              git =>
-                val (commits, diffs) =
+            val (commits, _) =
               getRequestCompareInfo(owner, name, pullreq.commitIdFrom, owner, name, pullreq.commitIdTo)
-                html.pullreq(
+
+            html.conversation(
               issue,
               pullreq,
-                  (commits.flatten
-                    .map(commit => getCommitComments(owner, name, commit.id, true))
-                    .flatten
-                    .toList ::: getComments(owner, name, issueId))
-                    .sortWith((a, b) => a.registeredDate before b.registeredDate),
+              commits.flatten,
+              getPullRequestComments(owner, name, issue.issueId, commits.flatten),
               getIssueLabels(owner, name, issueId),
               getAssignableUserNames(owner, name),
               getMilestonesWithIssueCount(owner, name),
               getPriorities(owner, name),
               getLabels(owner, name),
-                  commits,
-                  diffs,
               isEditable(repository),
               isManageable(repository),
               hasDeveloperRole(pullreq.requestUserName, pullreq.requestRepositoryName, context.loginAccount),
@@ -139,7 +133,69 @@ trait PullRequestsControllerBase extends ControllerBase {
               getRepository(pullreq.requestUserName, pullreq.requestRepositoryName),
               flash.toMap.map(f => f._1 -> f._2.toString)
             )
+
+//                html.pullreq(
+//                  issue,
+//                  pullreq,
+//                  comments,
+//                  getIssueLabels(owner, name, issueId),
+//                  getAssignableUserNames(owner, name),
+//                  getMilestonesWithIssueCount(owner, name),
+//                  getPriorities(owner, name),
+//                  getLabels(owner, name),
+//                  commits,
+//                  diffs,
+//                  isEditable(repository),
+//                  isManageable(repository),
+//                  hasDeveloperRole(pullreq.requestUserName, pullreq.requestRepositoryName, context.loginAccount),
+//                  repository,
+//                  getRepository(pullreq.requestUserName, pullreq.requestRepositoryName),
+//                  flash.toMap.map(f => f._1 -> f._2.toString)
+//                )
         }
+    } getOrElse NotFound()
+  })
+
+  get("/:owner/:repository/pull/:id/commits")(referrersOnly { repository =>
+    params("id").toIntOpt.flatMap { issueId =>
+      val owner = repository.owner
+      val name = repository.name
+      getPullRequest(owner, name, issueId) map {
+        case (issue, pullreq) =>
+          val (commits, _) =
+            getRequestCompareInfo(owner, name, pullreq.commitIdFrom, owner, name, pullreq.commitIdTo)
+
+          html.commits(
+            issue,
+            pullreq,
+            commits,
+            getPullRequestComments(owner, name, issue.issueId, commits.flatten),
+            isManageable(repository),
+            repository
+          )
+      }
+    } getOrElse NotFound()
+  })
+
+  get("/:owner/:repository/pull/:id/files")(referrersOnly { repository =>
+    params("id").toIntOpt.flatMap {
+      issueId =>
+        val owner = repository.owner
+        val name = repository.name
+        getPullRequest(owner, name, issueId) map {
+          case (issue, pullreq) =>
+            val (commits, diffs) =
+              getRequestCompareInfo(owner, name, pullreq.commitIdFrom, owner, name, pullreq.commitIdTo)
+
+            html.files(
+              issue,
+              pullreq,
+              diffs,
+              commits.flatten,
+              getPullRequestComments(owner, name, issue.issueId, commits.flatten),
+              isManageable(repository),
+              repository
+            )
         }
     } getOrElse NotFound()
   })

src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala

@@ -103,7 +103,8 @@ trait RepositoryViewerControllerBase extends ControllerBase {
     oldLineNumber: Option[Int],
     newLineNumber: Option[Int],
     content: String,
-    issueId: Option[Int]
+    issueId: Option[Int],
+    diff: Option[String]
   )
 
   val uploadForm = mapping(
@@ -138,7 +139,8 @@ trait RepositoryViewerControllerBase extends ControllerBase {
     "oldLineNumber" -> trim(label("Old line number", optional(number()))),
     "newLineNumber" -> trim(label("New line number", optional(number()))),
     "content" -> trim(label("Content", text(required))),
-    "issueId" -> trim(label("Issue Id", optional(number())))
+    "issueId" -> trim(label("Issue Id", optional(number()))),
+    "diff" -> optional(text())
   )(CommentForm.apply)
 
   /**
@@ -562,6 +564,22 @@ trait RepositoryViewerControllerBase extends ControllerBase {
       form.newLineNumber,
       form.issueId
     )
+
+    for {
+      fileName <- form.fileName
+      diff <- form.diff
+    } {
+      saveCommitCommentDiff(
+        repository.owner,
+        repository.name,
+        id,
+        fileName,
+        form.oldLineNumber,
+        form.newLineNumber,
+        diff
+      )
+    }
+
     form.issueId match {
       case Some(issueId) =>
         recordCommentPullRequestActivity(
@@ -613,6 +631,22 @@ trait RepositoryViewerControllerBase extends ControllerBase {
       form.newLineNumber,
       form.issueId
     )
+
+    for {
+      fileName <- form.fileName
+      diff <- form.diff
+    } {
+      saveCommitCommentDiff(
+        repository.owner,
+        repository.name,
+        id,
+        fileName,
+        form.oldLineNumber,
+        form.newLineNumber,
+        diff
+      )
+    }
+
     val comment = getCommitComment(repository.owner, repository.name, commentId.toString).get
     form.issueId match {
       case Some(issueId) =>

src/main/scala/gitbucket/core/model/Comment.scala

@@ -1,6 +1,7 @@
 package gitbucket.core.model
+import java.util.Date
 
-trait Comment {
+sealed trait Comment {
   val commentedUserName: String
   val registeredDate: java.util.Date
 }
@@ -87,3 +88,11 @@ case class CommitComment(
   updatedDate: java.util.Date,
   issueId: Option[Int]
 ) extends Comment
+
+case class CommitComments(
+  fileName: String,
+  commentedUserName: String,
+  registeredDate: Date,
+  comments: Seq[CommitComment],
+  diff: Option[String]
+) extends Comment

src/main/scala/gitbucket/core/service/CommitsService.scala

@@ -1,9 +1,14 @@
 package gitbucket.core.service
 
+import java.io.File
+
 import gitbucket.core.model.CommitComment
 import gitbucket.core.model.Profile._
 import gitbucket.core.model.Profile.profile.blockingApi._
 import gitbucket.core.model.Profile.dateColumnType
+import gitbucket.core.util.Directory._
+import gitbucket.core.util.StringUtil
+import org.apache.commons.io.FileUtils
 
 trait CommitsService {
 
@@ -68,4 +73,48 @@ trait CommitsService {
 
   def deleteCommitComment(commentId: Int)(implicit s: Session) =
     CommitComments filter (_.byPrimaryKey(commentId)) delete
+
+  def saveCommitCommentDiff(
+    owner: String,
+    repository: String,
+    commitId: String,
+    fileName: String,
+    oldLine: Option[Int],
+    newLine: Option[Int],
+    diffJson: String
+  ): Unit = {
+    val dir = new java.io.File(getDiffDir(owner, repository), commitId)
+    if (!dir.exists) {
+      dir.mkdirs()
+    }
+    val file = diffFile(dir, fileName, oldLine, newLine)
+    FileUtils.write(file, diffJson, "UTF-8")
+  }
+
+  def loadCommitCommentDiff(
+    owner: String,
+    repository: String,
+    commitId: String,
+    fileName: String,
+    oldLine: Option[Int],
+    newLine: Option[Int]
+  ): Option[String] = {
+    val dir = new java.io.File(getDiffDir(owner, repository), commitId)
+    val file = diffFile(dir, fileName, oldLine, newLine)
+    if (file.exists) {
+      Option(FileUtils.readFileToString(file, "UTF-8"))
+    } else None
+  }
+
+  private def diffFile(dir: java.io.File, fileName: String, oldLine: Option[Int], newLine: Option[Int]): File = {
+    new File(
+      dir,
+      StringUtil.sha1(
+        fileName +
+          "_oldLine:" + oldLine.map(_.toString).getOrElse("") +
+          "_newLine:" + newLine.map(_.toString).getOrElse("")
+      )
+    )
+  }
+
 }

src/main/scala/gitbucket/core/service/PullRequestService.scala

@@ -1,6 +1,6 @@
 package gitbucket.core.service
 
-import gitbucket.core.model.{Issue, PullRequest, CommitStatus, CommitState, CommitComment}
+import gitbucket.core.model.{CommitComments => _, Session => _, _}
 import gitbucket.core.model.Profile._
 import gitbucket.core.model.Profile.profile.blockingApi._
 import difflib.{Delta, DiffUtils}
@@ -12,6 +12,7 @@ import gitbucket.core.util.JGitUtil.{CommitInfo, DiffInfo}
 import gitbucket.core.view
 import gitbucket.core.view.helpers
 import org.eclipse.jgit.api.Git
+
 import scala.collection.JavaConverters._
 
 trait PullRequestService { self: IssuesService with CommitsService =>
@@ -314,11 +315,49 @@ trait PullRequestService { self: IssuesService with CommitsService =>
           helpers.date(commit1.commitTime) == view.helpers.date(commit2.commitTime)
         }
 
+      // TODO Isolate to an another method?
       val diffs = JGitUtil.getDiffs(newGit, Some(oldId.getName), newId.getName, true, false)
 
       (commits, diffs)
     }
 
+  def getPullRequestComments(userName: String, repositoryName: String, issueId: Int, commits: Seq[CommitInfo])(
+    implicit s: Session
+  ): Seq[Comment] = {
+    (commits
+      .map(commit => getCommitComments(userName, repositoryName, commit.id, true))
+      .flatten ++ getComments(userName, repositoryName, issueId))
+      .groupBy {
+        case x: IssueComment                        => (Some(x.commentId), None, None, None)
+        case x: CommitComment if x.fileName.isEmpty => (Some(x.commentId), None, None, None)
+        case x: CommitComment                       => (None, x.fileName, x.oldLine, x.newLine)
+        case x                                      => throw new MatchError(x)
+      }
+      .toSeq
+      .map {
+        // Normal comment
+        case ((Some(_), _, _, _), comments) =>
+          comments.head
+        // Comment on a specific line of a commit
+        case ((None, Some(fileName), oldLine, newLine), comments) =>
+          gitbucket.core.model.CommitComments(
+            fileName = fileName,
+            commentedUserName = comments.head.commentedUserName,
+            registeredDate = comments.head.registeredDate,
+            comments = comments.map(_.asInstanceOf[CommitComment]),
+            diff = loadCommitCommentDiff(
+              userName,
+              repositoryName,
+              comments.head.asInstanceOf[CommitComment].commitId,
+              fileName,
+              oldLine,
+              newLine
+            )
+          )
+      }
+      .sortWith(_.registeredDate before _.registeredDate)
+  }
+
 }
 
 object PullRequestService {

src/main/scala/gitbucket/core/util/Directory.scala

@@ -67,6 +67,12 @@ object Directory {
   def getLfsDir(owner: String, repository: String): File =
     new File(getRepositoryFilesDir(owner, repository), "lfs")
 
+  /**
+   * Directory for files which store diff fragment
+   */
+  def getDiffDir(owner: String, repository: String): File =
+    new File(getRepositoryFilesDir(owner, repository), "diff")
+
   /**
    * Directory for uploaded files by the specified user.
    */

src/main/scala/gitbucket/core/view/helpers.scala

@@ -3,6 +3,7 @@ package gitbucket.core.view
 import java.text.SimpleDateFormat
 import java.util.{Date, Locale, TimeZone}
 
+import com.nimbusds.jose.util.JSONObjectUtils
 import gitbucket.core.controller.Context
 import gitbucket.core.model.CommitState
 import gitbucket.core.plugin.{PluginRegistry, RenderRequest}
@@ -462,4 +463,44 @@ object helpers extends AvatarImageProvider with LinkConverter with RequestCache
    */
   def readableSize(size: Option[Long]): String = FileUtil.readableSize(size.getOrElse(0))
 
+  /**
+   * Make HTML fragment of the partial diff for a comment on a line of diff.
+   *
+   * @param jsonString JSON string which is stored in COMMIT_COMMENT table.
+   * @return HTML fragment of diff
+   */
+  def diff(jsonString: String): Html = {
+    import org.json4s._
+    import org.json4s.jackson.JsonMethods._
+    implicit val formats = DefaultFormats
+
+    val diff = parse(jsonString).extract[Seq[CommentDiffLine]]
+
+    val sb = new StringBuilder()
+    sb.append("<table class=\"diff inlinediff\">")
+    diff.foreach { line =>
+      sb.append("<tr>")
+      sb.append(s"""<th class="line-num oldline ${line.`type`}">""")
+      line.oldLine.foreach { oldLine =>
+        sb.append(oldLine)
+      }
+      sb.append("</th>")
+      sb.append(s"""<th class="line-num newline ${line.`type`}">""")
+      line.newLine.foreach { newLine =>
+        sb.append(newLine)
+      }
+      sb.append("</th>")
+
+      sb.append(s"""<td class="body ${line.`type`}">""")
+      sb.append(StringUtil.escapeHtml(line.text))
+      sb.append("</td>")
+      sb.append("</tr>")
+    }
+    sb.append("</table>")
+
+    Html(sb.toString())
+  }
+
+  case class CommentDiffLine(newLine: Option[String], oldLine: Option[String], `type`: String, text: String)
+
 }

src/main/twirl/gitbucket/core/helper/commitcomment.scala.html

@@ -3,26 +3,16 @@
   repository: gitbucket.core.service.RepositoryService.RepositoryInfo,
   latestCommitId: Option[String] = None)(implicit context: gitbucket.core.controller.Context)
 @import gitbucket.core.view.helpers
-<div class="@if(comment.fileName.isDefined && (!latestCommitId.isDefined || latestCommitId.get == comment.commitId)){inline-comment}"
-    id="discussion_r@comment.commentId"
+<div class="commit-comment-box @if((!latestCommitId.isDefined || latestCommitId.get == comment.commitId)){inline-comment}" id="discussion_r@comment.commentId"
   @if(comment.fileName.isDefined){filename="@comment.fileName.get"}
   @if(comment.newLine.isDefined){newline="@comment.newLine.get"}
   @if(comment.oldLine.isDefined){oldline="@comment.oldLine.get"}>
-  <div class="panel panel-default commit-comment-box commit-comment-@comment.commentId">
-    <div class="panel-heading">
+  <div class="commit-comment-@comment.commentId">
+    <div class="markdown-body">
+      <div>
         @helpers.avatar(comment.commentedUserName, 20)
         @helpers.user(comment.commentedUserName, styleClass="username strong")
-        <span class="muted">
-          commented on
-          @if(comment.issueId.isDefined){
-            <a href="@helpers.url(repository)/pull/@comment.issueId">#@comment.issueId</a>
-          }
-          @comment.fileName.map { fileName =>
-            @fileName in
-          }
-          <a href="@context.path/@repository.owner/@repository.name/commit/@comment.commitId">@comment.commitId.substring(0, 7)</a>
-          @gitbucket.core.helper.html.datetimeago(comment.registeredDate)
-        </span>
+        <span class="muted">@gitbucket.core.helper.html.datetimeago(comment.registeredDate)</span>
         <span class="pull-right">
         @if(hasWritePermission || context.loginAccount.map(_.userName == comment.commentedUserName).getOrElse(false)){
           <a href="#" data-comment-id="@comment.commentId"><i class="octicon octicon-pencil"></i></a>&nbsp;
@@ -30,7 +20,7 @@
         }
         </span>
       </div>
-    <div class="panel-body issue-content commit-commentContent-@comment.commentId markdown-body">
+      <div class="commit-commentContent-@comment.commentId">
         @helpers.markdown(
           markdown = comment.content,
           repository = repository,
@@ -42,4 +32,5 @@
         )
       </div>
     </div>
+  </div>
 </div>

src/main/twirl/gitbucket/core/helper/commitcomments.scala.html

@@ -0,0 +1,21 @@
+@(comments: gitbucket.core.model.CommitComments,
+  hasWritePermission: Boolean,
+  repository: gitbucket.core.service.RepositoryService.RepositoryInfo,
+  latestCommitId: Option[String] = None)(implicit context: gitbucket.core.controller.Context)
+@import gitbucket.core.view.helpers
+<div class="panel panel-default">
+  <div class="panel-heading">
+    <span class="monospace">@comments.fileName</span>
+    @if(!latestCommitId.contains(comments.comments.head.commitId)) {
+      <span class="pull-right"><a href="javascript:$('.outdated-comments-@comments.comments.head.commentId').toggle();"><i class="octicon octicon-unfold"></i></a></span>
+    }
+  </div>
+  <div style="@if(!latestCommitId.contains(comments.comments.head.commitId)){display: none;}" class="outdated-comments-@comments.comments.head.commentId">
+    @comments.diff.map(helpers.diff)
+    <div class="panel-body">
+    @comments.comments.map { comment =>
+      @gitbucket.core.helper.html.commitcomment(comment, hasWritePermission, repository, latestCommitId)
+    }
+    </div>
+  </div>
+</div>

src/main/twirl/gitbucket/core/helper/diff.scala.html

@@ -365,14 +365,14 @@ $(function(){
     });
     for(var key in elements){
       filename = elements[key]['filename'];
-      oldline = elements[key]['oldline'];
-      newline = elements[key]['newline'];
+      oldline = elements[key]['oldline'] ? elements[key]['oldline'] : '';
+      newline = elements[key]['newline'] ? elements[key]['newline'] : '';
 
       var $v = $('<div class="commit-comment-box reply-comment-box">')
-        .append($('<input type="text" class="form-control reply-comment" placeholder="Reply...">')
-          .data('filename', filename)
-          .data('newline',  newline)
-          .data('oldline',  oldline));
+        .append($('<input type="text" class="form-control reply-comment" placeholder="Reply..." '
+                       + 'data-filename="' + filename + '" '
+                       + 'data-newline="' + newline + '" '
+                       + 'data-oldline="' + oldline + '">'));
 
       var tmp;
       if (typeof oldline !== 'undefined') {

src/main/twirl/gitbucket/core/issues/commentlist.scala.html

@@ -4,9 +4,8 @@
   repository: gitbucket.core.service.RepositoryService.RepositoryInfo,
   pullreq: Option[gitbucket.core.model.PullRequest] = None)(implicit context: gitbucket.core.controller.Context)
 @import gitbucket.core.view.helpers
-@import gitbucket.core.model.CommitComment
 @issueOrPullRequest()={ @if(issue.exists(_.isPullRequest))( "pull request" )else( "issue" ) }
-@showFormatedComment(comment: gitbucket.core.model.IssueComment)={
+@showFormattedComment(comment: gitbucket.core.model.IssueComment)={
 <div class="panel panel-default issue-comment-box" id="comment-@comment.commentId">
   <div class="panel-heading">
     @helpers.avatar(comment.commentedUserName, 20)
@@ -29,7 +28,7 @@
       </span>
     }
   </div>
-  <div class="panel-body issue-content markdown-body" id="commentContent-@comment.commentId">
+  <div class="panel-body markdown-body" id="commentContent-@comment.commentId">
     @helpers.markdown(
       markdown = comment.content,
       repository = repository,
@@ -54,7 +53,7 @@
         }
       </span>
     </div>
-    <div class="panel-body issue-content markdown-body" id="issueContent">
+    <div class="panel-body markdown-body" id="issueContent">
       @helpers.markdown(
         markdown = issue.get.content getOrElse "No description provided.",
         repository = repository,
@@ -117,7 +116,7 @@
         </div>
       }
       case "merge" => {
-        @showFormatedComment(comment)
+        @showFormattedComment(comment)
         <div class="discussion-item discussion-item-merge">
           <div class="discussion-item-header">
             <span class="discussion-item-icon"><i class="octicon octicon-git-merge"></i></span>
@@ -136,7 +135,7 @@
       }
       case "close" | "close_comment" => {
         @if(comment.action == "close_comment"){
-          @showFormatedComment(comment)
+          @showFormattedComment(comment)
         }
         <div class="discussion-item discussion-item-close">
           <div class="discussion-item-header">
@@ -150,7 +149,7 @@
       }
       case "reopen" | "reopen_comment" => {
         @if(comment.action == "reopen_comment"){
-          @showFormatedComment(comment)
+          @showFormattedComment(comment)
         }
         <div class="discussion-item discussion-item-reopen">
           <div class="discussion-item-header">
@@ -229,12 +228,43 @@
         </div>
       }
       case _ => {
-        @showFormatedComment(comment)
+        @showFormattedComment(comment)
       }
     }
   }
-  case comment: CommitComment => {
-    @gitbucket.core.helper.html.commitcomment(comment, isManageable, repository, pullreq.map(_.commitIdTo))
+  case comments: gitbucket.core.model.CommitComments => {
+    @gitbucket.core.helper.html.commitcomments(comments, isManageable, repository, pullreq.map(_.commitIdTo))
+  }
+  case comment: gitbucket.core.model.CommitComment => {
+    <div class="panel panel-default issue-comment-box" id="comment-@comment.commentId">
+      <div class="panel-heading">
+        @helpers.avatar(comment.commentedUserName, 20)
+        @helpers.user(comment.commentedUserName, styleClass="username strong")
+      <span class="muted">
+        commented
+        <a href="#comment-@comment.commentId">@gitbucket.core.helper.html.datetimeago(comment.registeredDate)</a>
+        on
+        <a href="@helpers.url(repository)/commit/@comment.commitId" class="monospace">@comment.commitId.substring(0, 7)</a>
+      </span>
+        @if((isManageable || context.loginAccount.map(_.userName == comment.commentedUserName).getOrElse(false))){
+          <span class="pull-right">
+            <a href="#" data-comment-id="@comment.commentId"><i class="octicon octicon-pencil" aria-label="Edit"></i></a>&nbsp;
+            <a href="#" data-comment-id="@comment.commentId"><i class="octicon octicon-x" aria-label="Remove"></i></a>
+          </span>
+        }
+      </div>
+      <div class="panel-body markdown-body" id="commentContent-@comment.commentId">
+      @helpers.markdown(
+        markdown = comment.content,
+        repository = repository,
+        enableWikiLink = false,
+        enableRefsLink = true,
+        enableLineBreaks = true,
+        enableTaskList = true,
+        hasWritePermission = isManageable
+      )
+      </div>
+    </div>
   }
 }
 <script>
@@ -286,11 +316,25 @@ $(function(){
       $.post('@helpers.url(repository)/commit_comments/delete/' + id,
       function(data){
         if(data > 0) {
-          var comment = $('.commit-comment-' + id).closest('.not-diff');
-          if(comment.prev('.not-diff').length == 0){
-            comment.next('.not-diff').find('.reply-comment').remove();
+          var comment = $('.commit-comment-' + id);
+
+          // diff view
+          var tr = comment.closest('.not-diff');
+          if(tr.length > 0){
+            if(tr.prev('.not-diff').length == 0){
+              tr.next('.not-diff:has(.reply-comment)').remove();
+            }
+            tr.remove();
+          }
+
+          // comment list view
+          var panel = comment.closest('div.panel:has(.commit-comment-box)');
+          if(panel.length > 0){
+            comment.parent('.commit-comment-box').remove();
+            if(panel.has('.commit-comment-box').length == 0){
+              panel.remove();
+            }
           }
-          comment.remove();
         }
       });
     }

src/main/twirl/gitbucket/core/pulls/commits.scala.html

@@ -1,9 +1,13 @@
-@(commits: Seq[Seq[gitbucket.core.util.JGitUtil.CommitInfo]],
-  comments: Option[List[gitbucket.core.model.Comment]] = None,
+@(issue: gitbucket.core.model.Issue,
+  pullreq: gitbucket.core.model.PullRequest,
+  commits: Seq[Seq[gitbucket.core.util.JGitUtil.CommitInfo]],
+  comments: Seq[gitbucket.core.model.Comment],
+  isManageable: Boolean,
   repository: gitbucket.core.service.RepositoryService.RepositoryInfo)(implicit context: gitbucket.core.controller.Context)
 @import gitbucket.core.view.helpers
-<table class="table table-bordered">
-@commits.map { day =>
+@gitbucket.core.pulls.html.menu("commits", issue, pullreq, commits.flatten, comments, isManageable, repository){
+  <table class="table table-bordered">
+  @commits.map { day =>
     <tr>
       <th rowspan="@day.size" width="100">@helpers.date(day.head.commitTime)</th>
       @day.zipWithIndex.map { case (commit, i) =>
@@ -38,5 +42,6 @@
         </td>
         </tr>
       }
+  }
+  </table>
 }
-</table>

src/main/twirl/gitbucket/core/pulls/conversation.scala.html

@@ -1,7 +1,7 @@
 @(issue: gitbucket.core.model.Issue,
   pullreq: gitbucket.core.model.PullRequest,
   commits: Seq[gitbucket.core.util.JGitUtil.CommitInfo],
-  comments: List[gitbucket.core.model.Comment],
+  comments: Seq[gitbucket.core.model.Comment],
   issueLabels: List[gitbucket.core.model.Label],
   collaborators: List[String],
   milestones: List[(gitbucket.core.model.Milestone, Int, Int)],
@@ -11,11 +11,14 @@
   isManageable: Boolean,
   isManageableForkedRepository: Boolean,
   repository: gitbucket.core.service.RepositoryService.RepositoryInfo,
-  forkedRepository: Option[gitbucket.core.service.RepositoryService.RepositoryInfo])(implicit context: gitbucket.core.controller.Context)
+  forkedRepository: Option[gitbucket.core.service.RepositoryService.RepositoryInfo],
+  flash: Map[String, String])(implicit context: gitbucket.core.controller.Context)
 @import gitbucket.core.view.helpers
-<div class="col-md-9">
+@gitbucket.core.pulls.html.menu("conversation", issue, pullreq, commits, comments, isManageable, repository, flash){
+  <link href="@helpers.assets("/vendors/jsdifflib/diffview.css")" type="text/css" rel="stylesheet" />
+  <div class="col-md-9">
     <div id="comment-list">
-    @gitbucket.core.issues.html.commentlist(Some(issue), comments, isManageable, repository, Some(pullreq))
+      @gitbucket.core.issues.html.commentlist(Some(issue), comments.toList, isManageable, repository, Some(pullreq))
     </div>
     @defining(comments.flatMap {
       case comment: gitbucket.core.model.IssueComment => Some(comment)
@@ -24,7 +27,7 @@
       @if(!issue.closed){
         <div class="check-conflict" style="display: none;">
           <div class="issue-comment-box" style="background-color: #fbeed5">
-          <div class="box-content"class="issue-content" style="border: 1px solid #c09853; padding: 10px;">
+            <div class="box-content" style="border: 1px solid #c09853; padding: 10px;">
               <img src="@helpers.assets("/common/images/indicator.gif")"/> Checking...
             </div>
           </div>
@@ -33,7 +36,7 @@
       @if(isManageableForkedRepository && issue.closed && merged &&
         forkedRepository.map(r => (r.branchList.contains(pullreq.requestBranch) && r.repository.defaultBranch != pullreq.requestBranch)).getOrElse(false)){
         <div class="issue-comment-box" style="background-color: #d0eeff;">
-        <div class="box-content"class="issue-content" style="border: 1px solid #87a8c9; padding: 10px;">
+          <div class="box-content" style="border: 1px solid #87a8c9; padding: 10px;">
             <a href="@helpers.url(repository)/pull/@issue.issueId/delete_branch" class="btn btn-info pull-right delete-branch" data-name="@pullreq.requestBranch">Delete branch</a>
             <div>
               <span class="strong">Pull request successfully merged and closed</span>
@@ -44,12 +47,12 @@
       }
       @gitbucket.core.issues.html.commentform(issue, !merged, isEditable, isManageable, repository)
     }
-</div>
-<div class="col-md-3">
-  @gitbucket.core.issues.html.issueinfo(Some(issue), comments, issueLabels, collaborators, milestones, priorities, None, labels, isManageable, repository)
-</div>
-<script>
-$(function(){
+  </div>
+  <div class="col-md-3">
+    @gitbucket.core.issues.html.issueinfo(Some(issue), comments.toList, issueLabels, collaborators, milestones, priorities, None, labels, isManageable, repository)
+  </div>
+  <script>
+  $(function(){
     @if(commits.nonEmpty){
       var checkConflict = $('.check-conflict').show();
       if(checkConflict.length){
@@ -60,5 +63,6 @@ $(function(){
       var branchName = $(e.target).data('name');
       return confirm('Are you sure you want to remove the ' + branchName + ' branch?');
     });
-});
-</script>
+  });
+  </script>
+}

src/main/twirl/gitbucket/core/pulls/files.scala.html

@@ -0,0 +1,22 @@
+@(issue: gitbucket.core.model.Issue,
+  pullreq: gitbucket.core.model.PullRequest,
+  diffs: Seq[gitbucket.core.util.JGitUtil.DiffInfo],
+  commits: Seq[gitbucket.core.util.JGitUtil.CommitInfo],
+  comments: Seq[gitbucket.core.model.Comment],
+  isManageable: Boolean,
+  repository: gitbucket.core.service.RepositoryService.RepositoryInfo)(implicit context: gitbucket.core.controller.Context)
+@gitbucket.core.pulls.html.menu("files", issue, pullreq, commits, comments, isManageable, repository) {
+  @gitbucket.core.helper.html.diff(
+    diffs,
+    repository,
+    commits.headOption.map(_.id),
+    commits.lastOption.map(_.id),
+    true,
+    Some(pullreq.issueId),
+    isManageable,
+    true
+  )
+  <div id="comment-list" style="display: none;">
+    @gitbucket.core.issues.html.commentlist(Some(issue), comments.toList, isManageable, repository, Some(pullreq))
+  </div>
+}

src/main/twirl/gitbucket/core/pulls/menu.scala.html

@@ -0,0 +1,85 @@
+@(active: String,
+  issue: gitbucket.core.model.Issue,
+  pullreq: gitbucket.core.model.PullRequest,
+  commits: Seq[gitbucket.core.util.JGitUtil.CommitInfo],
+  comments: Seq[gitbucket.core.model.Comment],
+  isManageable: Boolean,
+  repository: gitbucket.core.service.RepositoryService.RepositoryInfo,
+  flash: Map[String, String] = Map.empty)(body: => Html)(implicit context: gitbucket.core.controller.Context)
+@import gitbucket.core.view.helpers
+@import gitbucket.core.model.IssueComment
+@gitbucket.core.html.main(s"${issue.title} - Pull request #${issue.issueId} - ${repository.owner}/${repository.name}", Some(repository)) {
+  @gitbucket.core.html.menu("pulls", repository) {
+      <div>
+        <div class="show-title pull-right">
+          @if(isManageable || context.loginAccount.map(_.userName == issue.openedUserName).getOrElse(false)) {
+            <a class="btn btn-default" href="#" id="edit">Edit</a>
+          }
+          @if(context.loginAccount.isDefined) {
+            <a class="btn btn-success" href="@helpers.url(repository)/compare">New pull request</a>
+          }
+        </div>
+        <div class="edit-title pull-right" style="display: none;">
+          <a class="btn btn-success" href="#" id="update">Save</a>  <a class="btn btn-default" href="#" id="cancel">
+          Cancel</a>
+        </div>
+        <h1 class="body-title">
+          <span class="show-title">
+            <span id="show-title">@issue.title</span>
+            <span class="muted">#@issue.issueId</span>
+          </span>
+          <span class="edit-title" style="display: none;">
+            <span id="error-edit-title" class="error"></span>
+            <input type="text" class="form-control" style="width: 700px;" id="edit-title" value="@issue.title"/>
+          </span>
+        </h1>
+      </div>
+      <div style="margin-bottom: 15px">
+      @if(issue.closed) {
+        @comments.flatMap @{
+          case comment: IssueComment => Some(comment)
+          case _ => None
+        }.find(_.action == "merge").map { comment =>
+        <span class="label label-info issue-status">Merged</span>
+        <span class="muted">
+          @helpers.user(comment.commentedUserName, styleClass = "username strong")
+          merged @commits.size @helpers.plural(commits.size, "commit")
+          into <code>@pullreq.userName:@pullreq.branch</code> from <code>@pullreq.requestUserName
+          :@pullreq.requestBranch</code>
+          @gitbucket.core.helper.html.datetimeago(comment.registeredDate)
+        </span>
+        }.getOrElse {
+          <span class="label label-important issue-status">Closed</span>
+          <span class="muted">
+            @helpers.user(issue.openedUserName, styleClass = "username strong")
+            wants to merge @commits.size @helpers.plural(commits.size, "commit")
+            into <code>@pullreq.userName:@pullreq.branch</code> from <code>@pullreq.requestUserName
+            :@pullreq.requestBranch</code>
+          </span>
+        }
+      } else {
+        <span class="label label-success issue-status">Open</span>
+        <span class="muted">
+          @helpers.user(issue.openedUserName, styleClass = "username strong")
+          wants to merge @commits.size @helpers.plural(commits.size, "commit")
+          into <code>@pullreq.userName:@pullreq.branch</code> from <code>@pullreq.requestUserName
+          :@pullreq.requestBranch</code>
+        </span>
+      }
+      </div>
+      <ul class="nav nav-tabs fill-width" id="pullreq-tab">
+        <li @if(active=="conversation"){class="active"}><a href="@helpers.url(repository)/pull/@issue.issueId">Conversation</a></li>
+        <li @if(active=="commits"){class="active"}><a href="@helpers.url(repository)/pull/@issue.issueId/commits">Commits</a></li>
+        <li @if(active=="files"){class="active"}><a href="@helpers.url(repository)/pull/@issue.issueId/files">Files Changed</a></li>
+      </ul>
+      <div class="tab-content fill-width" style="padding-top: 20px;">
+        @flash.get("error").map { error =>
+          <div class="alert alert-error">@error</div>
+        }
+        @flash.get("info").map { info =>
+          <div class="alert alert-info">@info</div>
+        }
+        @body
+      </div>
+  }
+}

src/main/twirl/gitbucket/core/pulls/mergeguide.scala.html

@@ -5,7 +5,7 @@
   forkedRepository: gitbucket.core.service.RepositoryService.RepositoryInfo)(implicit context: gitbucket.core.controller.Context)
 @import gitbucket.core.view.helpers
 <div class="issue-comment-box" style="background-color: @if(status.hasProblem){ #fbeed5 }else{ #d8f5cd };">
-  <div class="box-content issue-content" style="border: 1px solid @if(status.hasProblem){ #c09853 }else{ #95c97e };padding:0">
+  <div class="box-content" style="border: 1px solid @if(status.hasProblem){ #c09853 }else{ #95c97e };padding:0">
     <div id="merge-pull-request">
       @if(!status.statuses.isEmpty){
         <div class="build-statuses">

src/main/twirl/gitbucket/core/pulls/pullreq.scala.html

@@ -17,6 +17,7 @@
 @import gitbucket.core.view.helpers
 @import gitbucket.core.model.IssueComment
 @import gitbucket.core.model.CommitComment
+@*
 @gitbucket.core.html.main(s"${issue.title} - Pull request #${issue.issueId} - ${repository.owner}/${repository.name}", Some(repository)){
   @gitbucket.core.html.menu("pulls", repository){
     @defining(dayByDayCommits.flatten){ commits =>
@@ -73,7 +74,7 @@
       <ul class="nav nav-tabs fill-width" id="pullreq-tab">
         <li><a href="#conversation">Conversation <span class="badge">@comments.flatMap @{
           case comment: IssueComment => Some(comment)
-          case _: CommitComment => None
+          case _ => None
         }.size</span></a></li>
         <li><a href="#commits">Commits <span class="badge">@commits.size</span></a></li>
         <li><a href="#files">Files Changed <span class="badge">@diffs.size</span></a></li>
@@ -90,7 +91,7 @@
         </div>
         <div class="tab-pane" id="commits">
           @if(commits.nonEmpty){
-            @gitbucket.core.pulls.html.commits(dayByDayCommits, Some(comments), repository)
+            @gitbucket.core.pulls.html.commits(dayByDayCommits, repository)
           }
         </div>
         <div class="tab-pane" id="files">
@@ -157,3 +158,4 @@ $(function(){
   });
 });
 </script>
+*@

src/main/twirl/gitbucket/core/repo/commentform.scala.html

@@ -49,6 +49,15 @@
       $($form.serializeArray()).each(function(i, v) {
         param[v.name] = v.value;
       });
+
+      @if(newLineNumber.isDefined){
+        var diff = getDiffData($('table[filename="@fileName"] table.diff tr:has(th.line-num.newline[line-number=@newLineNumber])'));
+        param['diff'] = JSON.stringify(diff);
+      } else if(oldLineNumber.isDefined){
+        var diff = getDiffData($('table[filename="@fileName"] table.diff tr:has(th.line-num.oldline[line-number=@oldLineNumber])'));
+        param['diff'] = JSON.stringify(diff);
+      }
+
       $.ajax({
         url: '@helpers.url(repository)/commit/@commitId/comment/_data/new',
         type: 'POST',
@@ -85,10 +94,10 @@
           $tr.after(tmp);
         }
 
-        $('#comment-list').append(data);
-        if (typeof $('#show-notes')[0] !== 'undefined' && !$('#show-notes')[0].checked) {
-          $('#comment-list').children('.inline-comment').hide();
-        }
+        // $('#comment-list').append(data);
+        // if (typeof $('#show-notes')[0] !== 'undefined' && !$('#show-notes')[0].checked) {
+        //   $('#comment-list').children('.inline-comment').hide();
+        // }
       }).fail(function(req) {
         $('.btn-inline-comment').removeAttr('disabled');
         $('#error-content', $form).html($.parseJSON(req.responseText).content);
@@ -96,7 +105,6 @@
     });
 
     function getInlineContainer() {
-      console.log(window.viewType);
       if (window.viewType == 0) {
         if(@newLineNumber.isDefined){
           return $('<tr class="not-diff"><td colspan="2"></td><td colspan="2" class="comment-box-container"></td></tr>');
@@ -107,5 +115,31 @@
       }
       return $('<tr class="not-diff"><td colspan="3" class="comment-box-container"></td></tr>');
     }
+
+    function getDiffData(tr){
+      var result = [];
+      var count = 0;
+
+      while(tr && count < 4){
+        var oldTh = tr.find('th.oldline');
+        var newTh = tr.find('th.newline');
+
+        if(!oldTh.attr('line-number') && !newTh.attr('line-number')){
+          break;
+        }
+
+        result.unshift({
+          'oldLine': oldTh.attr('line-number'),
+          'newLine': newTh.attr('line-number'),
+          'type': tr.has('td.insert').length > 0 ? 'insert' : tr.has('td.delete').length > 0 ? 'delete' : 'equal',
+          'text': tr.find('td>span').text()
+        })
+
+        tr = tr.prev('tr:has(th.line-num)');
+        count++;
+      }
+
+      return result;
+    }
   </script>
 }

src/main/twirl/gitbucket/core/repo/commit.scala.html

@@ -1,3 +1,4 @@
+@import gitbucket.core.model.CommitComment
 @(commitId: String,
   commit: gitbucket.core.util.JGitUtil.CommitInfo,
   branches: List[String],
@@ -87,7 +88,20 @@
       <input type="checkbox" id="show-notes"> Show line notes below
     </label>
     <div id="comment-list">
-      @gitbucket.core.issues.html.commentlist(None, comments, hasWritePermission, repository, None)
+      @gitbucket.core.issues.html.commentlist(
+        None,
+        comments.filter(_.asInstanceOf[gitbucket.core.model.CommitComment].fileName.isEmpty),
+        hasWritePermission,
+        repository,
+        None)
+      <div style="display: none;">
+        @gitbucket.core.issues.html.commentlist(
+          None,
+          comments.filter(_.asInstanceOf[gitbucket.core.model.CommitComment].fileName.isDefined),
+          hasWritePermission,
+          repository,
+          None)
+      </div>
     </div>
     @gitbucket.core.repo.html.commentform(commitId = commitId, hasWritePermission = hasWritePermission, repository = repository)
   }

src/main/twirl/gitbucket/core/repo/editcomment.scala.html

@@ -14,7 +14,7 @@
   elastic            = true,
   tabIndex           = 1
 )
-<div class="pull-right">
+<div class="text-right">
   <input type="button" class="cancel-comment-@commentId btn btn-small btn-default" value="Cancel"/>
   <input type="button" class="update-comment-@commentId btn btn-small btn-success" value="Update comment"/>
 </div>

src/main/webapp/assets/common/css/gitbucket.css

@@ -772,20 +772,24 @@ div.issue-participants {
   margin-left: 50px;
 }
 
-div.issue-comment-box, div.commit-comment-box {
-  margin-bottom: 15px;
-}
-
-div.issue-comment-box > div.panel-body,
-div.commit-comment-box > div.panel-body {
-  padding: 8px;
-}
-
-
-div.issue-comment-box textarea {
+div.commit-comment-box textarea,
+div.issue-comment-box textarea{
   height: 100px;
   max-height: 300px;
 }
+/*
+div.commit-comment-box.panel-body {
+  padding-top: 8px;
+  padding-bottom: 0px;
+}
+*/
+div.commit-comment-box div.tabbable {
+  margin-top: 5px;
+}
+
+div.reply-comment-box {
+  margin-bottom: 5px;
+}
 
 div.issue-comment-action {
   padding-bottom: 10px;
@@ -1172,6 +1176,7 @@ table.diff tbody tr.not-diff:hover td {
 .not-diff > .comment-box-container {
   white-space: normal;
   line-height: initial;
+  padding-top: 8px;
   padding-left: 10px;
   padding-right: 10px;
 }