diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb
index aa65ba95c2e79d0b7784f87e42c5883ee667afdf..10c7b4032cf938d64b9f542007cc3e2283b57423 100644
--- a/app/helpers/diff_helper.rb
+++ b/app/helpers/diff_helper.rb
@@ -4,8 +4,8 @@ module DiffHelper
def mark_inline_diffs(old_line, new_line)
old_diffs, new_diffs = Gitlab::Diff::InlineDiff.new(old_line, new_line).inline_diffs
- marked_old_line = Gitlab::Diff::InlineDiffMarker.new(old_line).mark(old_diffs, mode: :deletion)
- marked_new_line = Gitlab::Diff::InlineDiffMarker.new(new_line).mark(new_diffs, mode: :addition)
+ marked_old_line = Gitlab::Diff::InlineDiffMarker.new(old_line).mark(old_diffs)
+ marked_new_line = Gitlab::Diff::InlineDiffMarker.new(new_line).mark(new_diffs)
[marked_old_line, marked_new_line]
end
diff --git a/app/services/system_notes/issuables_service.rb b/app/services/system_notes/issuables_service.rb
index 69008c702d51b8f5d151bb8ec6281ec9d73d8549..60dd56e772ace7faa6cece34fa72a853dc6c172f 100644
--- a/app/services/system_notes/issuables_service.rb
+++ b/app/services/system_notes/issuables_service.rb
@@ -125,8 +125,8 @@ def change_title(old_title)
old_diffs, new_diffs = Gitlab::Diff::InlineDiff.new(old_title, new_title).inline_diffs
- marked_old_title = Gitlab::Diff::InlineDiffMarkdownMarker.new(old_title).mark(old_diffs, mode: :deletion)
- marked_new_title = Gitlab::Diff::InlineDiffMarkdownMarker.new(new_title).mark(new_diffs, mode: :addition)
+ marked_old_title = Gitlab::Diff::InlineDiffMarkdownMarker.new(old_title).mark(old_diffs)
+ marked_new_title = Gitlab::Diff::InlineDiffMarkdownMarker.new(new_title).mark(new_diffs)
body = "changed title from **#{marked_old_title}** to **#{marked_new_title}**"
diff --git a/config/feature_flags/development/introduce_marker_ranges.yml b/config/feature_flags/development/introduce_marker_ranges.yml
new file mode 100644
index 0000000000000000000000000000000000000000..de59cf0e906767d24279001ae4255a0bf0f4b8b8
--- /dev/null
+++ b/config/feature_flags/development/introduce_marker_ranges.yml
@@ -0,0 +1,8 @@
+---
+name: introduce_marker_ranges
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55669
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/324068
+milestone: '13.10'
+type: development
+group: group::source code
+default_enabled: false
diff --git a/lib/banzai/filter/autolink_filter.rb b/lib/banzai/filter/autolink_filter.rb
index 0aa1ee8f604c62f86bbfb8dd8761d32a71b9271f..d569711431c27b8b5e72ba5a2e1d29937337c3f5 100644
--- a/lib/banzai/filter/autolink_filter.rb
+++ b/lib/banzai/filter/autolink_filter.rb
@@ -120,7 +120,7 @@ def autolink_match(match)
end
def autolink_filter(text)
- Gitlab::StringRegexMarker.new(CGI.unescapeHTML(text), text.html_safe).mark(LINK_PATTERN) do |link, left:, right:|
+ Gitlab::StringRegexMarker.new(CGI.unescapeHTML(text), text.html_safe).mark(LINK_PATTERN) do |link, left:, right:, mode:|
autolink_match(link).html_safe
end
end
diff --git a/lib/banzai/filter/spaced_link_filter.rb b/lib/banzai/filter/spaced_link_filter.rb
index 101b55a49e40904f4d169cb77157fce437e7b070..f52ffe117d9b389f291d04f03908508243780150 100644
--- a/lib/banzai/filter/spaced_link_filter.rb
+++ b/lib/banzai/filter/spaced_link_filter.rb
@@ -76,7 +76,7 @@ def spaced_link_match(link)
end
def spaced_link_filter(text)
- Gitlab::StringRegexMarker.new(CGI.unescapeHTML(text), text.html_safe).mark(LINK_OR_IMAGE_PATTERN) do |link, left:, right:|
+ Gitlab::StringRegexMarker.new(CGI.unescapeHTML(text), text.html_safe).mark(LINK_OR_IMAGE_PATTERN) do |link, left:, right:, mode:|
spaced_link_match(link).html_safe
end
end
diff --git a/lib/gitlab/dependency_linker/base_linker.rb b/lib/gitlab/dependency_linker/base_linker.rb
index d735fb55652d91902fbeaa82575069b2758ac4c3..36a840372c5a266ec093961ad63d0edc3a6938a7 100644
--- a/lib/gitlab/dependency_linker/base_linker.rb
+++ b/lib/gitlab/dependency_linker/base_linker.rb
@@ -80,7 +80,7 @@ def link_regex(regex, &url_proc)
highlighted_lines.map!.with_index do |rich_line, i|
marker = StringRegexMarker.new((plain_lines[i].chomp! || plain_lines[i]), rich_line.html_safe)
- marker.mark(regex, group: :name) do |text, left:, right:|
+ marker.mark(regex, group: :name) do |text, left:, right:, mode:|
url = yield(text)
url ? link_tag(text, url) : text
end
diff --git a/lib/gitlab/dependency_linker/go_mod_linker.rb b/lib/gitlab/dependency_linker/go_mod_linker.rb
index 4d6fe366333e3bc55a5755bbb3e76b797ef4b7ea..fae4ee23383de8f3047d2bcfbe68d00ee4d05e59 100644
--- a/lib/gitlab/dependency_linker/go_mod_linker.rb
+++ b/lib/gitlab/dependency_linker/go_mod_linker.rb
@@ -22,7 +22,7 @@ def link_dependencies
i, j = match.offset(:name)
marker = StringRangeMarker.new(plain_line, rich_line.html_safe)
- marker.mark([i..(j - 1)]) do |text, left:, right:|
+ marker.mark([i..(j - 1)]) do |text, left:, right:, mode:|
url = package_url(text, match[:version])
url ? link_tag(text, url) : text
end
diff --git a/lib/gitlab/dependency_linker/go_sum_linker.rb b/lib/gitlab/dependency_linker/go_sum_linker.rb
index 20dc82ede9f45a761be15522dec5bfd362f3dd10..44826332f66de30a0a2c5ed2061efa39093cc884 100644
--- a/lib/gitlab/dependency_linker/go_sum_linker.rb
+++ b/lib/gitlab/dependency_linker/go_sum_linker.rb
@@ -21,7 +21,7 @@ def link_dependencies
i2, j2 = match.offset(:checksum)
marker = StringRangeMarker.new(plain_line, rich_line.html_safe)
- marker.mark([i0..(j0 - 1), i2..(j2 - 1)]) do |text, left:, right:|
+ marker.mark([i0..(j0 - 1), i2..(j2 - 1)]) do |text, left:, right:, mode:|
if left
url = package_url(text, match[:version])
url ? link_tag(text, url) : text
diff --git a/lib/gitlab/diff/char_diff.rb b/lib/gitlab/diff/char_diff.rb
index c8bb39e9f5dedeed7e13e25fceb933ef0e82d973..1b3af8f75ca543c7133423bb040ef172b2200469 100644
--- a/lib/gitlab/diff/char_diff.rb
+++ b/lib/gitlab/diff/char_diff.rb
@@ -32,12 +32,12 @@ def changed_ranges(offset: 0)
end
if action == :delete
- old_diffs << (old_pointer..(old_pointer + content_size - 1))
+ old_diffs << MarkerRange.new(old_pointer, old_pointer + content_size - 1, mode: MarkerRange::DELETION)
old_pointer += content_size
end
if action == :insert
- new_diffs << (new_pointer..(new_pointer + content_size - 1))
+ new_diffs << MarkerRange.new(new_pointer, new_pointer + content_size - 1, mode: MarkerRange::ADDITION)
new_pointer += content_size
end
end
diff --git a/lib/gitlab/diff/highlight.rb b/lib/gitlab/diff/highlight.rb
index a525907934523277b13ed708adbd05e66d21dddd..baa46e7e30608c7e887aecf618ab0267486f97f9 100644
--- a/lib/gitlab/diff/highlight.rb
+++ b/lib/gitlab/diff/highlight.rb
@@ -3,12 +3,13 @@
module Gitlab
module Diff
class Highlight
- attr_reader :diff_file, :diff_lines, :raw_lines, :repository
+ attr_reader :diff_file, :diff_lines, :raw_lines, :repository, :project
delegate :old_path, :new_path, :old_sha, :new_sha, to: :diff_file, prefix: :diff
def initialize(diff_lines, repository: nil)
@repository = repository
+ @project = repository&.project
if diff_lines.is_a?(Gitlab::Diff::File)
@diff_file = diff_lines
@@ -30,6 +31,12 @@ def highlight
if line_inline_diffs = inline_diffs[i]
begin
+ # MarkerRange objects are converted to Ranges to keep the previous behavior
+ # Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/324068
+ if Feature.disabled?(:introduce_marker_ranges, project, default_enabled: :yaml)
+ line_inline_diffs = line_inline_diffs.map { |marker_range| marker_range.to_range }
+ end
+
rich_line = InlineDiffMarker.new(diff_line.text, rich_line).mark(line_inline_diffs)
# This should only happen when the encoding of the diff doesn't
# match the blob, which is a bug. But we shouldn't fail to render
diff --git a/lib/gitlab/diff/highlight_cache.rb b/lib/gitlab/diff/highlight_cache.rb
index e0f1f8a1e589284d343cb18b728694c884da1913..c5e9bfdc321baa4056eaf16b7e6f14382fcc75e7 100644
--- a/lib/gitlab/diff/highlight_cache.rb
+++ b/lib/gitlab/diff/highlight_cache.rb
@@ -69,7 +69,12 @@ def clear
def key
strong_memoize(:redis_key) do
- ['highlighted-diff-files', diffable.cache_key, VERSION, diff_options].join(":")
+ [
+ 'highlighted-diff-files',
+ diffable.cache_key, VERSION,
+ diff_options,
+ Feature.enabled?(:introduce_marker_ranges, diffable.project, default_enabled: :yaml)
+ ].join(":")
end
end
diff --git a/lib/gitlab/diff/inline_diff_markdown_marker.rb b/lib/gitlab/diff/inline_diff_markdown_marker.rb
index 3c536c43a9e302b86ed42f0b1f2d72849220b938..d8d596ebce71e7b7c52d6ebb07165326fea4caa6 100644
--- a/lib/gitlab/diff/inline_diff_markdown_marker.rb
+++ b/lib/gitlab/diff/inline_diff_markdown_marker.rb
@@ -8,8 +8,8 @@ class InlineDiffMarkdownMarker < Gitlab::StringRangeMarker
deletion: "-"
}.freeze
- def mark(line_inline_diffs, mode: nil)
- super(line_inline_diffs) do |text, left:, right:|
+ def mark(line_inline_diffs)
+ super(line_inline_diffs) do |text, left:, right:, mode:|
symbol = MARKDOWN_SYMBOLS[mode]
"{#{symbol}#{text}#{symbol}}"
end
diff --git a/lib/gitlab/diff/inline_diff_marker.rb b/lib/gitlab/diff/inline_diff_marker.rb
index 29dff699ba51e531885c26b4adcea3fb771eb9ba..c8cc1c0e649b34411538fd432e0a3155c4f93d9b 100644
--- a/lib/gitlab/diff/inline_diff_marker.rb
+++ b/lib/gitlab/diff/inline_diff_marker.rb
@@ -7,8 +7,8 @@ def initialize(line, rich_line = nil)
super(line, rich_line || line)
end
- def mark(line_inline_diffs, mode: nil)
- super(line_inline_diffs) do |text, left:, right:|
+ def mark(line_inline_diffs)
+ super(line_inline_diffs) do |text, left:, right:, mode:|
%{#{text}}.html_safe
end
end
diff --git a/lib/gitlab/marker_range.rb b/lib/gitlab/marker_range.rb
new file mode 100644
index 0000000000000000000000000000000000000000..50a59adebdf1eafa27b4c6507fa2a9aeb20d3b6a
--- /dev/null
+++ b/lib/gitlab/marker_range.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+# It is a Range object extended with `mode` attribute
+# MarkerRange not only keeps information about changed characters, but also
+# the type of changes
+module Gitlab
+ class MarkerRange < Range
+ DELETION = :deletion
+ ADDITION = :addition
+
+ # Converts Range object to MarkerRange class
+ def self.from_range(range)
+ return range if range.is_a?(self)
+
+ new(range.begin, range.end, exclude_end: range.exclude_end?)
+ end
+
+ def initialize(first, last, exclude_end: false, mode: nil)
+ super(first, last, exclude_end)
+ @mode = mode
+ end
+
+ def to_range
+ Range.new(self.begin, self.end, self.exclude_end?)
+ end
+
+ attr_reader :mode
+ end
+end
diff --git a/lib/gitlab/string_range_marker.rb b/lib/gitlab/string_range_marker.rb
index 780fe4c77255bd2b67d1d9bbe6c3f7f264a7e417..5ddc88edf5036d58a50c31fbca0ea00ad1470c9a 100644
--- a/lib/gitlab/string_range_marker.rb
+++ b/lib/gitlab/string_range_marker.rb
@@ -15,8 +15,10 @@ def initialize(raw_line, rich_line = nil)
end
end
- def mark(marker_ranges)
- return rich_line unless marker_ranges&.any?
+ def mark(ranges)
+ return rich_line unless ranges&.any?
+
+ marker_ranges = ranges.map { |range| Gitlab::MarkerRange.from_range(range) }
if html_escaped
rich_marker_ranges = []
@@ -24,7 +26,7 @@ def mark(marker_ranges)
# 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))
+ rich_marker_ranges.concat(collapse_ranges(rich_positions, range.mode))
end
else
rich_marker_ranges = marker_ranges
@@ -36,7 +38,7 @@ def mark(marker_ranges)
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)
+ text = yield(original_text, left: i == 0, right: i == rich_marker_ranges.length - 1, mode: range.mode)
rich_line[offset_range] = text
@@ -90,21 +92,21 @@ def position_mapping
end
# Takes an array of integers, and returns an array of ranges covering the same integers
- def collapse_ranges(positions)
+ def collapse_ranges(positions, mode)
return [] if positions.empty?
ranges = []
start = prev = positions[0]
- range = start..prev
+ range = MarkerRange.new(start, prev, mode: mode)
positions[1..-1].each do |pos|
if pos == prev + 1
- range = start..pos
+ range = MarkerRange.new(start, pos, mode: mode)
prev = pos
else
ranges << range
start = prev = pos
- range = start..prev
+ range = MarkerRange.new(start, prev, mode: mode)
end
end
ranges << range
diff --git a/spec/lib/gitlab/diff/highlight_cache_spec.rb b/spec/lib/gitlab/diff/highlight_cache_spec.rb
index efe8535ba3c8616a8b01180c5ee6e091c0551cc5..d26bc5fc9a856fe26b9a588238bdbe64bf49e7c5 100644
--- a/spec/lib/gitlab/diff/highlight_cache_spec.rb
+++ b/spec/lib/gitlab/diff/highlight_cache_spec.rb
@@ -238,7 +238,17 @@
subject { cache.key }
it 'returns cache key' do
- is_expected.to start_with("highlighted-diff-files:#{cache.diffable.cache_key}:2")
+ is_expected.to eq("highlighted-diff-files:#{cache.diffable.cache_key}:2:#{cache.diff_options}:true")
+ end
+
+ context 'when feature flag is disabled' do
+ before do
+ stub_feature_flags(introduce_marker_ranges: false)
+ end
+
+ it 'returns the original version of the cache' do
+ is_expected.to eq("highlighted-diff-files:#{cache.diffable.cache_key}:2:#{cache.diff_options}:false")
+ end
end
end
end
diff --git a/spec/lib/gitlab/diff/highlight_spec.rb b/spec/lib/gitlab/diff/highlight_spec.rb
index 283437e7fbd5461822f092667a8557d42ab2a885..e613674af3a0f2d487b3adc519941915532f78d6 100644
--- a/spec/lib/gitlab/diff/highlight_spec.rb
+++ b/spec/lib/gitlab/diff/highlight_spec.rb
@@ -50,11 +50,23 @@
end
it 'highlights and marks added lines' do
- code = %Q{+ raise RuntimeError, "System commands must be given as an array of strings"\n}
+ code = %Q{+ raise RuntimeError, "System commands must be given as an array of strings"\n}
expect(subject[5].rich_text).to eq(code)
end
+ context 'when introduce_marker_ranges is false' do
+ before do
+ stub_feature_flags(introduce_marker_ranges: false)
+ end
+
+ it 'keeps the old bevavior (without mode classes)' do
+ code = %Q{+ raise RuntimeError, "System commands must be given as an array of strings"\n}
+
+ expect(subject[5].rich_text).to eq(code)
+ end
+ end
+
context 'when no diff_refs' do
before do
allow(diff_file).to receive(:diff_refs).and_return(nil)
@@ -93,7 +105,7 @@
end
it 'marks added lines' do
- code = %q{+ raise RuntimeError, "System commands must be given as an array of strings"}
+ code = %q{+ raise RuntimeError, "System commands must be given as an array of strings"}
expect(subject[5].rich_text).to eq(code)
expect(subject[5].rich_text).to be_html_safe
diff --git a/spec/lib/gitlab/diff/inline_diff_markdown_marker_spec.rb b/spec/lib/gitlab/diff/inline_diff_markdown_marker_spec.rb
index 60f7f3a103f097b994c570aa3c2dec9a31a82760..3670074cc2143247935624c19da6380309aa7fd5 100644
--- a/spec/lib/gitlab/diff/inline_diff_markdown_marker_spec.rb
+++ b/spec/lib/gitlab/diff/inline_diff_markdown_marker_spec.rb
@@ -5,8 +5,8 @@
RSpec.describe Gitlab::Diff::InlineDiffMarkdownMarker do
describe '#mark' do
let(:raw) { "abc 'def'" }
- let(:inline_diffs) { [2..5] }
- let(:subject) { described_class.new(raw).mark(inline_diffs, mode: :deletion) }
+ let(:inline_diffs) { [Gitlab::MarkerRange.new(2, 5, mode: Gitlab::MarkerRange::DELETION)] }
+ let(:subject) { described_class.new(raw).mark(inline_diffs) }
it 'does not escape html etities and marks the range' do
expect(subject).to eq("ab{-c 'd-}ef'")
diff --git a/spec/lib/gitlab/marker_range_spec.rb b/spec/lib/gitlab/marker_range_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..5f73d2a5048ae04b3919d253a4bd27431b7ce03b
--- /dev/null
+++ b/spec/lib/gitlab/marker_range_spec.rb
@@ -0,0 +1,71 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+
+RSpec.describe Gitlab::MarkerRange do
+ subject(:marker_range) { described_class.new(first, last, mode: mode) }
+
+ let(:first) { 1 }
+ let(:last) { 10 }
+ let(:mode) { nil }
+
+ it { is_expected.to eq(first..last) }
+
+ it 'behaves like a Range' do
+ is_expected.to be_kind_of(Range)
+ end
+
+ describe '#mode' do
+ subject { marker_range.mode }
+
+ it { is_expected.to be_nil }
+
+ context 'when mode is provided' do
+ let(:mode) { :deletion }
+
+ it { is_expected.to eq(mode) }
+ end
+ end
+
+ describe '#to_range' do
+ subject { marker_range.to_range }
+
+ it { is_expected.to eq(first..last) }
+
+ context 'when mode is provided' do
+ let(:mode) { :deletion }
+
+ it 'is omitted during transformation' do
+ is_expected.not_to respond_to(:mode)
+ end
+ end
+ end
+
+ describe '.from_range' do
+ subject { described_class.from_range(range) }
+
+ let(:range) { 1..3 }
+
+ it 'converts Range to MarkerRange object' do
+ is_expected.to be_a(described_class)
+ end
+
+ it 'keeps correct range' do
+ is_expected.to eq(range)
+ end
+
+ context 'when range excludes end' do
+ let(:range) { 1...3 }
+
+ it 'keeps correct range' do
+ is_expected.to eq(range)
+ end
+ end
+
+ context 'when range is already a MarkerRange' do
+ let(:range) { marker_range }
+
+ it { is_expected.to be(marker_range) }
+ end
+ end
+end
diff --git a/spec/lib/gitlab/string_range_marker_spec.rb b/spec/lib/gitlab/string_range_marker_spec.rb
index 52fab6e3109efb03509c2ade0b40ec2bb84a87f2..6f63c8e2df4066e0dbc99a4bfb669399dcc05f87 100644
--- a/spec/lib/gitlab/string_range_marker_spec.rb
+++ b/spec/lib/gitlab/string_range_marker_spec.rb
@@ -8,7 +8,7 @@ def mark_diff(rich = nil)
raw = 'abc '
inline_diffs = [2..5]
- described_class.new(raw, rich).mark(inline_diffs) do |text, left:, right:|
+ described_class.new(raw, rich).mark(inline_diffs) do |text, left:, right:, mode:|
"LEFT#{text}RIGHT".html_safe
end
end
diff --git a/spec/lib/gitlab/string_regex_marker_spec.rb b/spec/lib/gitlab/string_regex_marker_spec.rb
index 2dadd222820d56df7433769bb9262b487ec2481c..a02be83558ca1104b4d9db8a7dac2f8579b6ff75 100644
--- a/spec/lib/gitlab/string_regex_marker_spec.rb
+++ b/spec/lib/gitlab/string_regex_marker_spec.rb
@@ -9,7 +9,7 @@
let(:rich) { %{"name": "AFNetworking"}.html_safe }
subject do
- described_class.new(raw, rich).mark(/"[^"]+":\s*"(?[^"]+)"/, group: :name) do |text, left:, right:|
+ described_class.new(raw, rich).mark(/"[^"]+":\s*"(?[^"]+)"/, group: :name) do |text, left:, right:, mode:|
%{#{text}}.html_safe
end
end
@@ -25,7 +25,7 @@
let(:rich) { %{a <b> <c> d}.html_safe }
subject do
- described_class.new(raw, rich).mark(/<[a-z]>/) do |text, left:, right:|
+ described_class.new(raw, rich).mark(/<[a-z]>/) do |text, left:, right:, mode:|
%{#{text}}.html_safe
end
end