14 files changed, 75 insertions, 8 deletions

diff --git a/app/controllers/merge_request_versions_controller.rb b/app/controllers/merge_request_versions_controller.rb
index bd38c2d..5264228 100644
--- a/app/controllers/merge_request_versions_controller.rb
+++ b/app/controllers/merge_request_versions_controller.rb
@@ -27,6 +27,8 @@ class MergeRequestVersionsController < ApplicationController
       @commit = @repository.git.commit(params[:commit_shas])
     end
 
+    @comments = @version.comments_for_sha(params[:commit_shas], :include_merge_request_comments => true).sort_by(&:created_at)
+
     respond_to {|wants|
       wants.js {render :layout => false}
     }
diff --git a/app/helpers/comments_helper.rb b/app/helpers/comments_helper.rb
index 4f3003a..d500f88 100644
--- a/app/helpers/comments_helper.rb
+++ b/app/helpers/comments_helper.rb
@@ -20,5 +20,10 @@
 #++
 
 module CommentsHelper
-
+  def comment_block(comment, &block)
+    css_classes = ["comment"]
+    css_classes << "inline" if comment.applies_to_line_numbers?
+    output = content_tag(:div, capture(&block), :class => css_classes.join(" "))
+    concat(output)
+  end
 end
diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb
index 58056ac..9245335 100644
--- a/app/helpers/commits_helper.rb
+++ b/app/helpers/commits_helper.rb
@@ -21,6 +21,7 @@
 
 module CommitsHelper
   include RepositoriesHelper
+  include CommentsHelper
   
   def format_commit_message(message)
     message.gsub(/\b[a-z0-9]{40}\b/) do |match|
diff --git a/app/helpers/merge_request_versions_helper.rb b/app/helpers/merge_request_versions_helper.rb
index 5683704..a2b447c 100644
--- a/app/helpers/merge_request_versions_helper.rb
+++ b/app/helpers/merge_request_versions_helper.rb
@@ -1,3 +1,4 @@
 module MergeRequestVersionsHelper
   include CommitsHelper
+  include CommentsHelper
 end
diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb
index cd6f333..497823f 100644
--- a/app/helpers/merge_requests_helper.rb
+++ b/app/helpers/merge_requests_helper.rb
@@ -20,6 +20,7 @@
 
 module MergeRequestsHelper
   include MergeRequestVersionsHelper
+  include CommentsHelper
   def render_status_tag_list(status_tags, repository)
     project_statuses = repository.project.merge_request_statuses
     
diff --git a/app/models/comment.rb b/app/models/comment.rb
index ae423d0..84faee7 100644
--- a/app/models/comment.rb
+++ b/app/models/comment.rb
@@ -102,6 +102,10 @@ class Comment < ActiveRecord::Base
     first, last = sha1.split("-")
     first..(last||first)
   end
+
+  def applies_to_line_numbers?
+    return MergeRequestVersion === target
+  end
   
   protected
     def notify_target_if_supported
diff --git a/app/models/merge_request_version.rb b/app/models/merge_request_version.rb
index f034b79..5f185f3 100644
--- a/app/models/merge_request_version.rb
+++ b/app/models/merge_request_version.rb
@@ -43,10 +43,15 @@ class MergeRequestVersion < ActiveRecord::Base
   end
 
   def comments_for_path_and_sha(path, sha)
-    if Range === sha
-      sha = [sha.begin, sha.end].join("-")
+    comments.select{|c|(c.path == path && c.sha1 == sha_range_string(sha))}
+  end
+
+  def comments_for_sha(sha, options={})
+    result = comments.select{|c|c.sha1 == sha_range_string(sha)}
+    if options[:include_merge_request_comments]
+      result.concat(merge_request.comments)
     end
-    comments.select{|c|(c.path == path && c.sha1 == sha)}
+    result
   end
 
   def short_merge_base
@@ -104,4 +109,13 @@ class MergeRequestVersion < ActiveRecord::Base
       end
     end
   end
+
+  private
+  # Returns a string representation of a sha range
+  def sha_range_string(string_or_range)
+    if Range === string_or_range
+      string_or_range = [string_or_range.begin, string_or_range.end].join("-")
+    end
+    string_or_range
+  end
 end
diff --git a/app/views/comments/_comment.html.erb b/app/views/comments/_comment.html.erb
index 4e62149..e89770c 100644
--- a/app/views/comments/_comment.html.erb
+++ b/app/views/comments/_comment.html.erb
@@ -21,7 +21,7 @@
 %>
 
 <a name="<%= dom_id(comment) -%>"></a>
-<div class="comment">
+<% comment_block(comment) do -%>
   <%- if comment.state_change -%>
   <p class="meta_body">
     &#x2192; State changed <%- if comment.state_changed_from -%>from 
@@ -37,4 +37,4 @@
     <%= link_to(h(comment.user.title), comment.user) -%> | 
     <span class="permalink"><%= link_to time_ago_in_words(comment.created_at) + " ago", "##{dom_id(comment)}" -%></span>
   </p>
