[refs #579] alpha version of the pull discussion

app/controllers/projects/comments_controller.rb

@@ -68,21 +68,30 @@ class Projects::CommentsController < Projects::BaseController
 
       diff = pull.diff repo, to_commit, from_commit
       diff_path = diff.select {|d| d.a_path == params[:path]}
-      return false if diff_path.blank?
-      comment_line = params[:line].to_i
-      return false if comment_line == 0 # NB! also dont want create comment to the diff header
+      return false unless @comment.actual_inline_comment?(diff, true)
 
-      line_number, line_presence = -1, false
+      comment_line, line_number, strings = params[:line].to_i, -1, []
       diff_path[0].diff.each_line do |line|
         line_number = line_number.succ
         # Save 2 lines above and bottom of the diff comment line
         break if line_number > comment_line + 2
         if (comment_line-2..comment_line+2).include? line_number
           @comment.data["line#{line_number-comment_line}"] = line.chomp
-          line_presence = true
+        end
+
+        # Save lines from the closest header for rendering in the discussion
+        if line_number < comment_line - 2
+          # Header is the line like "@@ -47,9 +50,8 @@ def initialize(user)"
+          if line =~ /^@@ [+-]([0-9]+)(?:,([0-9]+))? [+-]([0-9]+)(?:,([0-9]+))? @@/
+            strings = line
+          else
+            strings << line
+          end
         end
       end
-      return line_presence
+      @comment.data[:strings] = strings
+      @comment.data[:view_path] = h(diff_path[0].renamed_file ? "#{diff_path[0].a_path.rtruncate 60} -> #{diff_path[0].b_path.rtruncate 60}" : diff_path[0].a_path.rtruncate(120))
+      return true
     end
   end
 end

app/controllers/projects/pull_requests_controller.rb

@@ -128,16 +128,16 @@ class Projects::PullRequestsController < Projects::BaseController
   end
 
   def load_diff_commits_data
-    repo = Grit::Repo.new(@pull.path)
+    @repo = Grit::Repo.new(@pull.path)
     @base_commit = @pull.common_ancestor
-    @head_commit = repo.commits(@pull.head_branch).first
+    @head_commit = @repo.commits(@pull.head_branch).first
 
-    @commits = repo.commits_between(repo.commits(@pull.to_ref).first, @head_commit)
+    @commits = @repo.commits_between(@repo.commits(@pull.to_ref).first, @head_commit)
     @total_commits = @commits.count
     @commits = @commits.last(100)
 
-    @diff = @pull.diff repo, @base_commit, @head_commit
-    @stats = @pull.diff_stats repo, @base_commit, @head_commit
+    @diff = @pull.diff @repo, @base_commit, @head_commit
+    @stats = @pull.diff_stats @repo, @base_commit, @head_commit
     @comments = @issue.comments
     @commentable = @issue
   end

app/helpers/diff_helper.rb

@@ -21,18 +21,29 @@ module DiffHelper
   end
 
   #include Git::Diff::InlineCallback
-  def render_diff(diff, diff_counter)
-    diff_display ||= Diff::Display::Unified.new(diff.diff)
+  def render_diff(diff, diff_counter, comments, diffpath = nil)
+    if diff.respond_to?(:diff)
+      filepath = diff.a_path
+      diff = diff.diff
+      in_discussion = false
+      comments = comments.select{|c| c.data.try('[]', :path) == filepath}
+    else
+      filepath = diffpath
+      in_discussion = true
+    end
+
+    diff_display ||= Diff::Display::Unified.new(diff)
     url = if @pull
              @pull.id ? polymorphic_path([@project, @pull]) : ''
            elsif @commit
              commit_path @project, @commit
            end
-    prepare(diff, url, diff_counter)
+    prepare(url, diff_counter, filepath, comments, in_discussion)
 
     res = "<table class='diff inline' cellspacing='0' cellpadding='0'>"
     res += "<tbody>"
     res += renderer diff_display.data #diff_display.render(Git::Diff::InlineCallback.new comments, path)
+    res += tr_line_comments(comments) if in_discussion
     res += "</tbody>"
     res += "</table>"
     res.html_safe
@@ -41,9 +52,9 @@ module DiffHelper
   ########################################################
   # FIXME: Just to dev, remove to lib
   ########################################################
-  def prepare(diff, url, diff_counter)
-    @diff, @num_line, @filepath, @url, @diff_counter = diff, -1, diff.a_path, url, diff_counter
-    @line_comments = @comments.select{|c| c.data.try('[]', :path) == @filepath}
+  def prepare(url, diff_counter, filepath, comments, in_discussion)
+    @num_line, @url, @diff_counter, @in_discussion = -1, url, diff_counter, in_discussion
+    @filepath, @line_comments = filepath, comments
   end
 
   def headerline(line)
@@ -216,29 +227,26 @@ module DiffHelper
   end
 
   def render_line_comments
-    comments = @line_comments.select do |c|
-      next false if c.data.try('[]', :line) != @num_line.to_s
-      next true if c.commentable_type == 'Grit::Commit'
-      #diff = Diff::Display::Unified.new(@diff.diff)
-      res, ind = true, 0
-      @diff.diff.each_line do |line|
-        res = false if (@num_line-2..@num_line+2).include?(ind) && c.data.try('[]', "line#{ind-@num_line}") != line.chomp
-        ind = ind + 1
+    unless @in_discussion
+      comments = @line_comments.select do |c|
+        c.data.try('[]', :line) == @num_line.to_s && c.actual_inline_comment?(@diff)
       end
-      res
+      tr_line_comments(comments) if comments.count > 0
     end
+  end
+
+  def td_line_link id, num
+    "<td class='line_numbers' id='#{id}'><a href='#{@url}##{id}'>#{num}</a></td>"
+  end
+
+  def tr_line_comments comments
     "<tr class='inline-comments'>
       <td class='line_numbers' colspan='2'>#{comments.count}</td>
       <td>
         #{render("projects/comments/line_list", :list => comments, :project => @project, :commentable => @commentable)}
         #{link_to t('layout.comments.new_inline'), new_comment_path, :class => 'new_inline_comment button'}
       </td>
-
-     </tr>" if comments.count > 0
-  end
-
-  def td_line_link id, num
-    "<td class='line_numbers' id='#{id}'><a href='#{@url}##{id}'>#{num}</a></td>"
+     </tr>"
   end
 
   def new_comment_path

app/helpers/pull_request_helper.rb

@@ -1,9 +1,9 @@
 # -*- encoding : utf-8 -*-
 module PullRequestHelper
   def merge_activity comments, commits
-    issue_comments = @issue.comments.map{ |c| [c.created_at, c] }
+    pull_comments = comments.group_by(&:data).map{|data, c| [c.first.created_at, [data || {}, [c].flatten]]}
     commits = @commits.map{ |c| [(c.committed_date || c.authored_date), c] }
-    (issue_comments + commits).sort_by{ |c| c[0] }.map{ |c| c[1] }
+    (pull_comments + commits).sort_by{ |c| c[0] }.map{ |c| c[1] }
   end
 
   def pull_status_label pull

app/models/comment.rb

@@ -54,6 +54,36 @@ class Comment < ActiveRecord::Base
     User.find(subscribe.user).notifier.new_comment && User.find(subscribe.user).notifier.can_notify
   end
 
+  def actual_inline_comment?(diff, force = false)
+    return data[:actual] if data[:actual].present? && !force
+    filepath, line_number = data[:path], data[:line]
+    diff_path = diff.select {|d| d.a_path == data[:path]}
+    comment_line = data[:line].to_i
+    # NB! also dont create a comment to the diff header
+    return data[:actual] = false if diff_path.blank? || comment_line == 0
+    return data[:actual] = true if commentable_type == 'Grit::Commit'
+    res, ind = true, 0
+    diff_path[0].diff.each_line do |line|
+      if self.persisted? && (comment_line-2..comment_line+2).include?(ind) && data.try('[]', "line#{ind-comment_line}") != line.chomp
+        break res = false
+      end
+      ind = ind + 1
+    end
+    if ind < comment_line
+      return data[:actual] = false
+    else
+      return data[:actual] = res
+    end
+  end
+
+  def inline_diff(repo)
+    text = data[:strings]
+    Rails.logger.debug "Comment id is #{id}; text class is #{text.class.name}; text is #{text}"
+    closest = []
+    (-2..0).each {|shift| closest << data["line#{shift}"]}
+    text << closest.join("\n")
+  end
+
   protected
 
   def subscribe_on_reply

app/models/pull_request.rb

@@ -70,6 +70,7 @@ class PullRequest < ActiveRecord::Base
       else
         send(new_status)
       end
+      self.comments.each {|c| c.data[:actual]=nil; c.save} # maybe need add new column 'actual'?
     else
       self.status = new_status == 'block' ? 'blocked' : new_status
     end

app/views/projects/git/commits/_commit_diff.html.haml

@@ -6,4 +6,4 @@
     - if commit_diff.b_path.present?
       .r= link_to "view file @ #{short_hash_id(commit_id)}", blob_path(@project, commit_id, commit_diff.b_path)
     .clear
-  .diff_data= render_diff(commit_diff, commit_diff_counter) unless (@project.repo.tree(commit_id) / commit_diff.b_path).binary?
+  .diff_data= render_diff(commit_diff, commit_diff_counter, @comments) unless (@project.repo.tree(commit_id) / commit_diff.b_path).binary?

app/views/projects/pull_requests/_activity.html.haml

@@ -4,19 +4,8 @@
 
 - commits_queue = []
 - merge_activity(@comments, @commits).each do |item| #
-  - if item.is_a?(Comment)
-    - if commits_queue.present?
-      = render 'projects/git/commits/commits_small', :commits => commits_queue
-      - commits_queue = []
-    = render 'projects/comments/comment', :comment => item, :project => @project, :commentable => @issue, :add_id => nil
-  - else
-    - commits_queue << item
-    - comments_for_commit = Comment.for_commit(item)
-    - if comments_for_commit.present?
-      = render 'projects/git/commits/commits_small', :commits => commits_queue
-      - comments_for_commit.each do |comment|
-        = render 'projects/comments/comment', :comment => comment, :project => @project, :commentable => item, :add_id => nil
-      - commits_queue = []
-- if commits_queue.present?
-  = render 'projects/git/commits/commits_small', :commits => commits_queue
+  - if item.is_a? Grit::Commit
+    = render 'projects/git/commits/commits_small', :commits => [item]
+  - elsif item.is_a? Array
+    = render 'projects/pull_requests/discussion_comments', :item => item, :project => @project, :commentable => @issue, :add_id => nil
 

app/views/projects/pull_requests/_discussion_comments.html.haml

@@ -0,0 +1,13 @@
+-if item[0].blank?
+  -item[1].each do |comment|
+    = render 'projects/comments/comment', :comment => comment, :project => @project, :commentable => @issue, :add_id => nil
+-else
+  -comment = item[1].first
+  .file
+    %a{:name => "#"}
+    .top
+      .l=comment.data[:view_path]
+      .r=link_to "view full changes", '#'
+      .clear
+    .diff_data=render_diff(comment.inline_diff(@repo), 0, item[1], comment.data[:path])
+

app/views/projects/pull_requests/_pull_diff.html.haml

@@ -6,4 +6,4 @@
     - if pull_diff.b_path.present?
       .r= link_to "view file @ #{short_hash_id(commit_id)}", blob_path(@project, commit_id, pull_diff.b_path)
     .clear
-  .diff_data= render_diff(pull_diff, pull_diff_counter) unless (Grit::Repo.new(@pull.path).tree(commit_id) / pull_diff.b_path).binary?
+  .diff_data= render_diff(pull_diff, pull_diff_counter, @comments) unless (Grit::Repo.new(@pull.path).tree(commit_id) / pull_diff.b_path).binary?

app/views/projects/wiki/_diff_data.html.haml

@@ -1,4 +1,4 @@
 .blob_header
   .size= h(diff.deleted_file ? diff.a_path : diff.b_path)
   .clear
-.diff_data.highlight= render_diff(diff)
+.diff_data.highlight= render_diff(diff, diff_counter, [])