home/gitlabhq
2
0
Fork 0
mirror of https://github.com/wahyd4/gitlabhq.git synced 2026-08-15 23:56:09 +10:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'implement_comments_on_parallel_diff' into 'master'

Browse Source 
Implement comments on parallel diff

Fixes #1565

See merge request !1090
This commit is contained in:
Dmitriy Zaporozhets
2014-09-15 07:19:50 +00:00
parent e19c1f71d3 9b59570c8e
commit 79190fe06b
7 changed files with 105 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/assets/stylesheets/sections/notes.scss
+3
View File
@@ -90,6 +90,9 @@ ul.notes {
border-width: 1px 0; border-width: 1px 0;
padding-top: 0; padding-top: 0;
vertical-align: top; vertical-align: top;
&.parallel{
border-width: 1px;
}
} }
} }
} }
app/helpers/diff_helper.rb
+28 -8
View File
@@ -36,7 +36,10 @@ module DiffHelper
# Building array of lines # Building array of lines
# #
# [left_type, left_line_number, left_line_content, line_code, right_line_type, right_line_number, right_line_content] # [
# left_type, left_line_number, left_line_content, left_line_code,
# right_line_type, right_line_number, right_line_content, right_line_code
# ]
# #
diff_file.diff_lines.each do |line| diff_file.diff_lines.each do |line|
@@ -49,26 +52,25 @@ module DiffHelper
next_line = diff_file.next_line(line.index) next_line = diff_file.next_line(line.index)
if next_line if next_line
next_line_code = generate_line_code(diff_file.file_path, next_line)
next_type = next_line.type next_type = next_line.type
next_line = next_line.text next_line = next_line.text
end end
line = [type, line_old, full_line, line_code, next_type, line_new]
if type == 'match' || type.nil? if type == 'match' || type.nil?
# line in the right panel is the same as in the left one # line in the right panel is the same as in the left one
line = [type, line_old, full_line, line_code, type, line_new, full_line] line = [type, line_old, full_line, line_code, type, line_new, full_line, line_code]
lines.push(line) lines.push(line)
elsif type == 'old' elsif type == 'old'
if next_type == 'new' if next_type == 'new'
# Left side has text removed, right side has text added # Left side has text removed, right side has text added
line.push(next_line) line = [type, line_old, full_line, line_code, next_type, line_new, next_line, next_line_code]
lines.push(line) lines.push(line)
skip_next = true skip_next = true
elsif next_type == 'old' || next_type.nil? elsif next_type == 'old' || next_type.nil?
# Left side has text removed, right side doesn't have any change # Left side has text removed, right side doesn't have any change
line.pop # remove the newline # No next line code, no new line number, no new line text
line.push(nil) # no line number on the right panel line = [type, line_old, full_line, line_code, next_type, nil, " ", nil]
line.push(" ") # empty line on the right panel
lines.push(line) lines.push(line)
end end
elsif type == 'new' elsif type == 'new'
@@ -78,7 +80,7 @@ module DiffHelper
next next
else else
# Change is only on the right side, left side has no change # Change is only on the right side, left side has no change
line = [nil, nil, " ", line_code, type, line_new, full_line] line = [nil, nil, " ", line_code, type, line_new, full_line, line_code]
lines.push(line) lines.push(line)
end end
end end
@@ -97,4 +99,22 @@ module DiffHelper
line line
end end
end end
def line_comments
@line_comments ||= @line_notes.group_by(&:line_code)
end
def organize_comments(type_left, type_right, line_code_left, line_code_right)
comments_left = comments_right = nil
unless type_left.nil? && type_right == 'new'
comments_left = line_comments[line_code_left]
end
unless type_left.nil? && type_right.nil?
comments_right = line_comments[line_code_right]
end
[comments_left, comments_right]
end
end end
app/views/projects/diffs/_parallel_view.html.haml
+22 -8
View File
@@ -5,22 +5,36 @@
- type_left = line[0] - type_left = line[0]
- line_number_left = line[1] - line_number_left = line[1]
- line_content_left = line[2] - line_content_left = line[2]
- line_code = line[3] - line_code_left = line[3]
- type_right = line[4] - type_right = line[4]
- line_number_right = line[5] - line_number_right = line[5]
- line_content_right = line[6] - line_content_right = line[6]
- line_code_right = line[7]
%tr.line_holder.parallel{id: line_code} %tr.line_holder.parallel
- if type_left == 'match' - if type_left == 'match'
= render "projects/diffs/match_line_parallel", { line: line_content_left, = render "projects/diffs/match_line_parallel", { line: line_content_left,
line_old: line_number_left, line_new: line_number_right } line_old: line_number_left, line_new: line_number_right }
- elsif type_left == 'old' || type_left.nil? - elsif type_left == 'old' || type_left.nil?
%td.old_line{class: "#{type_left}"} %td.old_line{id: line_code_left, class: "#{type_left}"}
= link_to raw(line_number_left), "##{line_code}", id: line_code = link_to raw(line_number_left), "##{line_code_left}", id: line_code_left
%td.line_content{class: "parallel noteable_line #{type_left} #{line_code}", "line_code" => line_code }= raw line_content_left %td.line_content{class: "parallel noteable_line #{type_left} #{line_code_left}", "line_code" => line_code_left }= raw line_content_left
%td.new_line{ class: "#{type_right == 'new' ? 'new' : nil}", data: { linenumber: line_number_right }}
= link_to raw(line_number_right), "##{line_code}", id: line_code - if type_right == 'new'
%td.line_content.parallel{class: "noteable_line #{type_right == 'new' ? 'new' : nil} #{line_code}", "line_code" => line_code}= raw line_content_right - new_line_class = 'new'
- new_line_code = line_code_right
- else
- new_line_class = nil
- new_line_code = line_code_left
%td.new_line{id: new_line_code, class: "#{new_line_class}", data: { linenumber: line_number_right }}
= link_to raw(line_number_right), "##{new_line_code}", id: new_line_code
%td.line_content.parallel{class: "noteable_line #{new_line_class} #{new_line_code}", "line_code" => new_line_code}= raw line_content_right
- if @reply_allowed
- comments_left, comments_right = organize_comments(type_left, type_right, line_code_left, line_code_right)
- if comments_left.present? || comments_right.present?
= render "projects/notes/diff_notes_with_reply_parallel", notes1: comments_left, notes2: comments_right
- if diff_file.diff.diff.blank? && diff_file.mode_changed? - if diff_file.diff.diff.blank? && diff_file.mode_changed?
.file-mode-changed .file-mode-changed
app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml
+5 -6
View File
@@ -1,14 +1,13 @@
- note1 = notes1.first # example note - note1 = notes1.present? ? notes1.first : nil
- note2 = notes2.first # example note - note2 = notes2.present? ? notes2.first : nil
-# Check if line want not changed since comment was left
/- if !defined?(line) || line == note.diff_line
%tr.notes_holder %tr.notes_holder
- if note1 - if note1
%td.notes_line %td.notes_line
%span.btn.disabled %span.btn.disabled
%i.icon-comment %i.icon-comment
= notes1.count = notes1.count
%td.notes_content %td.notes_content.parallel
%ul.notes{ rel: note1.discussion_id } %ul.notes{ rel: note1.discussion_id }
= render notes1 = render notes1
@@ -23,7 +22,7 @@
%span.btn.disabled %span.btn.disabled
%i.icon-comment %i.icon-comment
= notes2.count = notes2.count
%td.notes_content %td.notes_content.parallel
%ul.notes{ rel: note2.discussion_id } %ul.notes{ rel: note2.discussion_id }
= render notes2 = render notes2
features/project/merge_requests.feature
+10
View File
@@ -147,3 +147,13 @@ Feature: Project Merge Requests
And I switch to the diff tab And I switch to the diff tab
And I unfold diff And I unfold diff
Then I should see additional file lines Then I should see additional file lines
@javascript
Scenario: I show comments on a merge request side-by-side diff with comments in multiple files
Given project "Shop" have "Bug NS-05" open merge request with diffs inside
And I visit merge request page "Bug NS-05"
And I switch to the diff tab
And I leave a comment like "Line is correct" on line 12 of the first file
And I leave a comment like "Line is wrong" on line 39 of the second file
And I click Side-by-side Diff tab
Then I should see comments on the side-by-side diff page
features/steps/project/merge_requests.rb
+10
View File
@@ -250,6 +250,16 @@ class ProjectMergeRequests < Spinach::FeatureSteps
expect(first('.text-file')).to have_content('.bundle') expect(first('.text-file')).to have_content('.bundle')
end end
step 'I click Side-by-side Diff tab' do
click_link 'Side-by-side Diff'
end
step 'I should see comments on the side-by-side diff page' do
within '.files [id^=diff]:nth-child(1) .note-text' do
page.should have_visible_content "Line is correct"
end
end
def project def project
@project ||= Project.find_by!(name: "Shop") @project ||= Project.find_by!(name: "Shop")
end end
spec/helpers/diff_helper_spec.rb
+27 -26
View File
@@ -67,32 +67,33 @@ describe DiffHelper do
def parallel_diff_result_array def parallel_diff_result_array
[ [
["match", 6, "@@ -6,12 +6,18 @@ module Popen", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6", "match", 6, "@@ -6,12 +6,18 @@ module Popen"], ["match", 6, "@@ -6,12 +6,18 @@ module Popen", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6", "match", 6, "@@ -6,12 +6,18 @@ module Popen", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6"],
[nil, 6, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6", nil, 6, " "], [nil, 6, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6", nil, 6, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6"], [nil, 7, " def popen(cmd, path=nil)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7", nil, 7, " def popen(cmd, path=nil)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"],
[nil, 7, " def popen(cmd, path=nil)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7", nil, 7, " def popen(cmd, path=nil)"], [nil, 8, " unless cmd.is_a?(Array)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8", nil, 8, " unless cmd.is_a?(Array)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8"],
[nil, 8, " unless cmd.is_a?(Array)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8", nil, 8, " unless cmd.is_a?(Array)"], ["old", 9, "- raise <span class='idiff'></span>&quot;System commands must be given as an array of strings&quot;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9", "new", 9, "+ raise <span class='idiff'>RuntimeError, </span>&quot;System commands must be given as an array of strings&quot;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"],
["old", 9, "- raise <span class='idiff'></span>&quot;System commands must be given as an array of strings&quot;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9", "new", 9, "+ raise <span class='idiff'>RuntimeError, </span>&quot;System commands must be given as an array of strings&quot;"], [nil, 10, " end", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10", nil, 10, " end", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10"],
[nil, 10, " end", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10", nil, 10, " end"], [nil, 11, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_11_11", nil, 11, " "], [nil, 11, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_11_11", nil, 11, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_11_11"],
[nil, 12, " path ||= Dir.pwd", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_12_12", nil, 12, " path ||= Dir.pwd"], [nil, 12, " path ||= Dir.pwd", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_12_12", nil, 12, " path ||= Dir.pwd", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_12_12"],
["old", 13, "- vars = { &quot;PWD&quot; =&gt; path }", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_13_13", "old", nil, "&nbsp;"], ["old", 13, "- vars = { &quot;PWD&quot; =&gt; path }", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_13_13", "old", nil, "&nbsp;", nil],
["old", 14, "- options = { chdir: path }", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_13", "new", 13, "+"], ["old", 14, "- options = { chdir: path }", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_13", "new", 13, "+", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_13"],
[nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14", "new", 14, "+ vars = {"], [nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14", "new", 14, "+ vars = {", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14"],
[nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15", "new", 15, "+ &quot;PWD&quot; =&gt; path"], [nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15", "new", 15, "+ &quot;PWD&quot; =&gt; path", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15"],
[nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_16", "new", 16, "+ }"], [nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_16", "new", 16, "+ }", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_16"],
[nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_17", "new", 17, "+"], [nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_17", "new", 17, "+", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_17"],
[nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_18", "new", 18, "+ options = {"], [nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_18", "new", 18, "+ options = {", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_18"],
[nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_19", "new", 19, "+ chdir: path"], [nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_19", "new", 19, "+ chdir: path", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_19"],
[nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_20", "new", 20, "+ }"], [nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_20", "new", 20, "+ }", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_20"],
[nil, 15, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_21", nil, 21, " "], [nil, 15, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_21", nil, 21, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_21"],
[nil, 16, " unless File.directory?(path)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_16_22", nil, 22, " unless File.directory?(path)"], [nil, 16, " unless File.directory?(path)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_16_22", nil, 22, " unless File.directory?(path)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_16_22"],
[nil, 17, " FileUtils.mkdir_p(path)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_17_23", nil, 23, " FileUtils.mkdir_p(path)"], [nil, 17, " FileUtils.mkdir_p(path)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_17_23", nil, 23, " FileUtils.mkdir_p(path)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_17_23"],
["match", 19, "@@ -19,6 +25,7 @@ module Popen", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25", "match", 25, "@@ -19,6 +25,7 @@ module Popen"], ["match", 19, "@@ -19,6 +25,7 @@ module Popen", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25", "match", 25, "@@ -19,6 +25,7 @@ module Popen", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25"],
[nil, 19, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25", nil, 25, " "], [nil, 20, " @cmd_output = &quot;&quot;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_20_26", nil, 26, " @cmd_output = &quot;&quot;"], [nil, 19, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25", nil, 25, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25"],
[nil, 21, " @cmd_status = 0", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_21_27", nil, 27, " @cmd_status = 0"], [nil, 20, " @cmd_output = &quot;&quot;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_20_26", nil, 26, " @cmd_output = &quot;&quot;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_20_26"],
[nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_28", "new", 28, "+"], [nil, 21, " @cmd_status = 0", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_21_27", nil, 27, " @cmd_status = 0", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_21_27"],
[nil, 22, " Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_29", nil, 29, " Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|"], [nil, nil, "&nbsp;", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_28", "new", 28, "+", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_28"],
[nil, 23, " @cmd_output &lt;&lt; stdout.read", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_23_30", nil, 30, " @cmd_output &lt;&lt; stdout.read"], [nil, 22, " Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_29", nil, 29, " Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_29"],
[nil, 24, " @cmd_output &lt;&lt; stderr.read", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_24_31", nil, 31, " @cmd_output &lt;&lt; stderr.read"] [nil, 23, " @cmd_output &lt;&lt; stdout.read", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_23_30", nil, 30, " @cmd_output &lt;&lt; stdout.read", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_23_30"],
[nil, 24, " @cmd_output &lt;&lt; stderr.read", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_24_31", nil, 31, " @cmd_output &lt;&lt; stderr.read", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_24_31"]
] ]
end end
end end