Extract generic parts of Gitlab::Diff::InlineDiffMarker

app/services/system_note_service.rb

@@ -291,8 +291,8 @@ module SystemNoteService
 
     old_diffs, new_diffs = Gitlab::Diff::InlineDiff.new(old_title, new_title).inline_diffs
 
-    marked_old_title = Gitlab::Diff::InlineDiffMarker.new(old_title).mark(old_diffs, mode: :deletion, markdown: true)
+    marked_old_title = Gitlab::Diff::InlineDiffMarkdownMarker.new(old_title).mark(old_diffs, mode: :deletion)
-    marked_new_title = Gitlab::Diff::InlineDiffMarker.new(new_title).mark(new_diffs, mode: :addition, markdown: true)
+    marked_new_title = Gitlab::Diff::InlineDiffMarkdownMarker.new(new_title).mark(new_diffs, mode: :addition)
 
     body = "changed title from **#{marked_old_title}** to **#{marked_new_title}**"
 

lib/gitlab/diff/inline_diff_markdown_marker.rb

@@ -0,0 +1,17 @@
+module Gitlab
+  module Diff
+    class InlineDiffMarkdownMarker < Gitlab::StringRangeMarker
+      MARKDOWN_SYMBOLS = {
+        addition: "+",
+        deletion: "-"
+      }.freeze
+
+      def mark(line_inline_diffs, mode: nil)
+        super(line_inline_diffs) do |text, left:, right:|
+          symbol = MARKDOWN_SYMBOLS[mode]
+          "{#{symbol}#{text}#{symbol}}"
+        end
+      end
+    end
+  end
+end

lib/gitlab/diff/inline_diff_marker.rb

@@ -1,137 +1,21 @@
 module Gitlab
   module Diff
