Change strategy to highlight diffs. #3945

Now we apply syntax highlighting to the whole old and new files.
This basically help us to highlight adequately multiline content.

app/controllers/application_controller.rb

@@ -24,6 +24,7 @@ class ApplicationController < ActionController::Base
   helper_method :abilities, :can?, :current_application_settings
   helper_method :import_sources_enabled?, :github_import_enabled?, :github_import_configured?, :gitlab_import_enabled?, :gitlab_import_configured?, :bitbucket_import_enabled?, :bitbucket_import_configured?, :gitorious_import_enabled?, :google_code_import_enabled?, :fogbugz_import_enabled?, :git_import_enabled?
+  helper_method :repository
   rescue_from Encoding::CompatibilityError do |exception|
     log_exception(exception)

app/controllers/projects/blob_controller.rb

@@ -65,8 +65,11 @@ class Projects::BlobController < Projects::ApplicationController
   end
   def diff
-    @form = UnfoldForm.new(params)
-    @lines = Gitlab::Diff::Highlight.process_diff_lines(@blob.name, @blob.data.lines[@form.since - 1..@form.to - 1])
+    ref, file_name = params[:id].split('/', 2)
+
+    @form  = UnfoldForm.new(params)
+    @lines = Gitlab::Diff::Highlight.process_file(repository, ref, file_name)
+    @lines = @lines[@form.since - 1..@form.to - 1]
     if @form.bottom?
       @match_line = ''

app/controllers/projects/commit_controller.rb

@@ -72,6 +72,7 @@ class Projects::CommitController < Projects::ApplicationController
       @diffs = commit.diffs
     end
+    @diff_refs = [commit.parent.id, commit.id]
     @notes_count = commit.notes.count
     @statuses = ci_commit.statuses if ci_commit

app/controllers/projects/compare_controller.rb

@@ -20,6 +20,7 @@ class Projects::CompareController < Projects::ApplicationController
     if compare_result
       @commits = Commit.decorate(compare_result.commits, @project)
       @diffs = compare_result.diffs
+      @diff_refs = [base_ref, head_ref]
       @commit = @project.commit(head_ref)
       @first_commit = @project.commit(base_ref)
       @line_notes = []

app/controllers/projects/merge_requests_controller.rb

@@ -59,6 +59,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   def diffs
     @commit = @merge_request.last_commit
     @first_commit = @merge_request.first_commit
