Actually render old and new sections of parallel diff next to each otherbetter-parallel-diff

4 files changed, 65 insertions, 103 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 3e4a10bb5a3..ac8f5de855c 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -33,6 +33,7 @@ v 8.10.0 (unreleased)
   - Fix user creation with stronger minimum password requirements !4054 (nathan-pmt)
   - Only show New Snippet button to users that can create snippets.
   - PipelinesFinder uses git cache data
+  - Actually render old and new sections of parallel diff next to each other
   - Throttle the update of `project.pushes_since_gc` to 1 minute.
   - Check for conflicts with existing Project's wiki path when creating a new project.
   - Show last push widget in upstream after push to fork
diff --git a/features/steps/shared/diff_note.rb b/features/steps/shared/diff_note.rb
index 56ef44ec969..2f20d8bd6da 100644
--- a/features/steps/shared/diff_note.rb
+++ b/features/steps/shared/diff_note.rb
@@ -32,8 +32,8 @@ module SharedDiffNote
   end
 
   step 'I leave a diff comment in a parallel view on the left side like "Old comment"' do
-    click_parallel_diff_line(sample_commit.line_code, 'old')
-    page.within("#{diff_file_selector} form[data-line-code='#{sample_commit.line_code}']") do
+    click_parallel_diff_line(sample_commit.del_line_code, 'old')
+    page.within("#{diff_file_selector} form[data-line-code='#{sample_commit.del_line_code}']") do
       fill_in "note[note]", with: "Old comment"
       find(".js-comment-button").trigger("click")
     end
diff --git a/lib/gitlab/diff/parallel_diff.rb b/lib/gitlab/diff/parallel_diff.rb
index 1c1fc148123..b069afdd28c 100644
--- a/lib/gitlab/diff/parallel_diff.rb
+++ b/lib/gitlab/diff/parallel_diff.rb
@@ -8,95 +8,78 @@ module Gitlab
       end
 
       def parallelize
-        lines = []
-        skip_next = false
 
+        i = 0
+        free_right_index = nil
+
+        lines = []
         highlighted_diff_lines = diff_file.highlighted_diff_lines
         highlighted_diff_lines.each do |line|
-          full_line = line.text
-          type = line.type
           line_code = diff_file.line_code(line)
-          line_new = line.new_pos
-          line_old = line.old_pos
           position = diff_file.position(line)
 
-          next_line = diff_file.next_line(line.index)
-
-          if next_line
-            next_line = highlighted_diff_lines[next_line.index]
-            full_next_line = next_line.text
-            next_line_code = diff_file.line_code(next_line)
-            next_type = next_line.type
-            next_position = diff_file.position(next_line)
-          end
-
-          case type
+          case line.type
           when 'match', nil
             # line in the right panel is the same as in the left one
             lines << {
               left: {
-                type:       type,
-                number:     line_old,
-                text:       full_line,
+                type:       line.type,
+                number:     line.old_pos,
+                text:       line.text,
                 line_code:  line_code,
                 position:   position
               },
               right: {
-                type:       type,
-                number:     line_new,
-                text:       full_line,
+                type:       line.type,
+                number:     line.new_pos,
+                text:       line.text,
                 line_code:  line_code,
                 position:   position
               }
             }
+
+            free_right_index = nil
+            i += 1
           when 'old'
-            case next_type
-            when 'new'
-              # Left side has text removed, right side has text added
-              lines << {
-                left: {
-                  type:       type,
-                  number:     line_old,
-                  text:       full_line,
-                  line_code:  line_code,
-                  position:   position
-                },
-                right: {
-                  type:       next_type,
-                  number:     line_new,
-                  text:       full_next_line,
-                  line_code:  next_line_code,
-                  position:   next_position,
-                }
-              }
-              skip_next = true
-            when 'old', 'nonewline', 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
-              lines << {
-                left: {
-                  type:       type,
-                  number:     line_old,
-                  text:       full_line,
-                  line_code:  line_code,
-                  position:   position
-                },
-                right: {
-                  type:       next_type,
-                  number:     nil,
-                  text:       "",
-                  line_code:  nil,
-                  position:   nil
-                }
+            lines << {
+              left: {
+                type:       line.type,
+                number:     line.old_pos,
+                text:       line.text,
+                line_code:  line_code,
+                position:   position
+              },
+              right: {
+                type:       nil,
+                number:     nil,
+                text:       "",
+                line_code:  line_code,
+                position:   position
               }
-            end
+            }
+
+            # Once we come upon a new line it can be put on the right of this old line
+            free_right_index ||= i
+            i += 1
           when 'new'
-            if skip_next
-              # Change has been already included in previous line so no need to do it again
-              skip_next = false
-              next
+            data = {
+              type:       line.type,
+              number:     line.new_pos,
+              text:       line.text,
+              line_code:  line_code,
+              position:   position
+            }
+
+            if free_right_index
+              # If an old line came before this without a line on the right, this
+              # line can be put to the right of it.
+              lines[free_right_index][:right] = data
+
+              # If there are any other old lines on the left that don't yet have
+              # a new counterpart on the right, update the free_right_index
+              next_free_right_index = free_right_index + 1
+              free_right_index = next_free_right_index < i ? next_free_right_index : nil
             else
-              # Change is only on the right side, left side has no change
               lines << {
                 left: {
                   type:       nil,
@@ -105,17 +88,15 @@ module Gitlab
                   line_code:  line_code,
                   position:   position
                 },
-                right: {
-                  type:       type,
-                  number:     line_new,
-                  text:       full_line,
-                  line_code:  line_code,
-                  position:   position
-                }
+                right: data
               }
+
+              free_right_index = nil
+              i += 1
             end
           end
         end
+
         lines
       end
     end
diff --git a/spec/fixtures/parallel_diff_result.yml b/spec/fixtures/parallel_diff_result.yml
index 7d01183e3ef..333eda1191a 100644
--- a/spec/fixtures/parallel_diff_result.yml
+++ b/spec/fixtures/parallel_diff_result.yml
@@ -253,27 +253,6 @@
         :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
         :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
   :right:
-    :type: old
-    :number:
-    :text: ''
-    :line_code:
-    :position:
-- :left:
-    :type: old
-    :number: 14
-    :text: |
-      -<span id="LC14" class="line">    <span class="n">options</span> <span class="o">=</span> <span class="p">{</span> <span class="ss">chdir: </span><span class="n">path</span> <span class="p">}</span></span>
-    :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_13
-    :position: !ruby/object:Gitlab::Diff::Position
-      attributes:
-        :old_path: files/ruby/popen.rb
-        :new_path: files/ruby/popen.rb
-        :old_line: 14
-        :new_line:
-        :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
-        :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
-        :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
-  :right:
     :type: new
     :number: 13
     :text: |
@@ -289,16 +268,17 @@
         :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
         :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
 - :left:
-    :type:
-    :number:
-    :text: ''
-    :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14
+    :type: old
+    :number: 14
+    :text: |
+      -<span id="LC14" class="line">    <span class="n">options</span> <span class="o">=</span> <span class="p">{</span> <span class="ss">chdir: </span><span class="n">path</span> <span class="p">}</span></span>
+    :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_13
     :position: !ruby/object:Gitlab::Diff::Position
       attributes:
         :old_path: files/ruby/popen.rb
         :new_path: files/ruby/popen.rb
-        :old_line:
-        :new_line: 14
+        :old_line: 14
+        :new_line:
         :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
         :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
         :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d