-    class InlineDiffMarker
+    class InlineDiffMarker < Gitlab::StringRangeMarker
-      MARKDOWN_SYMBOLS = {
+      def mark(line_inline_diffs, mode: nil)
-        addition: "+",
+        super(line_inline_diffs) do |text, left:, right:|
-        deletion: "-"
+          %{<span class="#{html_class_names(left, right, mode)}">#{text}</span>}
-      }.freeze
-
-      attr_accessor :raw_line, :rich_line
-
-      def initialize(raw_line, rich_line = raw_line)
-        @raw_line = raw_line
-        @rich_line = ERB::Util.html_escape(rich_line)
-      end
-
-      def mark(line_inline_diffs, mode: nil, markdown: false)
-        return rich_line unless line_inline_diffs
-
-        marker_ranges = []
-        line_inline_diffs.each do |inline_diff_range|
-          # Map the inline-diff range based on the raw line to character positions in the rich line
-          inline_diff_positions = position_mapping[inline_diff_range].flatten
-          # Turn the array of character positions into ranges
-          marker_ranges.concat(collapse_ranges(inline_diff_positions))
         end
-
-        offset = 0
-
-        # Mark each range
-        marker_ranges.each_with_index do |range, index|
-          before_content =
-            if markdown
-              "{#{MARKDOWN_SYMBOLS[mode]}"
-            else
-              "<span class='#{html_class_names(marker_ranges, mode, index)}'>"
-            end
-          after_content =
-            if markdown
-              "#{MARKDOWN_SYMBOLS[mode]}}"
-            else
-              "</span>"
-            end
-          offset = insert_around_range(rich_line, range, before_content, after_content, offset)
-        end
-
-        rich_line.html_safe
       end
 
       private
 
-      def html_class_names(marker_ranges, mode, index)
+      def html_class_names(left, right, mode)
         class_names = ["idiff"]
-        class_names << "left"  if index == 0
+        class_names << "left"  if left
-        class_names << "right" if index == marker_ranges.length - 1
+        class_names << "right" if right
         class_names << mode if mode
         class_names.join(" ")
       end
-
-      # Mapping of character positions in the raw line, to the rich (highlighted) line
-      def position_mapping
-        @position_mapping ||= begin
-          mapping = []
-          rich_pos = 0
-          (0..raw_line.length).each do |raw_pos|
-            rich_char = rich_line[rich_pos]
-
-            # The raw and rich lines are the same except for HTML tags,
-            # so skip over any `<...>` segment
-            while rich_char == '<'
-              until rich_char == '>'
-                rich_pos += 1
-                rich_char = rich_line[rich_pos]
-              end
-
-              rich_pos += 1
-              rich_char = rich_line[rich_pos]
-            end
-
-            # multi-char HTML entities in the rich line correspond to a single character in the raw line
-            if rich_char == '&'
-              multichar_mapping = [rich_pos]
-              until rich_char == ';'
-                rich_pos += 1
-                multichar_mapping << rich_pos
-                rich_char = rich_line[rich_pos]
-              end
-
-              mapping[raw_pos] = multichar_mapping
-            else
-              mapping[raw_pos] = rich_pos
-            end
-
-            rich_pos += 1
-          end
-
-          mapping
-        end
-      end
-
-      # Takes an array of integers, and returns an array of ranges covering the same integers
-      def collapse_ranges(positions)
-        return [] if positions.empty?
-        ranges = []
-
-        start = prev = positions[0]
-        range = start..prev
-        positions[1..-1].each do |pos|
-          if pos == prev + 1
-            range = start..pos
-            prev = pos
-          else
-            ranges << range
-            start = prev = pos
-            range = start..prev
-          end
-        end
-        ranges << range
-
-        ranges
-      end
-
-      # Inserts tags around the characters identified by the given range
-      def insert_around_range(text, range, before, after, offset = 0)
-        # Just to be sure
-        return offset if offset + range.end + 1 > text.length
-
-        text.insert(offset + range.begin, before)
-        offset += before.length
-
-        text.insert(offset + range.end + 1, after)
-        offset += after.length
-
-        offset
-      end
     end
   end
 end

lib/gitlab/string_range_marker.rb

@@ -0,0 +1,102 @@
+module Gitlab
+  class StringRangeMarker
+    attr_accessor :raw_line, :rich_line
+
+    def initialize(raw_line, rich_line = raw_line)
+      @raw_line = raw_line
+      @rich_line = ERB::Util.html_escape(rich_line)
+    end
+
+    def mark(marker_ranges)
+      return rich_line unless marker_ranges
+
+      rich_marker_ranges = []
+      marker_ranges.each do |range|
+        # Map the inline-diff range based on the raw line to character positions in the rich line
+        rich_positions = position_mapping[range].flatten
+        # Turn the array of character positions into ranges
+        rich_marker_ranges.concat(collapse_ranges(rich_positions))
+      end
+
+      offset = 0
+      # Mark each range
+      rich_marker_ranges.each_with_index do |range, i|
+        offset_range = (range.begin + offset)..(range.end + offset)
+        original_text = rich_line[offset_range]
+
+        text = yield(original_text, left: i == 0, right: i == rich_marker_ranges.length - 1)
+
+        rich_line[offset_range] = text
+
+        offset += text.length - original_text.length
+      end
+
+      rich_line.html_safe
+    end
+
+    private
+
+    # Mapping of character positions in the raw line, to the rich (highlighted) line
+    def position_mapping
+      @position_mapping ||= begin
+        mapping = []
+        rich_pos = 0
+        (0..raw_line.length).each do |raw_pos|
+          rich_char = rich_line[rich_pos]
+
+          # The raw and rich lines are the same except for HTML tags,
+          # so skip over any `<...>` segment
+          while rich_char == '<'
+            until rich_char == '>'
+              rich_pos += 1
+              rich_char = rich_line[rich_pos]
+            end
+
+            rich_pos += 1
+            rich_char = rich_line[rich_pos]
+          end
+
+          # multi-char HTML entities in the rich line correspond to a single character in the raw line
+          if rich_char == '&'
+            multichar_mapping = [rich_pos]
+            until rich_char == ';'
+              rich_pos += 1
+              multichar_mapping << rich_pos
+              rich_char = rich_line[rich_pos]
+            end
+
+            mapping[raw_pos] = multichar_mapping
+          else
+            mapping[raw_pos] = rich_pos
+          end
+
+          rich_pos += 1
+        end
+
+        mapping
+      end
+    end
+
+    # Takes an array of integers, and returns an array of ranges covering the same integers
+    def collapse_ranges(positions)
+      return [] if positions.empty?
+      ranges = []
+
+      start = prev = positions[0]
+      range = start..prev
+      positions[1..-1].each do |pos|
+        if pos == prev + 1
+          range = start..pos
+          prev = pos
+        else
+          ranges << range
+          start = prev = pos
+          range = start..prev
+        end
+      end
+      ranges << range
+
+      ranges
+    end
+  end
+end

spec/lib/gitlab/diff/inline_diff_markdown_marker_spec.rb

@@ -0,0 +1,14 @@
+require 'spec_helper'
+
+describe Gitlab::Diff::InlineDiffMarkdownMarker, lib: true do
+  describe '#mark' do
+    let(:raw) { "abc 'def'" }
+    let(:inline_diffs) { [2..5] }
+    let(:subject) { described_class.new(raw).mark(inline_diffs, mode: :deletion) }
+
+    it 'marks the range' do
+      expect(subject).to eq("ab{-c 'd-}ef'")
+      expect(subject).to be_html_safe
+    end
+  end
+end

spec/lib/gitlab/diff/inline_diff_marker_spec.rb

@@ -1,26 +1,26 @@
 require 'spec_helper'
 
 describe Gitlab::Diff::InlineDiffMarker, lib: true do
-  describe '#inline_diffs' do
+  describe '#mark' do
     context "when the rich text is html safe" do
-      let(:raw)  { "abc 'def'" }
+      let(:raw) { "abc 'def'" }
       let(:rich) { %{<span class="abc">abc</span><span class="space"> </span><span class="def">&#39;def&#39;</span>}.html_safe }
       let(:inline_diffs) { [2..5] }
-      let(:subject) { Gitlab::Diff::InlineDiffMarker.new(raw, rich).mark(inline_diffs) }
+      let(:subject) { described_class.new(raw, rich).mark(inline_diffs) }
 
-      it 'marks the inline diffs' do
+      it 'marks the range' do
-        expect(subject).to eq(%{<span class="abc">ab<span class='idiff left'>c</span></span><span class="space"><span class='idiff'> </span></span><span class="def"><span class='idiff right'>&#39;d</span>ef&#39;</span>})
+        expect(subject).to eq(%{<span class="abc">ab<span class="idiff left">c</span></span><span class="space"><span class="idiff"> </span></span><span class="def"><span class="idiff right">&#39;d</span>ef&#39;</span>})
         expect(subject).to be_html_safe
       end
     end
 
     context "when the text text is not html safe" do
-      let(:raw)  { "abc 'def'" }
+      let(:raw) { "abc 'def'" }
       let(:inline_diffs) { [2..5] }
-      let(:subject) { Gitlab::Diff::InlineDiffMarker.new(raw).mark(inline_diffs) }
+      let(:subject) { described_class.new(raw).mark(inline_diffs) }
 
-      it 'marks the inline diffs' do
+      it 'marks the range' do
-        expect(subject).to eq(%{ab<span class='idiff left right'>c &#39;d</span>ef&#39;})
+        expect(subject).to eq(%{ab<span class="idiff left right">c &#39;d</span>ef&#39;})
         expect(subject).to be_html_safe
       end
     end

spec/lib/gitlab/string_range_marker_spec.rb

@@ -0,0 +1,36 @@
+require 'spec_helper'
+
+describe Gitlab::StringRangeMarker, lib: true do
+  describe '#mark' do
+    context "when the rich text is html safe" do
+      let(:raw)  { "abc <def>" }
+      let(:rich) { %{<span class="abc">abc</span><span class="space"> </span><span class="def">&lt;def&gt;</span>}.html_safe }
+      let(:inline_diffs) { [2..5] }
+      let(:subject) do
+        described_class.new(raw, rich).mark(inline_diffs) do |text, left:, right:|
+          "LEFT#{text}RIGHT"
+        end
+      end
+
+      it 'marks the inline diffs' do
+        expect(subject).to eq(%{<span class="abc">abLEFTcRIGHT</span><span class="space">LEFT RIGHT</span><span class="def">LEFT&lt;dRIGHTef&gt;</span>})
+        expect(subject).to be_html_safe
+      end
+    end
+
+    context "when the rich text is not html safe" do
+      let(:raw)  { "abc <def>" }
+      let(:inline_diffs) { [2..5] }
+      let(:subject) do
+        described_class.new(raw).mark(inline_diffs) do |text, left:, right:|
+          "LEFT#{text}RIGHT"
+        end
+      end
+
+      it 'marks the inline diffs' do
+        expect(subject).to eq(%{abLEFTc &lt;dRIGHTef&gt;})
+        expect(subject).to be_html_safe
+      end
+    end
+  end
+end