From 33cd9c3545f984d970ba6e730325089478553123 Mon Sep 17 00:00:00 2001 From: Vasilii Iakliushin Date: Mon, 1 Mar 2021 18:10:56 +0100 Subject: [PATCH] Extract PairSelector logic into a class Contributes to https://gitlab.com/gitlab-org/gitlab/-/issues/16950 It is an intermediate step in Gitlab::Diff logic refactoring. --- lib/gitlab/diff/inline_diff.rb | 48 +------------ lib/gitlab/diff/pair_selector.rb | 58 +++++++++++++++ spec/lib/gitlab/diff/pair_selector_spec.rb | 84 ++++++++++++++++++++++ 3 files changed, 144 insertions(+), 46 deletions(-) create mode 100644 lib/gitlab/diff/pair_selector.rb create mode 100644 spec/lib/gitlab/diff/pair_selector_spec.rb diff --git a/lib/gitlab/diff/inline_diff.rb b/lib/gitlab/diff/inline_diff.rb index cf769262958e5d..1d00f502d1c469 100644 --- a/lib/gitlab/diff/inline_diff.rb +++ b/lib/gitlab/diff/inline_diff.rb @@ -3,22 +3,6 @@ module Gitlab module Diff class InlineDiff - # Regex to find a run of deleted lines followed by the same number of added lines - LINE_PAIRS_PATTERN = %r{ - # Runs start at the beginning of the string (the first line) or after a space (for an unchanged line) - (?:\A|\s) - - # This matches a number of `-`s followed by the same number of `+`s through recursion - (? - - - \g? - \+ - ) - - # Runs end at the end of the string (the last line) or before a space (for an unchanged line) - (?=\s|\z) - }x.freeze - attr_accessor :old_line, :new_line, :offset def initialize(old_line, new_line, offset: 0) @@ -40,11 +24,11 @@ def inline_diffs(project: nil) class << self def for_lines(lines, project: nil) - changed_line_pairs = find_changed_line_pairs(lines) + pair_selector = Gitlab::Diff::PairSelector.new(lines) inline_diffs = [] - changed_line_pairs.each do |old_index, new_index| + pair_selector.each do |old_index, new_index| old_line = lines[old_index] new_line = lines[new_index] @@ -56,34 +40,6 @@ def for_lines(lines, project: nil) inline_diffs end - - private - - # Finds pairs of old/new line pairs that represent the same line that changed - # rubocop: disable CodeReuse/ActiveRecord - def find_changed_line_pairs(lines) - # Prefixes of all diff lines, indicating their types - # For example: `" - + -+ ---+++ --+ -++"` - line_prefixes = lines.each_with_object(+"") { |line, s| s << (line[0] || ' ') }.gsub(/[^ +-]/, ' ') - - changed_line_pairs = [] - line_prefixes.scan(LINE_PAIRS_PATTERN) do - # For `"---+++"`, `begin_index == 0`, `end_index == 6` - begin_index, end_index = Regexp.last_match.offset(:del_ins) - - # For `"---+++"`, `changed_line_count == 3` - changed_line_count = (end_index - begin_index) / 2 - - halfway_index = begin_index + changed_line_count - (begin_index...halfway_index).each do |i| - # For `"---+++"`, index 1 maps to 1 + 3 = 4 - changed_line_pairs << [i, i + changed_line_count] - end - end - - changed_line_pairs - end - # rubocop: enable CodeReuse/ActiveRecord end private diff --git a/lib/gitlab/diff/pair_selector.rb b/lib/gitlab/diff/pair_selector.rb new file mode 100644 index 00000000000000..2e5ee3a736329b --- /dev/null +++ b/lib/gitlab/diff/pair_selector.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +module Gitlab + module Diff + class PairSelector + include Enumerable + + # Regex to find a run of deleted lines followed by the same number of added lines + # rubocop: disable Lint/MixedRegexpCaptureTypes + LINE_PAIRS_PATTERN = %r{ + # Runs start at the beginning of the string (the first line) or after a space (for an unchanged line) + (?:\A|\s) + + # This matches a number of `-`s followed by the same number of `+`s through recursion + (? + - + \g? + \+ + ) + + # Runs end at the end of the string (the last line) or before a space (for an unchanged line) + (?=\s|\z) + }x.freeze + # rubocop: enable Lint/MixedRegexpCaptureTypes + + def initialize(lines) + @lines = lines + end + + # Finds pairs of old/new line pairs that represent the same line that changed + # rubocop: disable CodeReuse/ActiveRecord + def each + # Prefixes of all diff lines, indicating their types + # For example: `" - + -+ ---+++ --+ -++"` + line_prefixes = lines.each_with_object(+"") { |line, s| s << (line[0] || ' ') }.gsub(/[^ +-]/, ' ') + + line_prefixes.scan(LINE_PAIRS_PATTERN) do + # For `"---+++"`, `begin_index == 0`, `end_index == 6` + begin_index, end_index = Regexp.last_match.offset(:del_ins) + + # For `"---+++"`, `changed_line_count == 3` + changed_line_count = (end_index - begin_index) / 2 + + halfway_index = begin_index + changed_line_count + (begin_index...halfway_index).each do |i| + # For `"---+++"`, index 1 maps to 1 + 3 = 4 + yield [i, i + changed_line_count] + end + end + end + # rubocop: enable CodeReuse/ActiveRecord + + private + + attr_reader :lines + end + end +end diff --git a/spec/lib/gitlab/diff/pair_selector_spec.rb b/spec/lib/gitlab/diff/pair_selector_spec.rb new file mode 100644 index 00000000000000..da5707bc377fb3 --- /dev/null +++ b/spec/lib/gitlab/diff/pair_selector_spec.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +RSpec.describe Gitlab::Diff::PairSelector do + subject(:selector) { described_class.new(lines) } + + describe '#to_a' do + subject { selector.to_a } + + let(:lines) { diff.lines } + + let(:diff) do + <<-EOF.strip_heredoc + class Test # 0 + - def initialize(test = true) # 1 + + def initialize(test = false) # 2 + @test = test # 3 + - if true # 4 + - @foo = "bar" # 5 + + unless false # 6 + + @foo = "baz" # 7 + end + end + end + EOF + end + + it 'finds all pairs' do + is_expected.to match_array([[1, 2], [4, 6], [5, 7]]) + end + + context 'when there are empty lines' do + let(:lines) { ['- bar', '+ baz', ''] } + + it { expect { subject }.not_to raise_error } + end + + context 'when there are only removals' do + let(:diff) do + <<-EOF.strip_heredoc + - class Test + - def initialize(test = true) + - end + - end + EOF + end + + it 'returns empty collection' do + is_expected.to eq([]) + end + end + + context 'when there are only additions' do + let(:diff) do + <<-EOF.strip_heredoc + + class Test + + def initialize(test = true) + + end + + end + EOF + end + + it 'returns empty collection' do + is_expected.to eq([]) + end + end + + context 'when there are no changes' do + let(:diff) do + <<-EOF.strip_heredoc + class Test + def initialize(test = true) + end + end + EOF + end + + it 'returns empty collection' do + is_expected.to eq([]) + end + end + end +end -- GitLab