Merge branch 'issue_3945' into 'master'

Add syntax highlighting to diff view

Closes #3945 

See merge request !2109

app/assets/stylesheets/highlight/dark.scss

@@ -90,4 +90,22 @@
   .vg { color: #cc6666 } /* Name.Variable.Global */
   .vi { color: #cc6666 } /* Name.Variable.Instance */
   .il { color: #de935f } /* Literal.Number.Integer.Long */
+
+  .line_holder {
+    &.parallel .new.new_line,
+    &.parallel .new.line_content,
+    &.new .old_line,
+    &.new .new_line,
+    &.new .line_content {
+      @include diff_background(255, 255, 255, #808080);
+    }
+
+    &.parallel .old.old_line,
+    &.parallel .old.line_content,
+    &.old .old_line,
+    &.old .new_line,
+    &.old .line_content {
+      @include diff_background(255, 51, 51, #808080);
+    }
+  }
 }

app/assets/stylesheets/highlight/monokai.scss

@@ -90,4 +90,22 @@
   .gu { color: #75715e; } /* Generic.Subheading & Diff Unified/Comment? */
   .gd { color: #f92672; } /* Generic.Deleted & Diff Deleted */
   .gi { color: #a6e22e; } /* Generic.Inserted & Diff Inserted */
+
+  .line_holder {
+    &.parallel .new.new_line,
+    &.parallel .new.line_content,
+    &.new .old_line,
+    &.new .new_line,
+    &.new .line_content {
+      @include diff_background(156, 175, 183, #808080);
+    }
+
+    &.parallel .old.old_line,
+    &.parallel .old.line_content,
+    &.old .old_line,
+    &.old .new_line,
+    &.old .line_content {
+      @include diff_background(254, 147, 140, #808080);
+    }
+  }
 }

app/assets/stylesheets/highlight/solarized_dark.scss

@@ -111,4 +111,22 @@
   .vg { color: #268bd2 } /* Name.Variable.Global */
   .vi { color: #268bd2 } /* Name.Variable.Instance */
   .il { color: #2aa198 } /* Literal.Number.Integer.Long */
+
+  .line_holder {
+    &.parallel .new.new_line,
+    &.parallel .new.line_content,
+    &.new .old_line,
+    &.new .new_line,
+    &.new .line_content {
+      @include diff_background(255, 255, 255, #808080);
+    }
+
+    &.parallel .old.old_line,
+    &.parallel .old.line_content,
+    &.old .old_line,
+    &.old .new_line,
+    &.old .line_content {
+      @include diff_background(255, 51, 51, #808080);
+    }
+  }
 }

app/assets/stylesheets/highlight/solarized_light.scss

@@ -111,4 +111,23 @@
   .vg { color: #268bd2 } /* Name.Variable.Global */
   .vi { color: #268bd2 } /* Name.Variable.Instance */
   .il { color: #2aa198 } /* Literal.Number.Integer.Long */
+
+
+  .line_holder {
+    &.parallel .new.new_line,
+    &.parallel .new.line_content,
+    &.new .old_line,
+    &.new .new_line,
+    &.new .line_content {
+      @include diff_background(92, 164, 169, #FAF3DD);
+    }
+
+    &.parallel .old.old_line,
+    &.parallel .old.line_content,
+    &.old .old_line,
+    &.old .new_line,
+    &.old .line_content {
+      @include diff_background(237, 106, 90, #FAF3DD);
+    }
+  }
 }

app/assets/stylesheets/pages/diff.scss

@@ -393,3 +393,18 @@
     right: 15px;
   }
 }
+
+@mixin diff_background($r, $g, $b, $custom-border) {
+  /* Fallback for web browsers that doesn't support RGBa */
+  background: rgb($r, $g, $b);
+  /* RGBa with 0.3 opacity */
+  background: rgba($r, $g, $b, 0.3);
+
+  &.new_line, &.old_line {
+    border-right-color: $custom-border !important;
+  }
+
+  &.line_content span.idiff {
+    background: rgb($r, $g, $b);
+  }
+}

app/controllers/application_controller.rb

@@ -25,6 +25,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,9 @@ class Projects::BlobController < Projects::ApplicationController
   end
   def diff
-    @form = UnfoldForm.new(params)
-    @lines = @blob.data.lines[@form.since - 1..@form.to - 1]
+    @form  = UnfoldForm.new(params)
+    @lines = Gitlab::Highlight.highlight_lines(repository, @ref, @path)
+    @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 || commit, commit]
     @notes_count = commit.notes.count
     @statuses = ci_commit.statuses if ci_commit

app/controllers/projects/compare_controller.rb

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

app/controllers/projects/merge_requests_controller.rb

@@ -58,7 +58,11 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   def diffs
     @commit = @merge_request.last_commit
-    @first_commit = @merge_request.first_commit
+    @base_commit = @merge_request.diff_base_commit
+
+    # MRs created before 8.4 don't have a diff_base_commit,
+    # but we need it for the "View file @ ..." link by deleted files
+    @base_commit ||= @merge_request.first_commit.parent || @merge_request.first_commit
     @comments_allowed = @reply_allowed = true
     @comments_target = {
@@ -102,7 +106,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     @source_project = merge_request.source_project
     @commits = @merge_request.compare_commits.reverse
     @commit = @merge_request.last_commit
-    @first_commit = @merge_request.first_commit
+    @base_commit = @merge_request.diff_base_commit
     @diffs = @merge_request.compare_diffs
     @ci_commit = @merge_request.ci_commit

app/helpers/blob_helper.rb

 module BlobHelper
-  def highlight(blob_name, blob_content, nowrap: false, continue: false)
-    @formatter ||= Rouge::Formatters::HTMLGitlab.new(
-      nowrap: nowrap,
-      cssclass: 'code highlight',
-      lineanchors: true,
-      lineanchorsid: 'LC'
-    )
-
-    begin
-      @lexer ||= Rouge::Lexer.guess(filename: blob_name, source: blob_content).new
-      result = @formatter.format(@lexer.lex(blob_content, continue: continue)).html_safe
-    rescue
-      @lexer = Rouge::Lexers::PlainText
-      result = @formatter.format(@lexer.lex(blob_content)).html_safe
-    end
-    result
+  def highlighter(blob_name, blob_content, nowrap: false)
+    Gitlab::Highlight.new(blob_name, blob_content, nowrap: nowrap)
+  end
+  def highlight(blob_name, blob_content, nowrap: false)
+    Gitlab::Highlight.highlight(blob_name, blob_content, nowrap: nowrap)
   end
   def no_highlight_files

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)
     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)
     end
     safe_files
   end
@@ -43,64 +43,6 @@ module DiffHelper
     Gitlab::Diff::LineCode.generate(file_path, line.new_pos, line.old_pos)
   end
-  def parallel_diff(diff_file, index)
-    lines = []
-    skip_next = false
-
-    # Building array of lines
-    #
-    # [
-    # left_type, left_line_number, left_line_content, left_line_code,
-    # right_line_type, right_line_number, right_line_content, right_line_code
-    # ]
-    #
-    diff_file.diff_lines.each do |line|
-
-      full_line = line.text
-      type = line.type
-      line_code = generate_line_code(diff_file.file_path, line)
-      line_new = line.new_pos
-      line_old = line.old_pos
-
-      next_line = diff_file.next_line(line.index)
-
-      if next_line
-        next_line_code = generate_line_code(diff_file.file_path, next_line)
-        next_type = next_line.type
-        next_line = next_line.text
-      end
-
-      if type == 'match' || type.nil?
-        # line in the right panel is the same as in the left one
-        line = [type, line_old, full_line, line_code, type, line_new, full_line, line_code]
-        lines.push(line)
-      elsif type == 'old'
-        if next_type == 'new'
-          # Left side has text removed, right side has text added
-          line = [type, line_old, full_line, line_code, next_type, line_new, next_line, next_line_code]
-          lines.push(line)
-          skip_next = true
-        elsif next_type == 'old' || next_type.nil?
-          # Left side has text removed, right side doesn't have any change
-          # No next line code, no new line number, no new line text
-          line = [type, line_old, full_line, line_code, next_type, nil, "&nbsp;", nil]
-          lines.push(line)
-        end
-      elsif type == 'new'
-        if skip_next
-          # Change has been already included in previous line so no need to do it again
-          skip_next = false
-          next
-        else
-          # Change is only on the right side, left side has no change
-          line = [nil, nil, "&nbsp;", line_code, type, line_new, full_line, line_code]
-          lines.push(line)
-        end
-      end
-    end
-    lines
-  end
-
   def unfold_bottom_class(bottom)
     (bottom) ? 'js-unfold-bottom' : ''
   end
@@ -111,9 +53,9 @@ module DiffHelper
   def diff_line_content(line)
     if line.blank?
-      " &nbsp;"
+      " &nbsp;".html_safe
     else
-      line
+      line.html_safe
     end
   end
@@ -160,8 +102,7 @@ module DiffHelper
   def commit_for_diff(diff)
     if diff.deleted_file
-      first_commit = @first_commit || @commit
-      first_commit.parent || @first_commit
+      @base_commit || @commit.parent || @commit
     else
       @commit
     end

app/models/merge_request.rb

@@ -180,6 +180,14 @@ class MergeRequest < ActiveRecord::Base
     merge_request_diff ? merge_request_diff.first_commit : compare_commits.first
   end
+  def diff_base_commit
+    if merge_request_diff
+      merge_request_diff.base_commit
+    else
+      self.target_project.commit(self.target_branch)
+    end
+  end
+
   def last_commit_short_sha
     last_commit.short_id
   end
@@ -254,7 +262,7 @@ class MergeRequest < ActiveRecord::Base
   end
   def mergeable?
-    return false unless open? && !work_in_progress?
+    return false unless open? && !work_in_progress? && !broken?
     check_if_can_be_merged
@@ -477,8 +485,7 @@ class MergeRequest < ActiveRecord::Base
   end
   def target_sha
-    @target_sha ||= target_project.
-      repository.commit(target_branch).sha
+    @target_sha ||= target_project.repository.commit(target_branch).sha
   end
   def source_sha
@@ -517,4 +524,10 @@ class MergeRequest < ActiveRecord::Base
   def ci_commit
     @ci_commit ||= source_project.ci_commit(last_commit.id) if last_commit && source_project
   end
+
+  def diff_refs
+    return nil unless diff_base_commit
+
+    [diff_base_commit, last_commit]
+  end
 end

app/models/merge_request_diff.rb

@@ -73,6 +73,12 @@ class MergeRequestDiff < ActiveRecord::Base
     commits.last
   end
+  def base_commit
+    return nil unless self.base_commit_sha
+
+    merge_request.target_project.commit(self.base_commit_sha)
+  end
+
   def last_commit_short_sha
     @last_commit_short_sha ||= last_commit.short_id
   end
@@ -156,6 +162,9 @@ class MergeRequestDiff < ActiveRecord::Base
     end
     self.st_diffs = new_diffs
+
+    self.base_commit_sha = merge_request.target_project.commit(target_branch).try(:sha)
+
     self.save
   end

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

@@ -15,6 +15,7 @@
     .file-content.blame.highlight
       %table
         - current_line = 1
+        - blame_highlighter = highlighter(@blob.name, @blob.data, nowrap: true)
         - @blame.each do |blame_group|
           %tr
             %td.blame-commit
@@ -41,5 +42,5 @@
               %pre{class: 'code highlight white'}
                 %code
                   - blame_group[:lines].each do |line|
-                    :erb
-                      <%= highlight(@blob.name, line, nowrap: true, continue: true).html_safe %>
+                    :preserve
+                      #{blame_highlighter.highlight(line)}

app/views/projects/blob/diff.html.haml

@@ -11,7 +11,7 @@
       %td.old_line.diff-line-num{data: {linenumber: line_old}}
         = link_to raw(line_old), "#"
       %td.new_line= link_to raw(line_new) , "#"
-      %td.line_content.noteable_line= ' ' * @form.indent + line
+      %td.line_content.noteable_line==#{' ' * @form.indent}#{line}
   - if @form.unfold? && @form.bottom? && @form.to < @blob.loc
     %tr.line_holder{ id: @form.to }

app/views/projects/blob/preview.html.haml

@@ -20,6 +20,6 @@
                 - else
                   %td.old_line
                   %td.new_line
-                  %td.line_content{class: "#{line.type}"}= raw diff_line_content(line.text)
+                  %td.line_content{class: "#{line.type}"}= diff_line_content(line.text)
         - else
           .nothing-here-block No changes.

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

@@ -9,5 +9,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)
 .content-block.oneline-block
   .inline-parallel-buttons