[refs #90] small refactoring of the diff method

app/controllers/projects/pull_requests_controller.rb

@@ -128,17 +128,12 @@ class Projects::PullRequestsController < Projects::BaseController
   end
 
   def load_diff_commits_data
-    @base_commit = @pull.common_ancestor
-    @head_commit = @pull.repo.commits(@pull.from_branch).first
-
-    @commits = @pull.repo.commits_between(@pull.repo.commits(@pull.to_ref).first, @head_commit)
+    @commits = @pull.repo.commits_between(@pull.from_commit, @pull.to_commit)
     @total_commits = @commits.count
     @commits = @commits.last(100)
 
-    @diff = @pull.diff @pull.repo, @base_commit, @head_commit
-    @stats = @pull.diff_stats @pull.repo, @base_commit, @head_commit
-    @comments = @issue.comments
-    @commentable = @issue
+    @diff, @stats = @pull.diff, @pull.diff_stats
+    @comments, @commentable = @issue.comments, @issue
   end
 
   def find_destination_project bang=true

app/models/comment.rb

@@ -95,10 +95,8 @@ class Comment < ActiveRecord::Base
     end
     self.data = {:path => params[:path], :line => params[:line]}
     if commentable.class == Issue && pull = commentable.pull_request
-      to_commit, from_commit = pull.common_ancestor, pull.repo.commits(pull.head_branch).first
-      diff = pull.diff pull.repo, to_commit, from_commit
-      diff_path = diff.select {|d| d.a_path == params[:path]}
-      return false unless actual_inline_comment?(diff, true)
+      diff_path = pull.diff.select {|d| d.a_path == params[:path]}
+      return false unless actual_inline_comment?(pull.diff, true)
 
       comment_line, line_number, strings = params[:line].to_i, -1, []
       diff_path[0].diff.each_line do |line|

app/models/pull_request.rb

@@ -97,14 +97,14 @@ class PullRequest < ActiveRecord::Base
   def common_ancestor
     return @common_ancestor if @common_ancestor
     base_commit = repo.commits(to_ref).first
-    head_commit = repo.commits(from_branch).first
-    @common_ancestor = repo.commit(repo.git.merge_base({}, base_commit, head_commit)) || base_commit
+    @common_ancestor = repo.commit(repo.git.merge_base({}, base_commit, from_commit)) || base_commit
   end
+  alias_method :to_commit, :common_ancestor
 
-  def diff_stats(repo, a,b)
+  def diff_stats
     stats = []
     Dir.chdir(path) do
-      lines = repo.git.native(:diff, {:numstat => true, :M => true}, "#{a.id}...#{b.id}").split("\n")
+      lines = repo.git.native(:diff, {:numstat => true, :M => true}, "#{to_commit.id}...#{from_commit.id}").split("\n")
       while !lines.empty?
         files = []
         while lines.first =~ /^([-\d]+)\s+([-\d]+)\s+(.+)/
@@ -119,8 +119,8 @@ class PullRequest < ActiveRecord::Base
   end
 
   # FIXME maybe move to warpc/grit?
-  def diff(repo, a, b)
-    diff = repo.git.native('diff', {:M => true}, "#{a}...#{b}")
+  def diff
+    diff = repo.git.native('diff', {:M => true}, "#{to_commit.id}...#{from_commit.id}")
 
     if diff =~ /diff --git a/
       diff = diff.sub(/.*?(diff --git a)/m, '\1')
@@ -151,6 +151,10 @@ class PullRequest < ActiveRecord::Base
     @repo = Grit::Repo.new path
   end
 
+  def from_commit
+    repo.commits(from_branch).first
+  end
+
   protected
 
   def merge