Commit 4a48fd27 authored by Vasilii Iakliushin's avatar Vasilii Iakliushin

Remove improved_merge_diff_highlighting feature flag

Introduced in
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52499
FF issue: https://gitlab.com/gitlab-org/gitlab/-/issues/299884

* Remove deprecated diff detection logic
parent c19699f1
---
title: Remove improved_merge_diff_highlighting feature flag
merge_request: 55771
author:
type: added
---
name: improved_merge_diff_highlighting
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52499
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/299884
milestone: '13.9'
type: development
group: group::source code
default_enabled: true
...@@ -3,13 +3,12 @@ ...@@ -3,13 +3,12 @@
module Gitlab module Gitlab
module Diff module Diff
class Highlight class Highlight
attr_reader :diff_file, :diff_lines, :raw_lines, :repository, :project attr_reader :diff_file, :diff_lines, :raw_lines, :repository
delegate :old_path, :new_path, :old_sha, :new_sha, to: :diff_file, prefix: :diff delegate :old_path, :new_path, :old_sha, :new_sha, to: :diff_file, prefix: :diff
def initialize(diff_lines, repository: nil) def initialize(diff_lines, repository: nil)
@repository = repository @repository = repository
@project = repository&.project
if diff_lines.is_a?(Gitlab::Diff::File) if diff_lines.is_a?(Gitlab::Diff::File)
@diff_file = diff_lines @diff_file = diff_lines
...@@ -67,7 +66,7 @@ module Gitlab ...@@ -67,7 +66,7 @@ module Gitlab
end end
def inline_diffs def inline_diffs
@inline_diffs ||= InlineDiff.for_lines(@raw_lines, project: project) @inline_diffs ||= InlineDiff.for_lines(@raw_lines)
end end
def old_lines def old_lines
......
...@@ -7,8 +7,7 @@ module Gitlab ...@@ -7,8 +7,7 @@ module Gitlab
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
EXPIRATION = 1.week EXPIRATION = 1.week
VERSION = 1 VERSION = 2
NEXT_VERSION = 2
delegate :diffable, to: :@diff_collection delegate :diffable, to: :@diff_collection
delegate :diff_options, to: :@diff_collection delegate :diff_options, to: :@diff_collection
...@@ -70,20 +69,12 @@ module Gitlab ...@@ -70,20 +69,12 @@ module Gitlab
def key def key
strong_memoize(:redis_key) do strong_memoize(:redis_key) do
['highlighted-diff-files', diffable.cache_key, version, diff_options].join(":") ['highlighted-diff-files', diffable.cache_key, VERSION, diff_options].join(":")
end end
end end
private private
def version
if Feature.enabled?(:improved_merge_diff_highlighting, diffable.project, default_enabled: :yaml)
NEXT_VERSION
else
VERSION
end
end
def set_highlighted_diff_lines(diff_file, content) def set_highlighted_diff_lines(diff_file, content)
diff_file.highlighted_diff_lines = content.map do |line| diff_file.highlighted_diff_lines = content.map do |line|
Gitlab::Diff::Line.safe_init_from_hash(line) Gitlab::Diff::Line.safe_init_from_hash(line)
......
...@@ -11,19 +11,15 @@ module Gitlab ...@@ -11,19 +11,15 @@ module Gitlab
@offset = offset @offset = offset
end end
def inline_diffs(project: nil) def inline_diffs
# Skip inline diff if empty line was replaced with content # Skip inline diff if empty line was replaced with content
return if old_line == "" return if old_line == ""
if Feature.enabled?(:improved_merge_diff_highlighting, project, default_enabled: :yaml) CharDiff.new(old_line, new_line).changed_ranges(offset: offset)
CharDiff.new(old_line, new_line).changed_ranges(offset: offset)
else
deprecated_diff
end
end end
class << self class << self
def for_lines(lines, project: nil) def for_lines(lines)
pair_selector = Gitlab::Diff::PairSelector.new(lines) pair_selector = Gitlab::Diff::PairSelector.new(lines)
inline_diffs = [] inline_diffs = []
...@@ -32,7 +28,7 @@ module Gitlab ...@@ -32,7 +28,7 @@ module Gitlab
old_line = lines[old_index] old_line = lines[old_index]
new_line = lines[new_index] new_line = lines[new_index]
old_diffs, new_diffs = new(old_line, new_line, offset: 1).inline_diffs(project: project) old_diffs, new_diffs = new(old_line, new_line, offset: 1).inline_diffs
inline_diffs[old_index] = old_diffs inline_diffs[old_index] = old_diffs
inline_diffs[new_index] = new_diffs inline_diffs[new_index] = new_diffs
...@@ -41,46 +37,6 @@ module Gitlab ...@@ -41,46 +37,6 @@ module Gitlab
inline_diffs inline_diffs
end end
end end
private
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/299884
def deprecated_diff
lcp = longest_common_prefix(old_line, new_line)
lcs = longest_common_suffix(old_line[lcp..-1], new_line[lcp..-1])
lcp += offset
old_length = old_line.length + offset
new_length = new_line.length + offset
old_diff_range = lcp..(old_length - lcs - 1)
new_diff_range = lcp..(new_length - lcs - 1)
old_diffs = [old_diff_range] if old_diff_range.begin <= old_diff_range.end
new_diffs = [new_diff_range] if new_diff_range.begin <= new_diff_range.end
[old_diffs, new_diffs]
end
def longest_common_prefix(a, b) # rubocop:disable Naming/UncommunicativeMethodParamName
max_length = [a.length, b.length].max
length = 0
(0..max_length - 1).each do |pos|
old_char = a[pos]
new_char = b[pos]
break if old_char != new_char
length += 1
end
length
end
def longest_common_suffix(a, b) # rubocop:disable Naming/UncommunicativeMethodParamName
longest_common_prefix(a.reverse, b.reverse)
end
end end
end end
end end
...@@ -237,18 +237,8 @@ RSpec.describe Gitlab::Diff::HighlightCache, :clean_gitlab_redis_cache do ...@@ -237,18 +237,8 @@ RSpec.describe Gitlab::Diff::HighlightCache, :clean_gitlab_redis_cache do
describe '#key' do describe '#key' do
subject { cache.key } subject { cache.key }
it 'returns the next version of the cache' do it 'returns cache key' do
is_expected.to start_with("highlighted-diff-files:#{cache.diffable.cache_key}:2") is_expected.to start_with("highlighted-diff-files:#{cache.diffable.cache_key}:2")
end end
context 'when feature flag is disabled' do
before do
stub_feature_flags(improved_merge_diff_highlighting: false)
end
it 'returns the original version of the cache' do
is_expected.to start_with("highlighted-diff-files:#{cache.diffable.cache_key}:1")
end
end
end end
end end
...@@ -52,17 +52,6 @@ RSpec.describe Gitlab::Diff::InlineDiff do ...@@ -52,17 +52,6 @@ RSpec.describe Gitlab::Diff::InlineDiff do
expect(subject[0]).to eq([3..6]) expect(subject[0]).to eq([3..6])
expect(subject[1]).to eq([3..3, 17..22]) expect(subject[1]).to eq([3..3, 17..22])
end end
context 'when feature flag is disabled' do
before do
stub_feature_flags(improved_merge_diff_highlighting: false)
end
it 'finds all inline diffs' do
expect(subject[0]).to eq([3..19])
expect(subject[1]).to eq([3..22])
end
end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment