Merge branch 'parallel_diff_refactor' into 'master'

Parallel diff refactor

Refactor side-by-side diff to remove code duplication.

Base the side-by-side diff on inline diff code.

In first iteration:

1. commenting on the side-by-side diff is not possible
1. no expanding of the surrounding code

See merge request !1066

app/controllers/projects/edit_tree_controller.rb

@@ -31,7 +31,7 @@ class Projects::EditTreeController < Projects::BaseTreeController
 
     diffy = Diffy::Diff.new(@blob.data, @content, diff: '-U 3',
                             include_diff_info: true)
-    @diff = Gitlab::DiffParser.new(diffy.diff.scan(/.*\n/))
+    @diff_lines = Gitlab::Diff::Parser.new.parse(diffy.diff.scan(/.*\n/))
 
     render layout: false
   end

app/helpers/commits_helper.rb

@@ -16,38 +16,6 @@ module CommitsHelper
     commit_person_link(commit, options.merge(source: :committer))
   end
 
-  def each_diff_line(diff, index)
-    Gitlab::DiffParser.new(diff.diff.lines.to_a, diff.new_path)
-      .each do |full_line, type, line_code, line_new, line_old|
-        yield(full_line, type, line_code, line_new, line_old)
-      end
-  end
-
-  def each_diff_line_near(diff, index, expected_line_code)
-    max_number_of_lines = 16
-
-    prev_match_line = nil
-    prev_lines = []
-
-    each_diff_line(diff, index) do |full_line, type, line_code, line_new, line_old|
-      line = [full_line, type, line_code, line_new, line_old]
-      if line_code != expected_line_code
-        if type == "match"
-          prev_lines.clear
-          prev_match_line = line
-        else
-          prev_lines.push(line)
-          prev_lines.shift if prev_lines.length >= max_number_of_lines
-        end
-      else
-        yield(prev_match_line) if !prev_match_line.nil?
-        prev_lines.each { |ln| yield(ln) }
-        yield(line)
-        break
-      end
-    end
-  end
-
   def image_diff_class(diff)
     if diff.deleted_file
       "deleted"
@@ -63,14 +31,6 @@ module CommitsHelper
     escape_javascript(render "projects/commits/#{template}", commit: commit, project: project) unless commit.nil?
   end
 
-  def diff_line_content(line)
-    if line.blank?
-      "  "
-    else
-      line
-    end
-  end
-
   # Breadcrumb links for a Project and, if applicable, a tree path
   def commits_breadcrumbs
     return unless @project && @ref
@@ -105,82 +65,6 @@ module CommitsHelper
     branches.sort.map { |branch| link_to(branch, project_tree_path(project, branch)) }.join(", ").html_safe
   end
 
-  def parallel_diff_lines(project, commit, diff, file)
-    old_file = project.repository.blob_at(commit.parent_id, diff.old_path) if commit.parent_id
-    deleted_lines = {}
-    added_lines = {}
-    each_diff_line(diff, 0) do |line, type, line_code, line_new, line_old|
-      if type == "old"
-        deleted_lines[line_old] = { line_code: line_code, type: type, line: line }
-      elsif type == "new"
-        added_lines[line_new]   = { line_code: line_code, type: type, line: line }
-      end
-    end
-    max_length = old_file ? [old_file.loc, file.loc].max : file.loc
-
-    offset1 = 0
-    offset2 = 0
-    old_lines = []
-    new_lines = []
-
-    max_length.times do |line_index|
-      line_index1 = line_index - offset1
-      line_index2 = line_index - offset2
-      deleted_line = deleted_lines[line_index1 + 1]
-      added_line = added_lines[line_index2 + 1]
-      old_line = old_file.lines[line_index1] if old_file
-      new_line = file.lines[line_index2]
-
-      if deleted_line && added_line
-      elsif deleted_line
-        new_line = nil
-        offset2 += 1
-      elsif added_line
-        old_line = nil
-        offset1 += 1
-      end
-
-      old_lines[line_index] = DiffLine.new
-      new_lines[line_index] = DiffLine.new
-
-      # old
-      if line_index == 0 && diff.new_file
-        old_lines[line_index].type = :file_created
-        old_lines[line_index].content = 'File was created'
-      elsif deleted_line
-        old_lines[line_index].type = :deleted
-        old_lines[line_index].content = old_line
-        old_lines[line_index].num = line_index1 + 1
-        old_lines[line_index].code = deleted_line[:line_code]
-      elsif old_line
-        old_lines[line_index].type = :no_change
-        old_lines[line_index].content = old_line
-        old_lines[line_index].num = line_index1 + 1
-      else
-        old_lines[line_index].type = :added
-      end
-
-      # new
-      if line_index == 0 && diff.deleted_file
-        new_lines[line_index].type = :file_deleted
-        new_lines[line_index].content = "File was deleted"
-      elsif added_line
-        new_lines[line_index].type = :added
-        new_lines[line_index].num = line_index2 + 1
-        new_lines[line_index].content = new_line
-        new_lines[line_index].code = added_line[:line_code]
-      elsif new_line
-        new_lines[line_index].type = :no_change
-        new_lines[line_index].num = line_index2 + 1
-        new_lines[line_index].content = new_line
-      else
-        new_lines[line_index].type = :deleted
-      end
-    end
-
-    return old_lines, new_lines
-  end
-
   def link_to_browse_code(project, commit)
     if current_controller?(:projects, :commits)
       if @repo.blob_at(commit.id, @path)
@@ -229,14 +113,6 @@ module CommitsHelper
     end
   end
 
-  def diff_file_mode_changed?(diff)
-    diff.a_mode && diff.b_mode && diff.a_mode != diff.b_mode
-  end
-
-  def unfold_bottom_class(bottom)
-    (bottom) ? 'js-unfold-bottom' : ''
-  end
-
   def view_file_btn(commit_sha, diff, project)
     link_to project_blob_path(project, tree_join(commit_sha, diff.new_path)),
             class: 'btn btn-small view-file js-view-file' do

app/helpers/diff_helper.rb

 module DiffHelper
-  def safe_diff_files(diffs)
+  def allowed_diff_size
     if diff_hard_limit_enabled?
-      diffs.first(Commit::DIFF_HARD_LIMIT_FILES)
+      Commit::DIFF_HARD_LIMIT_FILES
     else
-      diffs.first(Commit::DIFF_SAFE_FILES)
+      Commit::DIFF_SAFE_FILES
     end
   end
 
-  def show_diff_size_warninig?(diffs)
-    safe_diff_files(diffs).size < diffs.size
+  def safe_diff_files(diffs)
+    diffs.first(allowed_diff_size).map do |diff|
+      Gitlab::Diff::File.new(diff)
+    end
+  end
+
+  def show_diff_size_warning?(diffs)
+    diffs.size > allowed_diff_size
   end
 
   def diff_hard_limit_enabled?
@@ -19,4 +25,76 @@ module DiffHelper
       false
     end
   end
+
+  def generate_line_code(file_path, line)
+    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, line_code, right_line_type, right_line_number, right_line_content]
+    #
+    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_type = next_line.type
+        next_line = next_line.text
+      end
+
+      line = [type, line_old, full_line, line_code, next_type, line_new]
+      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]
+        lines.push(line)
+      elsif type == 'old'
+        if next_type == 'new'
+          # Left side has text removed, right side has text added
+          line.push(next_line)
+          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
+          line.pop # remove the newline
+          line.push(nil) # no line number on the right panel
+          line.push("&nbsp;") # empty line on the right panel
+          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]
+          lines.push(line)
+        end
+      end
+    end
+    lines
+  end
+
+  def unfold_bottom_class(bottom)
+    (bottom) ? 'js-unfold-bottom' : ''
+  end
+
+  def diff_line_content(line)
+    if line.blank?
+      " &nbsp;"
+    else
+      line
+    end
+  end
 end

app/models/note.rb

@@ -209,9 +209,10 @@ class Note < ActiveRecord::Base
     noteable.diffs.each do |mr_diff|
       next unless mr_diff.new_path == self.diff.new_path
 
-      Gitlab::DiffParser.new(mr_diff.diff.lines.to_a, mr_diff.new_path).
-        each do |full_line, type, line_code, line_new, line_old|
-        if full_line == diff_line
+      lines = Gitlab::Diff::Parser.new.parse(mr_diff.diff.lines.to_a)
+
+      lines.each do |line|
+        if line.text == diff_line
           return true
         end
       end
@@ -232,6 +233,14 @@ class Note < ActiveRecord::Base
     diff.new_path if diff
   end
 
+  def file_path
+    if diff.new_path.present?
+      diff.new_path
+    elsif diff.old_path.present?
+      diff.old_path
+    end
+  end
+
   def diff_old_line
     line_code.split('_')[1].to_i
   end
@@ -240,19 +249,49 @@ class Note < ActiveRecord::Base
     line_code.split('_')[2].to_i
   end
 
+  def generate_line_code(line)
+    Gitlab::Diff::LineCode.generate(file_path, line.new_pos, line.old_pos)
+  end
+
   def diff_line
     return @diff_line if @diff_line
 
     if diff
-      Gitlab::DiffParser.new(diff.diff.lines.to_a, diff.new_path)
-        .each do |full_line, type, line_code, line_new, line_old|
-          @diff_line = full_line if line_code == self.line_code
+      diff_lines.each do |line|
+        if generate_line_code(line) == self.line_code
+          @diff_line = line.text
+        end
       end
     end
 
     @diff_line
   end
 
+  def truncated_diff_lines
+    max_number_of_lines = 16
+    prev_match_line = nil
+    prev_lines = []
+
+    diff_lines.each do |line|
+      if generate_line_code(line) != self.line_code
+        if line.type == "match"
+          prev_lines.clear
+          prev_match_line = line
+        else
+          prev_lines.push(line)
+          prev_lines.shift if prev_lines.length >= max_number_of_lines
+        end
+      else
+        prev_lines << line
+        return prev_lines
+      end
+    end
+  end
+
+  def diff_lines
+    @diff_lines ||= Gitlab::Diff::Parser.new.parse(diff.diff.lines.to_a)
+  end
+
   def discussion_id
     @discussion_id ||= Note.build_discussion_id(noteable_type, noteable_id || commit_id, line_code)
   end

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

 - if @lines.present?
   - if @form.unfold? && @form.since != 1 && !@form.bottom?
     %tr.line_holder{ id: @form.since }
