From 5e8aca215243d2eaacb0a1b744909af2b7264a32 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Mon, 19 Jun 2017 11:25:21 -0500 Subject: [PATCH] Don't display comment on unchanged line on both sides in parallel diff --- app/helpers/diff_helper.rb | 4 ++-- app/models/legacy_diff_note.rb | 2 +- lib/gitlab/diff/line.rb | 20 ++++++++++---------- lib/gitlab/diff/parallel_diff.rb | 20 ++++++++++---------- spec/helpers/diff_helper_spec.rb | 11 ++++++++++- 5 files changed, 33 insertions(+), 24 deletions(-) diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 06822747d11..16a99addd0b 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -66,12 +66,12 @@ module DiffHelper discussions_left = discussions_right = nil - if left && (left.unchanged? || left.discussable?) + if left && left.discussable? && (left.unchanged? || left.removed?) line_code = diff_file.line_code(left) discussions_left = @grouped_diff_discussions[line_code] end - if right&.discussable? + if right && right.discussable? && right.added? line_code = diff_file.line_code(right) discussions_right = @grouped_diff_discussions[line_code] end diff --git a/app/models/legacy_diff_note.rb b/app/models/legacy_diff_note.rb index 7126de2d488..2d5909ab25e 100644 --- a/app/models/legacy_diff_note.rb +++ b/app/models/legacy_diff_note.rb @@ -42,7 +42,7 @@ class LegacyDiffNote < Note end def for_line?(line) - !line.meta? && diff_file.line_code(line) == self.line_code + line.discussable? && diff_file.line_code(line) == self.line_code end def original_line_code diff --git a/lib/gitlab/diff/line.rb b/lib/gitlab/diff/line.rb index bd52ae47e9f..2d89ccfc354 100644 --- a/lib/gitlab/diff/line.rb +++ b/lib/gitlab/diff/line.rb @@ -42,25 +42,25 @@ module Gitlab end def added? - type == 'new' || type == 'new-nonewline' + %w[new new-nonewline].include?(type) end def removed? - type == 'old' || type == 'old-nonewline' - end - - def rich_text - @parent_file.highlight_lines! if @parent_file && !@rich_text - - @rich_text + %w[old old-nonewline].include?(type) end def meta? - type == 'match' + %w[match new-nonewline old-nonewline].include?(type) end def discussable? - !['match', 'new-nonewline', 'old-nonewline'].include?(type) + !meta? + end + + def rich_text + @parent_file.highlight_lines! if @parent_file && !@rich_text + + @rich_text end def as_json(opts = nil) diff --git a/lib/gitlab/diff/parallel_diff.rb b/lib/gitlab/diff/parallel_diff.rb index 481536a380b..0cb26fa45c8 100644 --- a/lib/gitlab/diff/parallel_diff.rb +++ b/lib/gitlab/diff/parallel_diff.rb @@ -14,16 +14,7 @@ module Gitlab lines = [] highlighted_diff_lines = diff_file.highlighted_diff_lines highlighted_diff_lines.each do |line| - if line.meta? || line.unchanged? - # line in the right panel is the same as in the left one - lines << { - left: line, - right: line - } - - free_right_index = nil - i += 1 - elsif line.removed? + if line.removed? lines << { left: line, right: nil @@ -51,6 +42,15 @@ module Gitlab free_right_index = nil i += 1 end + elsif line.meta? || line.unchanged? + # line in the right panel is the same as in the left one + lines << { + left: line, + right: line + } + + free_right_index = nil + i += 1 end end diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb index 0ac030d3171..0d909e6e140 100644 --- a/spec/helpers/diff_helper_spec.rb +++ b/spec/helpers/diff_helper_spec.rb @@ -148,12 +148,21 @@ describe DiffHelper do it 'puts comments on added lines' do left = Gitlab::Diff::Line.new('\\nonewline', 'old-nonewline', 3, 3, 3) - right = Gitlab::Diff::Line.new('new line', 'add', 3, 3, 3) + right = Gitlab::Diff::Line.new('new line', 'new', 3, 3, 3) result = helper.parallel_diff_discussions(left, right, diff_file) expect(result).to eq([nil, 'comment']) end + + it 'puts comments on unchanged lines' do + left = Gitlab::Diff::Line.new('unchanged line', nil, 3, 3, 3) + right = Gitlab::Diff::Line.new('unchanged line', nil, 3, 3, 3) + + result = helper.parallel_diff_discussions(left, right, diff_file) + + expect(result).to eq(['comment', nil]) + end end describe "#diff_match_line" do -- GitLab