-</div>
+<% end -%>
diff --git a/app/views/merge_request_versions/_merge_request_version.html.erb b/app/views/merge_request_versions/_merge_request_version.html.erb
index eff7211..a7b0542 100644
--- a/app/views/merge_request_versions/_merge_request_version.html.erb
+++ b/app/views/merge_request_versions/_merge_request_version.html.erb
@@ -16,7 +16,9 @@
 #   along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #++
 %>
-
+<div id="__temp_comments">
+<%= render :partial => @comments -%>
+</div>
 <% if @commit -%>
 <div class="commit-infobox">
   <div class="commit-meta">
diff --git a/app/views/merge_requests/show.html.erb b/app/views/merge_requests/show.html.erb
index b0995ab..cf15f4e 100644
--- a/app/views/merge_requests/show.html.erb
+++ b/app/views/merge_requests/show.html.erb
@@ -74,7 +74,7 @@
 </div>
 
 <h2 id="comments">Comments</h2>
-<div class="comments">
+<div class="comments" id="merge_request_comments">
   <%= render :partial => @merge_request.comments -%>
 </div>
 
diff --git a/public/javascripts/application.js b/public/javascripts/application.js
index 653cc59..0d39592 100644
--- a/public/javascripts/application.js
+++ b/public/javascripts/application.js
@@ -629,6 +629,9 @@ Gitorious.DiffBrowser = function(shas)
     "success": function(data, responseText) {
       if (responseText === "success") {
         jQuery("#merge_request_diff").html(data);
+        var commentMarkup = jQuery("#__temp_comments").html();
+        jQuery("#__temp_comments").html("");
+        jQuery("#merge_request_comments").html(commentMarkup);
         var shaSpec = new Gitorious.ShaSpec();
         shaSpec.parseShas(shas);
         Gitorious.MergeRequestController.getInstance().didReceiveVersion(shaSpec);
diff --git a/public/stylesheets/base.css b/public/stylesheets/base.css
index cf84eea..e31ffba 100644
--- a/public/stylesheets/base.css
+++ b/public/stylesheets/base.css
@@ -1689,6 +1689,12 @@ ul.diff_stats small.deletions   { color: #DC0000; }
   margin-bottom: 10px;
 }
 
+.comments .comment.inline {
+/*
+TODO: Add different styling of comments that apply to specific line numbers
+*/
+}
+
 .comments .comment img {
   max-width: 500px; max-height: 500px;
 }
diff --git a/test/unit/comment_test.rb b/test/unit/comment_test.rb
index c357c60..9a8afd2 100644
--- a/test/unit/comment_test.rb
+++ b/test/unit/comment_test.rb
@@ -52,6 +52,14 @@ class CommentTest < ActiveSupport::TestCase
       end
     end
   end
+
+  context "In general" do
+    setup {@comment = Comment.new}
+
+    should "not apply to specific line numbers" do
+      assert !@comment.applies_to_line_numbers?
+    end
+  end
   
   context 'State change' do
     should 'be a list of previous and new state' do
@@ -118,6 +126,7 @@ class CommentTest < ActiveSupport::TestCase
       assert_nil comment.reload.state_changed_from
       assert_nil comment.state_changed_to
     end
+
   end
 
   context 'On merge request versions' do
@@ -135,6 +144,10 @@ class CommentTest < ActiveSupport::TestCase
       assert_equal @first_version, @comment.target
     end
 
+    should "know if it has line numbers" do
+      assert @comment.applies_to_line_numbers?
+    end
+
     should "have a range of shas" do
       assert_equal(("ffac".."aafc"), @comment.sha_range)
       @comment.sha1 = "ffac"
diff --git a/test/unit/merge_request_version_test.rb b/test/unit/merge_request_version_test.rb
index 2b8aa56..d96c742 100644
--- a/test/unit/merge_request_version_test.rb
+++ b/test/unit/merge_request_version_test.rb
@@ -138,6 +138,21 @@ class MergeRequestVersionTest < ActiveSupport::TestCase
       assert_equal([@comment], @first_version.comments_for_path_and_sha(@comment.path, "ffac-aafc"))
     end
 
+    should 'fetch all comments with the specified sha' do
+      assert_equal([@comment], @first_version.comments_for_sha("ffac-aafc"))
+    end
+
+    should 'combine version and MR comments into a single array' do
+      @mr_comment = @merge_request.comments.create!(
+        :body => "Beware high gamma levels",
+        :user => users(:moe),
+        :project => @merge_request.target_repository.project
+        )
+      assert_equal([@comment, @mr_comment], @first_version.comments_for_sha("ffac-aafc",
+          :include_merge_request_comments => true))
+    end
+
+
     should 'fetch all comments when given a Range' do
       assert_equal([@comment], @first_version.comments_for_path_and_sha(@comment.path, ("ffac".."aafc")))
     end