From 545a85dc6487d80c3bc64df85f43765937ca3c86 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sun, 10 Jul 2016 23:48:44 -0500 Subject: [PATCH] Actually render old and new sections of parallel diff next to each other --- CHANGELOG | 1 + features/steps/shared/diff_note.rb | 4 +- lib/gitlab/diff/parallel_diff.rb | 129 +++++++++++-------------- spec/fixtures/parallel_diff_result.yml | 34 ++----- 4 files changed, 65 insertions(+), 103 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 3e4a10bb5a3..ac8f5de855c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -33,6 +33,7 @@ v 8.10.0 (unreleased) - Fix user creation with stronger minimum password requirements !4054 (nathan-pmt) - Only show New Snippet button to users that can create snippets. - PipelinesFinder uses git cache data + - Actually render old and new sections of parallel diff next to each other - Throttle the update of `project.pushes_since_gc` to 1 minute. - Check for conflicts with existing Project's wiki path when creating a new project. - Show last push widget in upstream after push to fork diff --git a/features/steps/shared/diff_note.rb b/features/steps/shared/diff_note.rb index 56ef44ec969..2f20d8bd6da 100644 --- a/features/steps/shared/diff_note.rb +++ b/features/steps/shared/diff_note.rb @@ -32,8 +32,8 @@ module SharedDiffNote end step 'I leave a diff comment in a parallel view on the left side like "Old comment"' do - click_parallel_diff_line(sample_commit.line_code, 'old') - page.within("#{diff_file_selector} form[data-line-code='#{sample_commit.line_code}']") do + click_parallel_diff_line(sample_commit.del_line_code, 'old') + page.within("#{diff_file_selector} form[data-line-code='#{sample_commit.del_line_code}']") do fill_in "note[note]", with: "Old comment" find(".js-comment-button").trigger("click") end diff --git a/lib/gitlab/diff/parallel_diff.rb b/lib/gitlab/diff/parallel_diff.rb index 1c1fc148123..b069afdd28c 100644 --- a/lib/gitlab/diff/parallel_diff.rb +++ b/lib/gitlab/diff/parallel_diff.rb @@ -8,95 +8,78 @@ module Gitlab end def parallelize - lines = [] - skip_next = false + i = 0 + free_right_index = nil + + lines = [] highlighted_diff_lines = diff_file.highlighted_diff_lines highlighted_diff_lines.each do |line| - full_line = line.text - type = line.type line_code = diff_file.line_code(line) - line_new = line.new_pos - line_old = line.old_pos position = diff_file.position(line) - next_line = diff_file.next_line(line.index) - - if next_line - next_line = highlighted_diff_lines[next_line.index] - full_next_line = next_line.text - next_line_code = diff_file.line_code(next_line) - next_type = next_line.type - next_position = diff_file.position(next_line) - end - - case type + case line.type when 'match', nil # line in the right panel is the same as in the left one lines << { left: { - type: type, - number: line_old, - text: full_line, + type: line.type, + number: line.old_pos, + text: line.text, line_code: line_code, position: position }, right: { - type: type, - number: line_new, - text: full_line, + type: line.type, + number: line.new_pos, + text: line.text, line_code: line_code, position: position } } + + free_right_index = nil + i += 1 when 'old' - case next_type - when 'new' - # Left side has text removed, right side has text added - lines << { - left: { - type: type, - number: line_old, - text: full_line, - line_code: line_code, - position: position - }, - right: { - type: next_type, - number: line_new, - text: full_next_line, - line_code: next_line_code, - position: next_position, - } + lines << { + left: { + type: line.type, + number: line.old_pos, + text: line.text, + line_code: line_code, + position: position + }, + right: { + type: nil, + number: nil, + text: "", + line_code: line_code, + position: position } - skip_next = true - when 'old', 'nonewline', nil - # Left side has text removed, right side doesn't have any change - # No next line code, no new line number, no new line text - lines << { - left: { - type: type, - number: line_old, - text: full_line, - line_code: line_code, - position: position - }, - right: { - type: next_type, - number: nil, - text: "", - line_code: nil, - position: nil - } - } - end + } + + # Once we come upon a new line it can be put on the right of this old line + free_right_index ||= i + i += 1 when 'new' - if skip_next - # Change has been already included in previous line so no need to do it again - skip_next = false - next + data = { + type: line.type, + number: line.new_pos, + text: line.text, + line_code: line_code, + position: position + } + + if free_right_index + # If an old line came before this without a line on the right, this + # line can be put to the right of it. + lines[free_right_index][:right] = data + + # If there are any other old lines on the left that don't yet have + # a new counterpart on the right, update the free_right_index + next_free_right_index = free_right_index + 1 + free_right_index = next_free_right_index < i ? next_free_right_index : nil else - # Change is only on the right side, left side has no change lines << { left: { type: nil, @@ -105,17 +88,15 @@ module Gitlab line_code: line_code, position: position }, - right: { - type: type, - number: line_new, - text: full_line, - line_code: line_code, - position: position - } + right: data } + + free_right_index = nil + i += 1 end end end + lines end end diff --git a/spec/fixtures/parallel_diff_result.yml b/spec/fixtures/parallel_diff_result.yml index 7d01183e3ef..333eda1191a 100644 --- a/spec/fixtures/parallel_diff_result.yml +++ b/spec/fixtures/parallel_diff_result.yml @@ -252,27 +252,6 @@ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: old - :number: - :text: '' - :line_code: - :position: -- :left: - :type: old - :number: 14 - :text: | - - options = { chdir: path } - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_13 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 14 - :new_line: - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: new :number: 13 @@ -289,16 +268,17 @@ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: - :type: - :number: - :text: '' - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14 + :type: old + :number: 14 + :text: | + - options = { chdir: path } + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_13 :position: !ruby/object:Gitlab::Diff::Position attributes: :old_path: files/ruby/popen.rb :new_path: files/ruby/popen.rb - :old_line: - :new_line: 14 + :old_line: 14 + :new_line: :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d