Improve lines counting for match headersosw-improve-lines-counting-for-match-headers

2 files changed, 140 insertions, 13 deletions

diff --git a/lib/gitlab/diff/lines_unfolder.rb b/lib/gitlab/diff/lines_unfolder.rb
index 9306b7e16a2..3da193139e9 100644
--- a/lib/gitlab/diff/lines_unfolder.rb
+++ b/lib/gitlab/diff/lines_unfolder.rb
@@ -122,7 +122,7 @@ module Gitlab
           old_pos = from_blob_line
           new_pos = from_blob_line + offset
 
-          build_match_line(old_pos, new_pos)
+          build_match_line(old_pos: old_pos, new_pos: new_pos)
         end
       end
 
@@ -133,19 +133,37 @@ module Gitlab
           next if bottom?
           next unless @generate_bottom_match_line
 
-          position = line_after_unfold_position.old_pos
+          line_after_unfold = line_after(unfold_line)
+          position = line_after_unfold.old_pos
 
+          # The following calculations exists in order to generate the correct
+          # match line header, e.g.:
+          # @@ +<old_pos>,<old_lines_length> -<new_pos>,<new_lines_length> @@
           old_pos = position
           new_pos = position + offset
 
-          build_match_line(old_pos, new_pos)
+          next_match = next_match_line(line_after_unfold)
+          line_before_next_match = line_before(next_match) || last_line
+
+          old_lines_length = (line_before_next_match.old_pos - old_pos) + 1
+          new_lines_length = (line_before_next_match.new_pos - new_pos) + 1
+
+          build_match_line(old_pos: old_pos,
+                           new_pos: new_pos,
+                           old_lines_length: old_lines_length,
+                           new_lines_length: new_lines_length)
         end
       end
 
-      def build_match_line(old_pos, new_pos)
+      def build_match_line(old_pos:, new_pos:, old_lines_length: nil, new_lines_length: nil)
         blob_lines_length = blob_lines.length
-        old_line_ref = [old_pos, blob_lines_length].join(',')
-        new_line_ref = [new_pos, blob_lines_length].join(',')
+
+        old_lines_length ||= blob_lines_length
+        new_lines_length ||= blob_lines_length
+
+        old_line_ref = [old_pos, old_lines_length].join(',')
+        new_line_ref = [new_pos, new_lines_length].join(',')
+
         new_match_line_str = "@@ -#{old_line_ref}+#{new_line_ref} @@"
 
         Gitlab::Diff::Line.new(new_match_line_str, 'match', nil, old_pos, new_pos)
@@ -160,7 +178,7 @@ module Gitlab
 
         # There's no line before the match if it's in the top-most
         # position.
-        prev_line_number = line_before_unfold_position&.old_pos || 0
+        prev_line_number = line_before(unfold_line)&.old_pos || 0
 
         if from <= prev_line_number + 1
           @generate_top_match_line = false
@@ -179,7 +197,7 @@ module Gitlab
 
         return to if bottom?
 
-        next_line_number = line_after_unfold_position.old_pos
+        next_line_number = line_after(unfold_line).old_pos
 
         if to >= next_line_number - 1
           @generate_bottom_match_line = false
@@ -193,18 +211,22 @@ module Gitlab
         unfold_line.new_pos - unfold_line.old_pos
       end
 
-      def line_before_unfold_position
-        return unless index = unfold_line&.index
+      def line_before(line)
+        return unless index = line&.index
 
         @diff_file.diff_lines[index - 1] if index > 0
       end
 
-      def line_after_unfold_position
-        return unless index = unfold_line&.index
+      def line_after(line)
+        return unless index = line&.index
 
         @diff_file.diff_lines[index + 1] if index >= 0
       end
 
+      def next_match_line(line)
+        @diff_file.diff_lines.find { |l| l.index > line.index && l.type == 'match' }
+      end
+
       def bottom?
         strong_memoize(:bottom) do
           @position.old_line > last_line.old_pos
diff --git a/spec/lib/gitlab/diff/lines_unfolder_spec.rb b/spec/lib/gitlab/diff/lines_unfolder_spec.rb
index 8e00c8e0e30..88bf458eaa4 100644
--- a/spec/lib/gitlab/diff/lines_unfolder_spec.rb
+++ b/spec/lib/gitlab/diff/lines_unfolder_spec.rb
@@ -273,6 +273,7 @@ describe Gitlab::Diff::LinesUnfolder do
          # Second match line
          [62, 59, "@@ -62,7+59,7 @@"],
 
+         # Remaining diff
          [62, 59, "         },"],
          [63, 60, "         {"],
          [64, 61, "             \"name\": \"gnome-desktop\","],
@@ -373,7 +374,7 @@ describe Gitlab::Diff::LinesUnfolder do
          # end
 
          # Second match line
