[refs #674] first step of the diff between commits

app/controllers/projects/git/base_controller.rb

@@ -1,7 +1,7 @@
 # -*- encoding : utf-8 -*-
 class Projects::Git::BaseController < Projects::BaseController
   before_filter :authenticate_user!
-  skip_before_filter :authenticate_user!, :only => [:show, :index, :blame, :raw, :archive] if APP_CONFIG['anonymous_access']
+  skip_before_filter :authenticate_user!, :only => [:show, :index, :blame, :raw, :archive, :diff] if APP_CONFIG['anonymous_access']
   load_and_authorize_resource :project
 
   before_filter :set_treeish_and_path

app/controllers/projects/git/commits_controller.rb

@@ -18,4 +18,30 @@ class Projects::Git::CommitsController < Projects::Git::BaseController
       format.patch { render :text => (@commit.to_patch rescue ''), :content_type => "text/plain" }
     end
   end
+
+  def diff
+    if params[:commit2].present?
+      params1 = params[:commit1]
+      params2 = params[:commit2] == 'HEAD' ? @project.default_branch : params[:commit2]
+    else # get only one parameter
+      params1 = @project.default_branch
+      params2 = params[:commit1]
+    end
+    params1.sub! 'HEAD', @project.default_branch
+    params2.sub! 'HEAD', @project.default_branch
+
+    ref1 = if @project.repo.branches_and_tags.include? params1
+             @project.repo.commits(params1).first
+           else
+             params1 #FIXME git has other ref?
+           end
+    @commit1 = @project.repo.commit(ref1) || raise(ActiveRecord::RecordNotFound)
+
+    ref = if @project.repo.branches_and_tags.include? params2
+            @project.repo.commits(params2).first
+          else
+            params2 #FIXME git has other ref?
+          end
+    @commit = @project.repo.commit(ref) || raise(ActiveRecord::RecordNotFound)
+  end
 end

app/helpers/diff_helper.rb

@@ -1,7 +1,7 @@
 # -*- encoding : utf-8 -*-
 module DiffHelper
   def render_diff_stats(stats)
-    path = @pull.id ? polymorphic_path([@project, @pull]) : ''
+    path = @pull.try(:id) ? polymorphic_path([@project, @pull]) : ''
     res = ["<table class='commit_stats'>"]
     stats.each_with_index do |stat, ind|
       res << "<tr>"
@@ -51,12 +51,14 @@ module DiffHelper
   ########################################################
   def prepare(args)
     @url, @diff_counter, @in_discussion = args[:url], args[:diff_counter], args[:in_discussion]
-    @filepath, @line_comments, @in_wiki = args[:filepath], args[:comments], args[:in_wiki]
+    @filepath, @line_comments = args[:filepath], args[:comments]
     @add_reply_id, @num_line = if @in_discussion
         [@line_comments[0].id, @line_comments[0].data[:line].to_i - @line_comments[0].data[:strings].lines.count.to_i-1]
       else
         [nil, -1]
       end
+
+    @no_commit_comment = true if params[:controller] == 'projects/wiki' || (params[:action] == 'diff')
   end
 
   def headerline(line)
@@ -225,12 +227,12 @@ module DiffHelper
   end
 
   def line_comment
-    return if @in_wiki || (@in_discussion && @add_reply_id && @line_comments[0].data[:line].to_i != @num_line)
+    return if @no_commit_comment || (@in_discussion && @add_reply_id && @line_comments[0].data[:line].to_i != @num_line)
     link_to image_tag('line_comment.png', :alt => t('layout.comments.new_header')), new_comment_path, :class => 'add_line-comment'
   end
 
   def render_line_comments
-    unless @in_wiki || @in_discussion
+    unless @no_commit_comment || @in_discussion
       comments = @line_comments.select do |c|
         c.data.try('[]', :line) == @num_line.to_s && c.actual_inline_comment?
       end
@@ -243,7 +245,7 @@ module DiffHelper
   end
 
   def tr_line_comments comments
-    return if @in_wiki
+    return if @no_commit_comment
     res="<tr class='inline-comments'>
       <td class='line_numbers' colspan='2'>#{comments.count}</td>
       <td>"

app/models/pull_request.rb

@@ -104,33 +104,12 @@ class PullRequest < ActiveRecord::Base
   alias_method :to_commit, :common_ancestor
 
   def diff_stats
-    stats = []
-    Dir.chdir(path) do
-      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+(.+)/
-          additions, deletions, filename = lines.shift.gsub(' => ', '=>').split
-          additions, deletions = additions.to_i, deletions.to_i
-          stat = Grit::DiffStat.new filename, additions, deletions
-          stats << stat
-        end
-      end
-      stats
-    end
+    @diff_stats ||= repo.diff_stats(to_commit.id, from_commit.id)
   end
 
   # FIXME maybe move to warpc/grit?
   def diff
-    return @diff if @diff.present?
-    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')
-    else
-      diff = ''
-    end
-    @diff = Grit::Diff.list_from_string(repo, diff)
+    @diff ||= repo.diff(to_commit.id, from_commit.id)
   end
 
   def set_user_and_time user

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

@@ -0,0 +1,6 @@
+-begin
+  =render_diff_stats(@project.repo.diff_stats @commit1.id, @commit.id)
+  = "#{@commit1}...#{@commit}"
+  =render :partial => 'commit_diff', :collection => @project.repo.diff(@commit1.id, @commit.id)
+- rescue Grit::Git::GitTimeout
+  %p= t 'layout.git.repositories.commit_diff_too_big'

config/routes.rb

@@ -304,6 +304,8 @@ Rosa::Application.routes.draw do
           get '/raw/:treeish/*path' => "git/blobs#raw", :as => :raw, :format => false
           # Archive
           get '/archive/:treeish.:format' => "git/trees#archive", :as => :archive, :format => /zip|tar\.gz/
+          # Git diff
+          get '/diff/:commit1(...:commit2)' => "git/commits#diff", :as => :diff
         end
       end
     end

lib/ext/git/grit.rb

@@ -91,6 +91,41 @@ module Grit
     def branches_and_tags
       branches + tags # @branches_and_tags ||= # ???
     end
+
+    def diff(a, b, *paths)
+      diff = self.git.native('diff', {:M => true}, "#{a}...#{b}", '--', *paths)
+
+      if diff =~ /diff --git a/
+        diff = diff.sub(/.*?(diff --git a)/m, '\1')
+      else
+        diff = ''
+      end
+      Diff.list_from_string(self, diff)
+    end
+
+    # The diff stats for the given treeish
+    #   git diff --numstat -M a...b
+    #
+    #   +a+ is the base treeish
+    #   +b+ is the head treeish
+    #
+    # Returns Grit::DiffStat[]
+    def diff_stats(a,b)
+      stats = []
+      Dir.chdir(path) do
+        lines = self.git.native(:diff, {:numstat => true, :M => true}, "#{a}...#{b}").split("\n")
+        while !lines.empty?
+          files = []
+          while lines.first =~ /^([-\d]+)\s+([-\d]+)\s+(.+)/
+            additions, deletions, filename = lines.shift.gsub(' => ', '=>').split
+            additions, deletions = additions.to_i, deletions.to_i
+            stat = DiffStat.new filename, additions, deletions
+            stats << stat
+          end
+        end
+        stats
+      end
+    end
   end
 end