+    @diff_refs = [@merge_request.target_sha, @merge_request.source_sha]
     @comments_allowed = @reply_allowed = true
     @comments_target = {
@@ -103,6 +104,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     @commit = @merge_request.last_commit
     @first_commit = @merge_request.first_commit
     @diffs = @merge_request.compare_diffs
+    @diff_refs = [@merge_request.target_sha, @merge_request.source_branch]
     @ci_commit = @merge_request.ci_commit
     @statuses = @ci_commit.statuses if @ci_commit

app/helpers/diff_helper.rb

@@ -19,13 +19,13 @@ module DiffHelper
     end
   end
-  def safe_diff_files(diffs)
+  def safe_diff_files(diffs, diff_refs, repository)
     lines = 0
     safe_files = []
     diffs.first(allowed_diff_size).each do |diff|
       lines += diff.diff.lines.count
       break if lines > allowed_diff_lines
-      safe_files << Gitlab::Diff::File.new(diff)
+      safe_files << Gitlab::Diff::File.new(diff, diff_refs, repository)
     end
     safe_files
   end

app/views/projects/commit/show.html.haml

@@ -5,5 +5,6 @@
   = render "ci_menu"
 - else
   %div.block-connector
-= render "projects/diffs/diffs", diffs: @diffs, project: @project
+= render "projects/diffs/diffs", diffs: @diffs, project: @project,
+         diff_refs: @diff_refs
 = render "projects/notes/notes_with_form"

app/views/projects/compare/show.html.haml

@@ -9,7 +9,7 @@
 - if @commits.present?
   .prepend-top-default
     = render "projects/commits/commit_list"
-    = render "projects/diffs/diffs", diffs: @diffs, project: @project
+    = render "projects/diffs/diffs", diffs: @diffs, project: @project, diff_refs: @diff_refs
 - else
   .light-well.prepend-top-default
     .center

app/views/projects/diffs/_diffs.html.haml

 - if diff_view == 'parallel'
   - fluid_layout true
-- diff_files = safe_diff_files(diffs)
+- diff_files = safe_diff_files(diffs, diff_refs, repository)
 .gray-content-block.middle-block.oneline-block
   .inline-parallel-buttons

app/views/projects/merge_requests/_new_submit.html.haml

@@ -38,7 +38,7 @@
       = render "projects/merge_requests/show/commits"
     #diffs.diffs.tab-pane.active
       - if @diffs.present?
-        = render "projects/diffs/diffs", diffs: @diffs, project: @project
+        = render "projects/diffs/diffs", diffs: @diffs, project: @project, diff_refs: @diff_refs
       - elsif @commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE
         .alert.alert-danger
           %h4 This comparison includes more than #{MergeRequestDiff::COMMITS_SAFE_SIZE} commits.

app/views/projects/merge_requests/show/_diffs.html.haml

 - if @merge_request_diff.collected?
-  = render "projects/diffs/diffs", diffs: params[:w] == '1' ? @merge_request.diffs_no_whitespace : @merge_request.diffs, project: @merge_request.project
+  = render "projects/diffs/diffs", diffs: params[:w] == '1' ? @merge_request.diffs_no_whitespace : @merge_request.diffs, project: @merge_request.project, diff_refs: @diff_refs
 - elsif @merge_request_diff.empty?
   .nothing-here-block Nothing to merge from #{@merge_request.source_branch} into #{@merge_request.target_branch}
 - else

lib/gitlab/diff/file.rb

 module Gitlab
   module Diff
     class File
-      attr_reader :diff
+      attr_reader :diff, :repository, :new_ref, :old_ref
       delegate :new_file, :deleted_file, :renamed_file,
         :old_path, :new_path, to: :diff, prefix: false
-      def initialize(diff)
+      def initialize(diff, diff_refs, repository)
         @diff = diff
+        @repository = repository
+        @old_ref, @new_ref = diff_refs
       end
       # Array of Gitlab::DIff::Line objects
@@ -16,7 +18,7 @@ module Gitlab
       end
       def highlighted_diff_lines
-        Gitlab::Diff::Highlight.process_diff_lines(new_path, diff_lines)
+        Gitlab::Diff::Highlight.process_diff_lines(self)
       end
       def mode_changed?

lib/gitlab/diff/highlight.rb

 module Gitlab
   module Diff
     class Highlight
+      attr_reader :diff_file
+
+      delegate :repository, :old_path, :new_path, :old_ref, :new_ref,
+        to: :diff_file, prefix: :diff
+
       # Apply syntax highlight to provided source code
       #
-      # file_name - The file name related to the code.
-      # lines     - It can be an Array of Gitlab::Diff::Line objects or simple Strings.
-      #             When passing Strings you need to provide the required 'end of lines'
-      #             chars ("\n") for each String given that we don't append them automatically.
+      # diff_file - an instance of Gitlab::Diff::File
       #
       # Returns an Array with the processed items.
-      def self.process_diff_lines(file_name, lines)
-        processor = new(file_name, lines)
+      def self.process_diff_lines(diff_file)
+        processor = new(diff_file)
         processor.highlight
       end
-      def initialize(file_name, lines)
-        @file_name  = file_name
-        @lines      = lines
+      def self.process_file(repository, ref, file_name)
+        blob = repository.blob_at(ref, file_name)
+        return [] unless blob
+
+        content = blob.data
+        lexer = Rouge::Lexer.guess(filename: file_name, source: content).new rescue Rouge::Lexers::PlainText.new
+        formatter.format(lexer.lex(content)).lines
+      end
+
+      def self.formatter
+        @formatter ||= Rouge::Formatters::HTMLGitlab.new(
+                         nowrap: true,
+                         cssclass: 'code highlight',
+                         lineanchors: true,
+                         lineanchorsid: 'LC'
+                       )
+      end
+
+      def initialize(diff_file)
+        @diff_file = diff_file
+        @file_name = diff_file.new_path
+        @lines     = diff_file.diff_lines
       end
       def highlight
@@ -47,7 +68,7 @@ module Gitlab
       def extract_line_prefixes
         @diff_line_prefixes ||= begin
           if is_diff_line?
-            text_lines.map { |line| line.sub!(/\A((\+|\-)\s*)/, '');$1 }
+            text_lines.map { |line| line.sub!(/\A((\+|\-))/, '');$1 }
           else
             []
           end
@@ -57,11 +78,17 @@ module Gitlab
       def update_diff_lines
         @highlighted_code.lines.each_with_index do |line, i|
           diff_line = @lines[i]
+          line_prefix = @diff_line_prefixes[i] || ' '
           # ignore highlighting for "match" lines
           next if diff_line.type == 'match'
-          diff_line.text = "#{@diff_line_prefixes[i]}#{line}"
+          case diff_line.type
+          when 'new', nil
+            diff_line.text = new_lines[diff_line.new_pos - 1].try(:gsub!, /\A\s/, line_prefix)
+          when 'old'
+            diff_line.text = old_lines[diff_line.old_pos - 1].try(:gsub!, /\A\s/, line_prefix)
+          end
         end
         @lines
@@ -79,12 +106,21 @@ module Gitlab
       end
       def formatter
-        Rouge::Formatters::HTMLGitlab.new(
-          nowrap: true,
-          cssclass: 'code highlight',
-          lineanchors: true,
-          lineanchorsid: 'LC'
-        )
+        self.class.formatter
+      end
+
+      def old_lines
+        @old_lines ||= begin
+          lines = self.class.process_file(diff_repository, diff_old_ref, diff_old_path)
+          lines.map! { |line| " #{line}" }
+        end
+      end
+
+      def new_lines
+        @new_lines ||= begin
+          lines = self.class.process_file(diff_repository, diff_new_ref, diff_new_path)
+          lines.map! { |line| " #{line}" }
+        end
       end
     end
   end