-         [62, 59, "@@ -62,4+59,4 @@"],
+         [62, 59, "@@ -62,7+59,7 @@"],
 
          [62, 59, "         },"],
          [63, 60, "         {"],
@@ -524,6 +525,108 @@ describe Gitlab::Diff::LinesUnfolder do
     end
   end
 
+  context 'position requires a middle expansion and no top match line' do
+    let(:position) do
+      Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19",
+                                 start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19",
+                                 head_sha: "1487062132228de836236c522fe52fed4980a46c",
+                                 old_path: "build-aux/flatpak/org.gnome.Nautilus.json",
+                                 new_path: "build-aux/flatpak/org.gnome.Nautilus.json",
+                                 position_type:  "text",
+                                 old_line: 69,
+                                 new_line: 66)
+    end
+
+    context 'blob lines' do
+      let(:expected_blob_lines) do
+        [[69, 69, "                     \"url\": \"https://git.gnome.org/browse/gnome-desktop\""],
+         [70, 70, "                 }"],
+         [71, 71, "             ]"],
+         [72, 72, "         },"]]
+      end
+
+      it 'returns the extracted blob lines correctly' do
+        extracted_lines = subject.blob_lines
+
+        expect(extracted_lines.size).to eq(4)
+
+        extracted_lines.each_with_index do |line, i|
+          expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i])
+        end
+      end
+    end
+
+    context 'diff lines' do
+      let(:expected_diff_lines) do
+        [[7, 7, "@@ -7,9 +7,6 @@"],
+         [7, 7, "     \"tags\": [\"devel\", \"development\", \"nightly\"],"],
+         [8, 8, "     \"desktop-file-name-prefix\": \"(Development) \","],
+         [9, 9, "     \"finish-args\": ["],
+         [10, 10, "-        \"--share=ipc\", \"--socket=x11\","],
+         [11, 10, "-        \"--socket=wayland\","],
+         [12, 10, "-        \"--talk-name=org.gnome.OnlineAccounts\","],
+         [13, 10, "         \"--talk-name=org.freedesktop.Tracker1\","],
+         [14, 11, "         \"--filesystem=home\","],
+         [15, 12, "         \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","],
+         [62, 59, "@@ -62,7 +59,7 @@"],
+         [62, 59, "         },"],
+         [63, 60, "         {"],
+         [64, 61, "             \"name\": \"gnome-desktop\","],
+         [65, 62, "-            \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"],
+         [66, 62, "+            \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"],
+         [66, 63, "             \"sources\": ["],
+         [67, 64, "                 {"],
+         [68, 65, "                     \"type\": \"git\","],
+         # No new match line
+
+         # Injected blob lines
+         [69, 66, "                     \"url\": \"https://git.gnome.org/browse/gnome-desktop\""],
+         [70, 67, "                 }"],
+         [71, 68, "             ]"],
+         [72, 69, "         },"],
+         # end
+
+         # New second match line
+         [83, 80, "@@ -83,11+80,6 @@"],
+
+         [83, 80, "             \"buildsystem\": \"meson\","],
+         [84, 81, "             \"builddir\": true,"],
+         [85, 82, "             \"name\": \"nautilus\","],
+         [86, 83, "-            \"config-opts\": ["],
+         [87, 83, "-                \"-Denable-desktop=false\","],
+         [88, 83, "-                \"-Denable-selinux=false\","],
+         [89, 83, "-                \"--libdir=/app/lib\""],
+         [90, 83, "-            ],"],
+         [91, 83, "             \"sources\": ["],
+         [92, 84, "                 {"],
+         [93, 85, "                     \"type\": \"git\","]]
+      end
+
+      it 'return merge of blob lines with diff lines correctly' do
+        new_diff_lines = subject.unfolded_diff_lines
+
+        expected_diff_lines.each_with_index do |expected_line, i|
+          line = new_diff_lines[i]
+
+          expect([line.old_pos, line.new_pos, line.text])
+            .to eq([expected_line[0], expected_line[1], expected_line[2]])
+        end
+      end
+
+      it 'merged lines have correct line codes' do
+        new_diff_lines = subject.unfolded_diff_lines
+
+        new_diff_lines.each_with_index do |line, i|
+          old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1]
+
+          unless line.type == 'match'
+            expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos))
+          end
+        end
+      end
+    end
+  end
+
   context 'position requires a short top expansion' do
     let(:position) do
       Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19",
@@ -566,6 +669,8 @@ describe Gitlab::Diff::LinesUnfolder do
          [5, 5, "     \"sdk\": \"org.gnome.Sdk\","],
          [6, 6, "     \"command\": \"nautilus\","],
          # end
+
+         # No new second match line
          [7, 7, "     \"tags\": [\"devel\", \"development\", \"nightly\"],"],
          [8, 8, "     \"desktop-file-name-prefix\": \"(Development) \","],
          [9, 9, "     \"finish-args\": ["],