-      = render "projects/commits/diffs/match_line", {line: @match_line,
+      = render "projects/diffs/match_line", {line: @match_line,
         line_old: @form.since, line_new: @form.since, bottom: false}
 
   - @lines.each_with_index do |line, index|
@@ -15,5 +15,5 @@
 
   - if @form.unfold? && @form.bottom? && @form.to < @blob.loc
     %tr.line_holder{ id: @form.to }
-      = render "projects/commits/diffs/match_line", {line: @match_line,
+      = render "projects/diffs/match_line", {line: @match_line,
         line_old: @form.to, line_new: @form.to, bottom: true}

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

 = render "commit_box"
-= render "projects/commits/diffs", diffs: @diffs, project: @project
+= render "projects/diffs/diffs", diffs: @diffs, project: @project
 = render "projects/notes/notes_with_form"

app/views/projects/commits/_parallel_view.html.haml

-/ Side-by-side diff view
-- old_lines, new_lines = parallel_diff_lines(project, @commit, diff, file)
-- num_lines = old_lines.length
-
-%div.text-file
-  %table
-    - num_lines.times do |index|
-      - new_line = new_lines[index]
-      - old_line = old_lines[index]
-      %tr.line_holder.parallel
-        -# For old line
-        - if old_line.type == :file_created
-          %td.old_line= old_line.num
-          %td.line_content.parallel= "File was created"
-        - elsif old_line.type == :deleted
-          %td.old_line.old= old_line.num
-          %td.line_content{class: "parallel noteable_line old #{old_line.code}", "line_code" => old_line.code}= old_line.content
-        - else old_line.type == :no_change
-          %td.old_line= old_line.num
-          %td.line_content.parallel= old_line.content
-
-        -# For new line
-        - if new_line.type == :file_deleted
-          %td.new_line= new_line.num
-          %td.line_content.parallel= "File was deleted"
-        - elsif new_line.type == :added
-          %td.new_line.new= new_line.num
-          %td.line_content{class: "parallel noteable_line new #{new_line.code}", "line_code" => new_line.code}= new_line.content
-        - else new_line.type == :no_change
-          %td.new_line= new_line.num
-          %td.line_content.parallel= new_line.content
-
-      - if @reply_allowed
-        - comments1 = @line_notes.select { |n| n.line_code == old_line.code }.sort_by(&:created_at)
-        - comments2 = @line_notes.select { |n| n.line_code == new_line.code }.sort_by(&:created_at)
-        - unless comments1.empty? and comments2.empty?
-          = render "projects/notes/diff_notes_with_reply_parallel", notes1: comments1, notes2: comments2
-

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

@@ -18,7 +18,7 @@
     - else
       %ul.well-list= render Commit.decorate(@commits), project: @project
 
-  = render "projects/commits/diffs", diffs: @diffs, project: @project
+  = render "projects/diffs/diffs", diffs: @diffs, project: @project
 
 - else
   .light-well

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

 .row
   .col-md-8
-    = render 'projects/commits/diff_stats', diffs: diffs
+    = render 'projects/diffs/stats', diffs: diffs
   .col-md-4
     %ul.nav.nav-tabs
       %li.pull-right{class: params[:view] == 'parallel' ? 'active' : ''}
@@ -11,12 +11,13 @@
         - params_copy[:view] = 'inline'
         = link_to "Inline Diff", url_for(params_copy), {id: "commit-diff-viewtype"}
 
-- if show_diff_size_warninig?(diffs)
-  = render 'projects/commits/diff_warning', diffs: diffs
+
+- if show_diff_size_warning?(diffs)
+  = render 'projects/diffs/warning', diffs: diffs
 
 .files
-  - safe_diff_files(diffs).each_with_index do |diff, i|
-    = render 'projects/commits/diff_file', diff: diff, i: i, project: project
+  - safe_diff_files(diffs).each_with_index do |diff_file, index|
+    = render 'projects/diffs/file', diff_file: diff_file, i: index, project: project
 
 - if @diff_timeout
   .alert.alert-danger

app/views/projects/commits/_diff_file.html.haml → app/views/projects/diffs/_file.html.haml

-- file = project.repository.blob_for_diff(@commit, diff)
-- return unless file
-- blob_diff_path = diff_project_blob_path(project,
-  tree_join(@commit.id, diff.new_path))
+- blob = project.repository.blob_for_diff(@commit, diff_file.diff)
+- return unless blob
+- blob_diff_path = diff_project_blob_path(project, tree_join(@commit.id, diff_file.file_path))
 .diff-file{id: "diff-#{i}", data: {blob_diff_path: blob_diff_path }}
-  .diff-header{id: "file-path-#{hexdigest(diff.new_path || diff.old_path)}"}
-    - if diff.deleted_file
-      %span= diff.old_path
+  .diff-header{id: "file-path-#{hexdigest(diff_file.new_path || diff_file.old_path)}"}
+    - if diff_file.deleted_file
+      %span= diff_file.old_path
 
       .diff-btn-group
         - if @commit.parent_ids.present?
-          = view_file_btn(@commit.parent_id, diff, project)
+          = view_file_btn(@commit.parent_id, diff_file, project)
     - else
-      %span= diff.new_path
-      - if diff_file_mode_changed?(diff)
+      %span= diff_file.new_path
+      - if diff_file.mode_changed?
         %span.file-mode= "#{diff.a_mode} → #{diff.b_mode}"
 
       .diff-btn-group
@@ -26,23 +25,23 @@
          
 
         - if @merge_request && @merge_request.source_project
-          = link_to project_edit_tree_path(@merge_request.source_project, tree_join(@merge_request.source_branch, diff.new_path), from_merge_request_id: @merge_request.id), { class: 'btn btn-small' } do
+          = link_to project_edit_tree_path(@merge_request.source_project, tree_join(@merge_request.source_branch, diff_file.new_path), from_merge_request_id: @merge_request.id), { class: 'btn btn-small' } do
             Edit
            
 
-        = view_file_btn(@commit.id, diff, project)
+        = view_file_btn(@commit.id, diff_file, project)
 
   .diff-content
     -# Skipp all non non-supported blobs
-    - return unless file.respond_to?('text?')
-    - if file.text?
+    - return unless blob.respond_to?('text?')
+    - if blob.text?
       - if params[:view] == 'parallel'
-        = render "projects/commits/parallel_view", diff: diff, project: project, file: file, index: i
+        = render "projects/diffs/parallel_view", diff_file: diff_file, project: project, blob: blob, index: i
       - else
-        = render "projects/commits/text_file", diff: diff, index: i
-    - elsif file.image?
-      - old_file = project.repository.prev_blob_for_diff(@commit, diff)
-      = render "projects/commits/image", diff: diff, old_file: old_file, file: file, index: i
+        = render "projects/diffs/text_file", diff_file: diff_file, index: i
+    - elsif blob.image?
+      - old_file = project.repository.prev_blob_for_diff(@commit, diff_file)
+      = render "projects/diffs/image", diff_file: diff_file, old_file: old_file, file: blob, index: i
     - else
       .nothing-here-block No preview for this file type
 

app/views/projects/commits/_image.html.haml → app/views/projects/diffs/_image.html.haml

+- diff = diff_file.diff
 - if diff.renamed_file || diff.new_file || diff.deleted_file
   .image
     %span.wrap

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

+%td.old_line
+  %td.line_content.parallel.matched= line
+%td.new_line
+  %td.line_content.parallel.matched= line

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

+/ Side-by-side diff view
+%div.text-file
+  %table
+    - parallel_diff(diff_file, index).each do |line|
+      - type_left = line[0]
+      - line_number_left = line[1]
+      - line_content_left = line[2]
+      - line_code = line[3]
+      - type_right = line[4]
+      - line_number_right = line[5]
+      - line_content_right = line[6]
+
+      %tr.line_holder.parallel{id: line_code}
+        - if type_left == 'match'
+          = render "projects/diffs/match_line_parallel", { line: line_content_left,
+          line_old: line_number_left, line_new: line_number_right }
+        - elsif type_left ==  'old' || type_left.nil?
+          %td.old_line{class: "#{type_left}"}
+            = link_to raw(line_number_left), "##{line_code}", id: line_code
+            %td.line_content{class: "parallel noteable_line #{type_left} #{line_code}", "line_code" => line_code }= raw line_content_left
+          %td.new_line{ class: "#{type_right == 'new' ? 'new' : nil}", data: { linenumber: line_number_right }}
+            = link_to raw(line_number_right), "##{line_code}", id: line_code
+            %td.line_content.parallel{class: "noteable_line #{type_right == 'new' ? 'new' : nil} #{line_code}", "line_code" => line_code}= raw line_content_right
+
+- if diff_file.diff.diff.blank? && diff_file.mode_changed?
+  .file-mode-changed
+    File mode changed

app/views/projects/commits/_text_file.html.haml → app/views/projects/diffs/_text_file.html.haml

-- too_big = diff.diff.lines.count > Commit::DIFF_SAFE_LINES
+- too_big = diff_file.diff_lines.count > Commit::DIFF_SAFE_LINES
 - if too_big
   %a.supp_diff_link Changes suppressed. Click to show
 
 %table.text-file{class: "#{'hide' if too_big}"}
   - last_line = 0
-  - each_diff_line(diff, index) do |line, type, line_code, line_new, line_old, raw_line|
-    - last_line = line_new
+  - diff_file.diff_lines.each_with_index do |line, index|
+    - type = line.type
+    - last_line = line.new_pos
+    - line_code = generate_line_code(diff_file.file_path, line)
+    - line_old = line.old_pos
     %tr.line_holder{ id: line_code, class: "#{type}" }
       - if type == "match"
-        = render "projects/commits/diffs/match_line", {line: line,
-          line_old: line_old, line_new: line_new, bottom: false}
+        = render "projects/diffs/match_line", {line: line.text,
+          line_old: line_old, line_new: line.new_pos, bottom: false}
       - else
         %td.old_line
           = link_to raw(type == "new" ? " " : line_old), "##{line_code}", id: line_code
           - if @comments_allowed
             = link_to_new_diff_note(line_code)
-        %td.new_line{data: {linenumber: line_new}}
-          = link_to raw(type == "old" ? " " : line_new) , "##{line_code}", id: line_code
-        %td.line_content{class: "noteable_line #{type} #{line_code}", "line_code" => line_code}= raw diff_line_content(line)
+        %td.new_line{data: {linenumber: line.new_pos}}
+          = link_to raw(type == "old" ? " " : line.new_pos) , "##{line_code}", id: line_code
+        %td.line_content{class: "noteable_line #{type} #{line_code}", "line_code" => line_code}= raw diff_line_content(line.text)
 
     - if @reply_allowed
       - comments = @line_notes.select { |n| n.line_code == line_code }.sort_by(&:created_at)
       - unless comments.empty?
-        = render "projects/notes/diff_notes_with_reply", notes: comments, line: line
+        = render "projects/notes/diff_notes_with_reply", notes: comments, line: line.text
 
   - if last_line > 0
-    = render "projects/commits/diffs/match_line", {line: "",
+    = render "projects/diffs/match_line", {line: "",
       line_old: last_line, line_new: last_line, bottom: true}
 
-- if diff.diff.blank? && diff_file_mode_changed?(diff)
+- if diff_file.diff.blank? && diff_file.mode_changed?
   .file-mode-changed
     File mode changed

app/views/projects/commits/_diff_warning.html.haml → app/views/projects/diffs/_warning.html.haml

@@ -14,6 +14,6 @@
           = link_to "Email patch", project_merge_request_path(@project, @merge_request, format: :patch), class: "btn btn-warning btn-small"
   %p
     To preserve performance only
-    %strong #{safe_diff_files(diffs).size} of #{diffs.size}
+    %strong #{allowed_diff_size} of #{diffs.size}
     files displayed.
 

app/views/projects/edit_tree/_diff.html.haml

-%table.text-file
-  - each_diff_line(diff, 1) do |line, type, line_code, line_new, line_old, raw_line|
-    %tr.line_holder{ id: line_code, class: "#{type}" }
-      - if type == "match"
-        %td.old_line= "..."
-        %td.new_line= "..."
-        %td.line_content.matched= line
-      - else
-        %td.old_line
-          = link_to raw(type == "new" ? " " : line_old), "##{line_code}", id: line_code
-        %td.new_line= link_to raw(type == "old" ? " " : line_new) , "##{line_code}", id: line_code
-        %td.line_content{class: "noteable_line #{type} #{line_code}", "line_code" => line_code}= raw diff_line_content(line)
-

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

@@ -9,18 +9,17 @@
         = raw render_markup(@blob.name, @content)
     - else
       .file-content.code
-        - unless @diff.empty?
+        - unless @diff_lines.empty?
           %table.text-file
-            - @diff.each do |line, type, line_code, line_new, line_old, raw_line|
-              %tr.line_holder{ id: line_code, class: "#{type}" }
-                - if type == "match"
+            - @diff_lines.each do |line|
+              %tr.line_holder{ class: "#{line.type}" }
+                - if line.type == "match"
                   %td.old_line= "..."
                   %td.new_line= "..."
-                  %td.line_content.matched= line
+                  %td.line_content.matched= line.text
                 - else
                   %td.old_line
-                    = link_to raw(type == "new" ? " " : line_old), "##{line_code}", id: line_code
-                  %td.new_line= link_to raw(type == "old" ? " " : line_new) , "##{line_code}", id: line_code
-                  %td.line_content{class: "noteable_line #{type} #{line_code}", "line_code" => line_code}= raw diff_line_content(line)
+                  %td.new_line
+                  %td.line_content{class: "#{line.type}"}= raw diff_line_content(line.text)
         - else
           .nothing-here-block No changes.

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

@@ -75,7 +75,7 @@
 
   %h4 Changes
   - if @diffs.present?
-    = render "projects/commits/diffs", diffs: @diffs, project: @project
+    = render "projects/diffs/diffs", diffs: @diffs, project: @project
   - elsif @commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE
     .bs-callout.bs-callout-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/commits/diffs", diffs: @merge_request.diffs, project: @merge_request.source_project
+  = render "projects/diffs/diffs", diffs: @merge_request.diffs, project: @merge_request.source_project
 - elsif @merge_request_diff.empty?
   .nothing-here-block Nothing to merge from #{@merge_request.source_branch} into #{@merge_request.target_branch}
 - else

app/views/projects/notes/_diff_note_link.html.haml

-- note = @project.notes.new(@comments_target.merge({ line_code: line_code }))
-= link_to "",
-          "javascript:;",
-          class: "add-diff-note js-add-diff-note-button",
-          data: { noteable_type: note.noteable_type,
-                  noteable_id:   note.noteable_id,
-                  commit_id:     note.commit_id,
-                  line_code:     note.line_code,
-                  discussion_id: note.discussion_id },
-          title: "Add a comment to this line"

app/views/projects/notes/discussions/_diff.html.haml

@@ -11,16 +11,17 @@
       %br/
     .diff-content
       %table
-        - each_diff_line_near(diff, note.diff_file_index, note.line_code) do |line, type, line_code, line_new, line_old|
+        - note.truncated_diff_lines.each do |line|
+          - line_code = generate_line_code(note.file_path, line)
           %tr.line_holder{ id: line_code }
-            - if type == "match"
+            - if line.type == "match"
               %td.old_line= "..."
               %td.new_line= "..."
-              %td.line_content.matched= line
+              %td.line_content.matched= line.text
             - else
-              %td.old_line= raw(type == "new" ? " " : line_old)
-              %td.new_line= raw(type == "old" ? " " : line_new)
-              %td.line_content{class: "noteable_line #{type} #{line_code}", "line_code" => line_code}= raw "#{line}  "
+              %td.old_line= raw(line.type == "new" ? " " : line.old_pos)
+              %td.new_line= raw(line.type == "old" ? " " : line.new_pos)
+              %td.line_content{class: "noteable_line #{line.type} #{line_code}", "line_code" => line_code}= raw "#{line.text}  "
 
               - if line_code == note.line_code
                 = render "projects/notes/diff_notes_with_reply", notes: discussion_notes

lib/gitlab/diff/file.rb

+module Gitlab
+  module Diff
+    class File
+      attr_reader :diff
+
+      delegate :new_file, :deleted_file, :renamed_file,
+        :old_path, :new_path, to: :diff, prefix: false
+
+      def initialize(diff)
+        @diff = diff
+      end
+
+      # Array of Gitlab::DIff::Line objects
+      def diff_lines
+        @lines ||= parser.parse(raw_diff.lines)
+      end
+
+      def mode_changed?
+        !!(diff.a_mode && diff.b_mode && diff.a_mode != diff.b_mode)
+      end
+
+      def parser
+        Gitlab::Diff::Parser.new
+      end
+
+      def raw_diff
+        diff.diff
+      end
+
+      def next_line(index)
+        diff_lines[index + 1]
+      end
+
+      def prev_line(index)
+        if index > 0
+          diff_lines[index - 1]
+        end
+      end
+
+      def file_path
+        if diff.new_path.present?
+          diff.new_path
+        elsif diff.old_path.present?
+          diff.old_path
+        end
+      end
+    end
+  end
+end

lib/gitlab/diff/line.rb

+module Gitlab
+  module Diff
+    class Line
+      attr_reader :type, :text, :index, :old_pos, :new_pos
+
+      def initialize(text, type, index, old_pos, new_pos)
+        @text, @type, @index = text, type, index
+        @old_pos, @new_pos = old_pos, new_pos
+      end
+    end
+  end
+end

lib/gitlab/diff/line_code.rb

+module Gitlab
+  module Diff
+    class LineCode
+      def self.generate(file_path, new_line_position, old_line_position)
+        "#{Digest::SHA1.hexdigest(file_path)}_#{old_line_position}_#{new_line_position}"
+      end
+    end
+  end
+end

lib/gitlab/diff/parser.rb

+module Gitlab
+  module Diff
+    class Parser
+      include Enumerable
+
+      def parse(lines)
+        @lines = lines,
+        lines_obj = []
+        line_obj_index = 0
+        line_old = 1
+        line_new = 1
+        type = nil
+
+        lines_arr = ::Gitlab::InlineDiff.processing lines
+
+        lines_arr.each do |line|
+          raw_line = line.dup
+
+          next if filename?(line)
+
+          full_line = html_escape(line.gsub(/\n/, ''))
+          full_line = ::Gitlab::InlineDiff.replace_markers full_line
+
+          if line.match(/^@@ -/)
+            type = "match"
+
+            line_old = line.match(/\-[0-9]*/)[0].to_i.abs rescue 0
+            line_new = line.match(/\+[0-9]*/)[0].to_i.abs rescue 0
+
+            next if line_old == 1 && line_new == 1 #top of file
+            lines_obj << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new)
+            line_obj_index += 1
+            next
+          else
+            type = identification_type(line)
+            lines_obj << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new)
+            line_obj_index += 1
+          end
+
+
+          if line[0] == "+"
+            line_new += 1
+          elsif line[0] == "-"
+            line_old += 1
+          else
+            line_new += 1
+            line_old += 1
+          end
+        end
+
+        lines_obj
+      end
+
+      def empty?
+        @lines.empty?
+      end
+
+      private
+
+      def filename?(line)
+        line.start_with?('--- /dev/null', '+++ /dev/null', '--- a', '+++ b',
+                         '--- /tmp/diffy', '+++ /tmp/diffy')
+      end
+
+      def identification_type(line)
+        if line[0] == "+"
+          "new"
+        elsif line[0] == "-"
+          "old"
+        else
+          nil
+        end
+      end
+
+      def html_escape str
+        replacements = { '&' => '&amp;', '>' => '&gt;', '<' => '&lt;', '"' => '&quot;', "'" => '&#39;' }
+          str.gsub(/[&"'><]/, replacements)
+      end
+    end
+  end
+end

lib/gitlab/diff_parser.rb

-module Gitlab
-  class DiffParser
-    include Enumerable
-
-    attr_reader :lines, :new_path
-
-    def initialize(lines, new_path = '')
-      @lines = lines
-      @new_path = new_path
-    end
-
-    def each
-      line_old = 1
-      line_new = 1
-      type = nil
-
-      lines_arr = ::Gitlab::InlineDiff.processing lines
-      lines_arr.each do |line|
-        raw_line = line.dup
-
-        next if filename?(line)
-
-        full_line = html_escape(line.gsub(/\n/, ''))
-        full_line = ::Gitlab::InlineDiff.replace_markers full_line
-
-        if line.match(/^@@ -/)
-          type = "match"
-
-          line_old = line.match(/\-[0-9]*/)[0].to_i.abs rescue 0
-          line_new = line.match(/\+[0-9]*/)[0].to_i.abs rescue 0
-
-          next if line_old == 1 && line_new == 1 #top of file
-          yield(full_line, type, nil, line_new, line_old)
-          next
-        else
-          type = identification_type(line)
-          line_code = generate_line_code(new_path, line_new, line_old)
-          yield(full_line, type, line_code, line_new, line_old, raw_line)
-        end
-
-
-        if line[0] == "+"
-          line_new += 1
-        elsif line[0] == "-"
-          line_old += 1
-        else
-          line_new += 1
-          line_old += 1
-        end
-      end
-    end
-
-    def empty?
-      @lines.empty?
-    end
-
-    private
-
-    def filename?(line)
-      line.start_with?('--- /dev/null', '+++ /dev/null', '--- a', '+++ b',
-                       '--- /tmp/diffy', '+++ /tmp/diffy')
-    end
-
-    def identification_type(line)
-      if line[0] == "+"
-        "new"
-      elsif line[0] == "-"
-        "old"
-      else
-        nil
-      end
-    end
-
-    def generate_line_code(path, line_new, line_old)
-      "#{Digest::SHA1.hexdigest(path)}_#{line_old}_#{line_new}"
-    end
-
-    def html_escape str
-      replacements = { '&' => '&amp;', '>' => '&gt;', '<' => '&lt;', '"' => '&quot;', "'" => '&#39;' }
-        str.gsub(/[&"'><]/, replacements)
-    end
-  end
-end

spec/helpers/diff_helper_spec.rb

+require 'spec_helper'
+
+describe DiffHelper do
+  include RepoHelpers
+
+  let(:project) { create(:project) }
+  let(:commit) { project.repository.commit(sample_commit.id) }
+  let(:diff) { commit.diffs.first }
+  let(:diff_file) { Gitlab::Diff::File.new(diff) }
+
+  describe 'diff_hard_limit_enabled?' do
+    it 'should return true if param is provided' do
+      controller.stub(:params).and_return { { :force_show_diff => true } }
+      diff_hard_limit_enabled?.should be_true
+    end
+
+    it 'should return false if param is not provided' do
+      diff_hard_limit_enabled?.should be_false
+    end
+  end
+
+  describe 'allowed_diff_size' do
+    it 'should return hard limit for a diff if force diff is true' do
+      controller.stub(:params).and_return { { :force_show_diff => true } }
+      allowed_diff_size.should eq(1000)
+    end
+
+    it 'should return safe limit for a diff if force diff is false' do
+      allowed_diff_size.should eq(100)
+    end
+  end
+
+  describe 'parallel_diff' do
+    it 'should return an array of arrays containing the parsed diff' do
+      parallel_diff(diff_file, 0).should match_array(parallel_diff_result_array)
+    end
+  end
+
+  describe 'generate_line_code' do
+    it 'should generate correct line code' do
+      generate_line_code(diff_file.file_path, diff_file.diff_lines.first).should == '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6'
+    end
+  end
+
+  describe 'unfold_bottom_class' do
+    it 'should return empty string when bottom line shouldnt be unfolded' do
+      unfold_bottom_class(false).should == ''
+    end
+
+    it 'should return js class when bottom lines should be unfolded' do
+      unfold_bottom_class(true).should == 'js-unfold-bottom'
+    end
+  end
+
+  describe 'diff_line_content' do
+
+    it 'should return non breaking space when line is empty' do
+      diff_line_content(nil).should eq("  ")
+    end
+
+    it 'should return the line itself' do
+      diff_line_content(diff_file.diff_lines.first.text).should eq("@@ -6,12 +6,18 @@ module Popen")
+      diff_line_content(diff_file.diff_lines.first.type).should eq("match")
+      diff_line_content(diff_file.diff_lines.first.new_pos).should eq(6)
+    end
+  end
+
+  def parallel_diff_result_array
+    [
+      ["match", 6, "@@ -6,12 +6,18 @@ module Popen", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6", "match", 6, "@@ -6,12 +6,18 @@ module Popen"],
+      [nil, 6, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6", nil, 6, " "],
+      [nil, 7, "   def popen(cmd, path=nil)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7", nil, 7, "   def popen(cmd, path=nil)"],
+      [nil, 8, "     unless cmd.is_a?(Array)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8", nil, 8, "     unless cmd.is_a?(Array)"],
+      ["old", 9, "-      raise <span class='idiff'></span>&quot;System commands must be given as an array of strings&quot;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9", "new", 9, "+      raise <span class='idiff'>RuntimeError, </span>&quot;System commands must be given as an array of strings&quot;"],
+      [nil, 10, "     end", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10", nil, 10, "     end"], [nil, 11, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_11_11", nil, 11, " "],
+      [nil, 12, "     path ||= Dir.pwd", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_12_12", nil, 12, "     path ||= Dir.pwd"],
+      ["old", 13, "-    vars = { &quot;PWD&quot; =&gt; path }", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_13_13", "old", nil, "&nbsp;"],
+      ["old", 14, "-    options = { chdir: path }", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_13", "new", 13, "+"],
+      [nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14", "new", 14, "+    vars = {"],
+      [nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15", "new", 15, "+      &quot;PWD&quot; =&gt; path"],
+      [nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_16", "new", 16, "+    }"],
+      [nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_17", "new", 17, "+"],
+      [nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_18", "new", 18, "+    options = {"],
+      [nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_19", "new", 19, "+      chdir: path"],
+      [nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_20", "new", 20, "+    }"],
+      [nil, 15, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_21", nil, 21, " "],
+      [nil, 16, "     unless File.directory?(path)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_16_22", nil, 22, "     unless File.directory?(path)"],
+      [nil, 17, "       FileUtils.mkdir_p(path)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_17_23", nil, 23, "       FileUtils.mkdir_p(path)"],
+      ["match", 19, "@@ -19,6 +25,7 @@ module Popen", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25", "match", 25, "@@ -19,6 +25,7 @@ module Popen"],
+      [nil, 19, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25", nil, 25, " "], [nil, 20, "     @cmd_output = &quot;&quot;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_20_26", nil, 26, "     @cmd_output = &quot;&quot;"],
+      [nil, 21, "     @cmd_status = 0", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_21_27", nil, 27, "     @cmd_status = 0"],
+      [nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_28", "new", 28, "+"],
+      [nil, 22, "     Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_29", nil, 29, "     Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|"],
+      [nil, 23, "       @cmd_output &lt;&lt; stdout.read", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_23_30", nil, 30, "       @cmd_output &lt;&lt; stdout.read"],
+      [nil, 24, "       @cmd_output &lt;&lt; stderr.read", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_24_31", nil, 31, "       @cmd_output &lt;&lt; stderr.read"]
+    ]
+  end
+end

spec/lib/gitlab/diff/file_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Diff::File do
+  include RepoHelpers
+
+  let(:project) { create(:project) }
+  let(:commit) { project.repository.commit(sample_commit.id) }
+  let(:diff) { commit.diffs.first }
+  let(:diff_file) { Gitlab::Diff::File.new(diff) }
+
+  describe :diff_lines do
+    let(:diff_lines) { diff_file.diff_lines }
+
+    it { diff_lines.size.should == 30 }
+    it { diff_lines.first.should be_kind_of(Gitlab::Diff::Line) }
+  end
+
+  describe :mode_changed? do
+    it { diff_file.mode_changed?.should be_false }
+  end
+end

spec/lib/gitlab/diff/parser_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Diff::Parser do
+  include RepoHelpers
+
+  let(:project) { create(:project) }
+  let(:commit) { project.repository.commit(sample_commit.id) }
+  let(:diff) { commit.diffs.first }
+  let(:parser) { Gitlab::Diff::Parser.new }
+
+  describe :parse do
+    let(:diff) do
+      <<eos
+--- a/files/ruby/popen.rb
++++ b/files/ruby/popen.rb
+@@ -6,12 +6,18 @@ module Popen
+
+   def popen(cmd, path=nil)
+     unless cmd.is_a?(Array)
+-      raise "System commands must be given as an array of strings"
++      raise RuntimeError, "System commands must be given as an array of strings"
+     end
+
+     path ||= Dir.pwd
+-    vars = { "PWD" => path }
+-    options = { chdir: path }
++
++    vars = {
++      "PWD" => path
++    }
++
++    options = {
++      chdir: path
++    }
+
+     unless File.directory?(path)
+       FileUtils.mkdir_p(path)
+@@ -19,6 +25,7 @@ module Popen
+
+     @cmd_output = ""
+     @cmd_status = 0
++
+     Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|
+       @cmd_output << stdout.read
+       @cmd_output << stderr.read
+eos
+    end
+
+    before do
+      @lines = parser.parse(diff.lines)
+    end
+
+    it { @lines.size.should == 30 }
+
+    describe 'lines' do
+      describe 'first line' do
+        let(:line) { @lines.first }
+
+        it { line.type.should == 'match' }
+        it { line.old_pos.should == 6 }
+        it { line.new_pos.should == 6 }
+        it { line.text.should == '@@ -6,12 +6,18 @@ module Popen' }
+      end
+
+      describe 'removal line' do
+        let(:line) { @lines[10] }
+
+        it { line.type.should == 'old' }
+        it { line.old_pos.should == 14 }
+        it { line.new_pos.should == 13 }
+        it { line.text.should == '-    options = { chdir: path }' }
+      end
+
+      describe 'addition line' do
+        let(:line) { @lines[16] }
+
+        it { line.type.should == 'new' }
+        it { line.old_pos.should == 15 }
+        it { line.new_pos.should == 18 }
+        it { line.text.should == '+    options = {' }
+      end
+
+      describe 'unchanged line' do
+        let(:line) { @lines.last }
+
+        it { line.type.should == nil }
+        it { line.old_pos.should == 24 }
+        it { line.new_pos.should == 31 }
+        it { line.text.should == '       @cmd_output &lt;&lt; stderr.read' }
+      end
+    end
+  end
+end