Merge branch 'dm-highlight-diff-file-blob' into 'master'

Use Diff::File blob methods from diff highlighter

See merge request !11978

app/controllers/projects/blob_controller.rb

@@ -93,9 +93,11 @@ class Projects::BlobController < Projects::ApplicationController
   def diff
     apply_diff_view_cookie!
 
+    @blob.load_all_data!
+    @lines = Gitlab::Highlight.highlight(@blob.path, @blob.data, repository: @repository).lines
+
     @form = UnfoldForm.new(params)
-    @lines = Gitlab::Highlight.highlight_lines(repository, @ref, @path)
-    @lines = @lines[@form.since - 1..@form.to - 1]
+    @lines = @lines[@form.since - 1..@form.to - 1].map(&:html_safe)
 
     if @form.bottom?
       @match_line = ''

lib/gitlab/diff/file.rb

@@ -58,19 +58,19 @@ module Gitlab
         diff_refs&.head_sha
       end
 
-      def content_sha
-        return old_content_sha if deleted_file?
-        return @content_sha if defined?(@content_sha)
+      def new_content_sha
+        return if deleted_file?
+        return @new_content_sha if defined?(@new_content_sha)
 
         refs = diff_refs || fallback_diff_refs
-        @content_sha = refs&.head_sha
+        @new_content_sha = refs&.head_sha
       end
 
-      def content_commit
-        return @content_commit if defined?(@content_commit)
+      def new_content_commit
+        return @new_content_commit if defined?(@new_content_commit)
 
-        sha = content_sha
-        @content_commit = repository.commit(sha) if sha
+        sha = new_content_commit
+        @new_content_commit = repository.commit(sha) if sha
       end
 
       def old_content_sha
@@ -88,13 +88,13 @@ module Gitlab
         @old_content_commit = repository.commit(sha) if sha
       end
 
-      def blob
-        return @blob if defined?(@blob)
+      def new_blob
+        return @new_blob if defined?(@new_blob)
 
-        sha = content_sha
-        return @blob = nil unless sha
+        sha = new_content_sha
+        return @new_blob = nil unless sha
 
-        repository.blob_at(sha, file_path)
+        @new_blob = repository.blob_at(sha, file_path)
       end
 
       def old_blob
@@ -106,6 +106,18 @@ module Gitlab
         @old_blob = repository.blob_at(sha, old_path)
       end
 
+      def content_sha
+        new_content_sha || old_content_sha
+      end
+
+      def content_commit
+        new_content_commit || old_content_commit
+      end
+
+      def blob
+        new_blob || old_blob
+      end
+
       attr_writer :highlighted_diff_lines
 
       # Array of Gitlab::Diff::Line objects

lib/gitlab/diff/highlight.rb

@@ -42,9 +42,9 @@ module Gitlab
 
         rich_line =
           if diff_line.unchanged? || diff_line.added?
-            new_lines[diff_line.new_pos - 1]
+            new_lines[diff_line.new_pos - 1]&.html_safe
           elsif diff_line.removed?
-            old_lines[diff_line.old_pos - 1]
+            old_lines[diff_line.old_pos - 1]&.html_safe
           end
 
         # Only update text if line is found. This will prevent
@@ -60,13 +60,18 @@ module Gitlab
       end
 
       def old_lines
-        return unless diff_file
-        @old_lines ||= Gitlab::Highlight.highlight_lines(self.repository, diff_old_sha, diff_old_path)
+        @old_lines ||= highlighted_blob_lines(diff_file.old_blob)
       end
 
       def new_lines
-        return unless diff_file
-        @new_lines ||= Gitlab::Highlight.highlight_lines(self.repository, diff_new_sha, diff_new_path)
+        @new_lines ||= highlighted_blob_lines(diff_file.new_blob)
+      end
+
+      def highlighted_blob_lines(blob)
+        return [] unless blob
+
+        blob.load_all_data!
+        Gitlab::Highlight.highlight(blob.path, blob.data, repository: repository).lines
       end
     end
   end

lib/gitlab/highlight.rb

@@ -5,14 +5,6 @@ module Gitlab
         highlight(blob_content, continue: false, plain: plain)
     end
 
-    def self.highlight_lines(repository, ref, file_name)
-      blob = repository.blob_at(ref, file_name)
-      return [] unless blob
-
-      blob.load_all_data!
-      highlight(file_name, blob.data, repository: repository).lines.map!(&:html_safe)
-    end
-
     attr_reader :blob_name
 
     def initialize(blob_name, blob_content, repository: nil)

spec/lib/gitlab/highlight_spec.rb

@@ -7,30 +7,6 @@ describe Gitlab::Highlight, lib: true do
   let(:repository) { project.repository }
   let(:commit) { project.commit(sample_commit.id) }
 
-  describe '.highlight_lines' do
-    let(:lines) do
-      Gitlab::Highlight.highlight_lines(project.repository, commit.id, 'files/ruby/popen.rb')
-    end
-
-    it 'highlights all the lines properly' do
-      expect(lines[4]).to eq(%Q{<span id="LC5" class="line" lang="ruby">  <span class="kp">extend</span> <span class="nb">self</span></span>\n})
-      expect(lines[21]).to eq(%Q{<span id="LC22" class="line" lang="ruby">    <span class="k">unless</span> <span class="no">File</span><span class="p">.</span><span class="nf">directory?</span><span class="p">(</span><span class="n">path</span><span class="p">)</span></span>\n})
-      expect(lines[26]).to eq(%Q{<span id="LC27" class="line" lang="ruby">    <span class="vi">@cmd_status</span> <span class="o">=</span> <span class="mi">0</span></span>\n})
-    end
-
-    describe 'with CRLF' do
-      let(:branch) { 'crlf-diff' }
-      let(:blob) { repository.blob_at_branch(branch, path) }
-      let(:lines) do
-        Gitlab::Highlight.highlight_lines(project.repository, 'crlf-diff', 'files/whitespace')
-      end
-
-      it 'strips extra LFs' do
-        expect(lines[0]).to eq("<span id=\"LC1\" class=\"line\" lang=\"plaintext\">test  </span>")
-      end
-    end
-  end
-
   describe 'custom highlighting from .gitattributes' do
     let(:branch) { 'gitattributes' }
     let(:blob) { repository.blob_at_branch(branch, path) }
@@ -59,6 +35,19 @@ describe Gitlab::Highlight, lib: true do
   end
 
   describe '#highlight' do
+    describe 'with CRLF' do
+      let(:branch) { 'crlf-diff' }
+      let(:path) { 'files/whitespace' }
+      let(:blob) { repository.blob_at_branch(branch, path) }
+      let(:lines) do
+        Gitlab::Highlight.highlight(blob.path, blob.data, repository: repository).lines
+      end
+
+      it 'strips extra LFs' do
+        expect(lines[0]).to eq("<span id=\"LC1\" class=\"line\" lang=\"plaintext\">test  </span>")
+      end
+    end
+
     it 'links dependencies via DependencyLinker' do
       expect(Gitlab::DependencyLinker).to receive(:link).
         with('file.name', 'Contents', anything).and_call_original