Merge branch 'highlight-blame' into 'master'

Fix highlighting in blame view.

See merge request !2630

6 files changed, 84 insertions, 43 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 145d4c03731..322765c6d08 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -29,6 +29,7 @@ v 8.4.2
     track them in Performance Monitoring.
   - Increase contrast between highlighted code comments and inline diff marker
   - Fix method undefined when using external commit status in builds
+  - Fix highlighting in blame view.
 
 v 8.4.1
   - Apply security updates for Rails (4.2.5.1), rails-html-sanitizer (1.0.3),
diff --git a/app/controllers/projects/blame_controller.rb b/app/controllers/projects/blame_controller.rb
index 9ea518e6c85..f576d0be1fc 100644
--- a/app/controllers/projects/blame_controller.rb
+++ b/app/controllers/projects/blame_controller.rb
@@ -8,28 +8,6 @@ class Projects::BlameController < Projects::ApplicationController
 
   def show
     @blob = @repository.blob_at(@commit.id, @path)
-    @blame = group_blame_lines
-  end
-
-  def group_blame_lines
-    blame = Gitlab::Git::Blame.new(@repository, @commit.id, @path)
-
-    prev_sha = nil
-    groups = []
-    current_group = nil
-
-    blame.each do |commit, line|
-      if prev_sha && prev_sha == commit.sha
-        current_group[:lines] << line
-      else
-        groups << current_group if current_group.present?
-        current_group = { commit: commit, lines: [line] }
-      end
-
-      prev_sha = commit.sha
-    end
-
-    groups << current_group if current_group.present?
-    groups
+    @blame_groups = Gitlab::Blame.new(@blob, @commit).groups
   end
 end
diff --git a/app/views/projects/blame/show.html.haml b/app/views/projects/blame/show.html.haml
index 53dcac78a9f..eb6fbfaffa0 100644
--- a/app/views/projects/blame/show.html.haml
+++ b/app/views/projects/blame/show.html.haml
@@ -15,12 +15,11 @@
     .file-content.blame.code.js-syntax-highlight
       %table
         - current_line = 1
-        - blame_highlighter = highlighter(@blob.name, @blob.data, nowrap: true)
-        - @blame.each do |blame_group|
+        - @blame_groups.each do |blame_group|
           %tr
             %td.blame-commit
               .commit
-                - commit = Commit.new(blame_group[:commit], @project)
+                - commit = blame_group[:commit]
                 .commit-row-title
                   %strong
                     = link_to_gfm truncate(commit.title, length: 35), namespace_project_commit_path(@project.namespace, @project, commit.id), class: "cdark"
@@ -38,8 +37,7 @@
                 \
               - current_line += line_count
             %td.lines
-              %pre{class: 'code highlight'}
+              %pre.code.highlight
                 %code
                   - blame_group[:lines].each do |line|
-                    :preserve
-                      #{blame_highlighter.highlight(line)}
+                    #{line}
diff --git a/lib/gitlab/blame.rb b/lib/gitlab/blame.rb
new file mode 100644
index 00000000000..313e6b9fc03
--- /dev/null
+++ b/lib/gitlab/blame.rb
@@ -0,0 +1,54 @@
+module Gitlab
+  class Blame
+    attr_accessor :blob, :commit
+
+    def initialize(blob, commit)
+      @blob = blob
+      @commit = commit
+    end
+
+    def groups(highlight: true)
+      prev_sha = nil
+      groups = []
+      current_group = nil
+
+      i = 0
+      blame.each do |commit, line|
+        commit = Commit.new(commit, project)
+
+        sha = commit.sha
+        if prev_sha != sha
+          groups << current_group if current_group
+          current_group = { commit: commit, lines: [] }
+        end
+
+        line = highlighted_lines[i].html_safe if highlight
+        current_group[:lines] << line
+
+        prev_sha = sha
+        i += 1
+      end
+      groups << current_group if current_group
+
+      groups
+    end
+
+    private
+
+    def blame
+      @blame ||= Gitlab::Git::Blame.new(repository, @commit.id, @blob.path)
+    end
+
+    def highlighted_lines
+      @highlighted_lines ||= Gitlab::Highlight.highlight(@blob.name, @blob.data).lines
+    end
+
+    def project
+      commit.project
+    end
+
+    def repository
+      project.repository
+    end
+  end
+end
diff --git a/spec/controllers/blame_controller_spec.rb b/spec/controllers/blame_controller_spec.rb
index 3ad4d5fc0a8..25f06299a29 100644
--- a/spec/controllers/blame_controller_spec.rb
+++ b/spec/controllers/blame_controller_spec.rb
@@ -24,20 +24,6 @@ describe Projects::BlameController do
     context "valid file" do
       let(:id) { 'master/files/ruby/popen.rb' }
       it { is_expected.to respond_with(:success) }
-
-      it 'groups blames properly' do
-        blame = assigns(:blame)
-        # Sanity check a few items
-        expect(blame.count).to eq(18)
-        expect(blame[0][:commit].sha).to eq('913c66a37b4a45b9769037c55c2d238bd0942d2e')
-        expect(blame[0][:lines]).to eq(["require 'fileutils'", "require 'open3'", ""])
-
-        expect(blame[1][:commit].sha).to eq('874797c3a73b60d2187ed6e2fcabd289ff75171e')
-        expect(blame[1][:lines]).to eq(["module Popen", "  extend self"])
-
-        expect(blame[-1][:commit].sha).to eq('913c66a37b4a45b9769037c55c2d238bd0942d2e')
-        expect(blame[-1][:lines]).to eq(["  end", "end"])
-      end
     end
   end
 end
diff --git a/spec/lib/gitlab/blame_spec.rb b/spec/lib/gitlab/blame_spec.rb
new file mode 100644
index 00000000000..89245761b6f
--- /dev/null
+++ b/spec/lib/gitlab/blame_spec.rb
@@ -0,0 +1,24 @@
+require 'spec_helper'
+
+describe Gitlab::Blame, lib: true do
+  let(:project) { create(:project) }
+  let(:path) { 'files/ruby/popen.rb' }
+  let(:commit) { project.commit('master') }
+  let(:blob) { project.repository.blob_at(commit.id, path) }
+
+  describe "#groups" do
+    let(:subject) { described_class.new(blob, commit).groups(highlight: false) }
+
+    it 'groups lines properly' do
+      expect(subject.count).to eq(18)
+      expect(subject[0][:commit].sha).to eq('913c66a37b4a45b9769037c55c2d238bd0942d2e')
+      expect(subject[0][:lines]).to eq(["require 'fileutils'", "require 'open3'", ""])
+
+      expect(subject[1][:commit].sha).to eq('874797c3a73b60d2187ed6e2fcabd289ff75171e')
+      expect(subject[1][:lines]).to eq(["module Popen", "  extend self"])
+
+      expect(subject[-1][:commit].sha).to eq('913c66a37b4a45b9769037c55c2d238bd0942d2e')
+      expect(subject[-1][:lines]).to eq(["  end", "end"])
+    end
+  end
+end