summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/assets/javascripts/copy_as_gfm.js61
-rw-r--r--app/assets/javascripts/diff.js15
-rw-r--r--app/assets/stylesheets/pages/diff.scss12
-rw-r--r--app/helpers/diff_helper.rb16
-rw-r--r--app/views/projects/blob/diff.html.haml31
-rw-r--r--app/views/projects/diffs/_parallel_view.html.haml16
-rw-r--r--changelogs/unreleased/dm-copy-parallel-diff.yml5
-rw-r--r--features/steps/shared/diff_note.rb2
-rw-r--r--spec/features/copy_as_gfm_spec.rb128
-rw-r--r--spec/features/merge_requests/diff_notes_avatars_spec.rb22
10 files changed, 208 insertions, 100 deletions
diff --git a/app/assets/javascripts/copy_as_gfm.js b/app/assets/javascripts/copy_as_gfm.js
index e3e2c798570..93b0cbf4209 100644
--- a/app/assets/javascripts/copy_as_gfm.js
+++ b/app/assets/javascripts/copy_as_gfm.js
@@ -298,7 +298,7 @@ class CopyAsGFM {
const documentFragment = getSelectedFragment();
if (!documentFragment) return;
- const el = transformer(documentFragment.cloneNode(true));
+ const el = transformer(documentFragment.cloneNode(true), e.currentTarget);
if (!el) return;
e.preventDefault();
@@ -338,55 +338,64 @@ class CopyAsGFM {
}
static transformGFMSelection(documentFragment) {
- const gfmEls = documentFragment.querySelectorAll('.md, .wiki');
- switch (gfmEls.length) {
+ const gfmElements = documentFragment.querySelectorAll('.md, .wiki');
+ switch (gfmElements.length) {
case 0: {
return documentFragment;
}
case 1: {
- return gfmEls[0];
+ return gfmElements[0];
}
default: {
- const allGfmEl = document.createElement('div');
+ const allGfmElement = document.createElement('div');
- for (let i = 0; i < gfmEls.length; i += 1) {
- const lineEl = gfmEls[i];
- allGfmEl.appendChild(lineEl);
- allGfmEl.appendChild(document.createTextNode('\n\n'));
+ for (let i = 0; i < gfmElements.length; i += 1) {
+ const gfmElement = gfmElements[i];
+ allGfmElement.appendChild(gfmElement);
+ allGfmElement.appendChild(document.createTextNode('\n\n'));
}
- return allGfmEl;
+ return allGfmElement;
}
}
}
- static transformCodeSelection(documentFragment) {
- const lineEls = documentFragment.querySelectorAll('.line');
+ static transformCodeSelection(documentFragment, target) {
+ let lineSelector = '.line';
- let codeEl;
- if (lineEls.length > 1) {
- codeEl = document.createElement('pre');
- codeEl.className = 'code highlight';
+ if (target) {
+ const lineClass = ['left-side', 'right-side'].filter(name => target.classList.contains(name))[0];
+ if (lineClass) {
+ lineSelector = `.line_content.${lineClass} ${lineSelector}`;
+ }
+ }
+
+ const lineElements = documentFragment.querySelectorAll(lineSelector);
+
+ let codeElement;
+ if (lineElements.length > 1) {
+ codeElement = document.createElement('pre');
+ codeElement.className = 'code highlight';
- const lang = lineEls[0].getAttribute('lang');
+ const lang = lineElements[0].getAttribute('lang');
if (lang) {
- codeEl.setAttribute('lang', lang);
+ codeElement.setAttribute('lang', lang);
}
} else {
- codeEl = document.createElement('code');
+ codeElement = document.createElement('code');
}
- if (lineEls.length > 0) {
- for (let i = 0; i < lineEls.length; i += 1) {
- const lineEl = lineEls[i];
- codeEl.appendChild(lineEl);
- codeEl.appendChild(document.createTextNode('\n'));
+ if (lineElements.length > 0) {
+ for (let i = 0; i < lineElements.length; i += 1) {
+ const lineElement = lineElements[i];
+ codeElement.appendChild(lineElement);
+ codeElement.appendChild(document.createTextNode('\n'));
}
} else {
- codeEl.appendChild(documentFragment);
+ codeElement.appendChild(documentFragment);
}
- return codeEl;
+ return codeElement;
}
static nodeToGFM(node, respectWhitespaceParam = false) {
diff --git a/app/assets/javascripts/diff.js b/app/assets/javascripts/diff.js
index 6a008112203..ae8338f5fd2 100644
--- a/app/assets/javascripts/diff.js
+++ b/app/assets/javascripts/diff.js
@@ -24,7 +24,8 @@ class Diff {
if (!isBound) {
$(document)
.on('click', '.js-unfold', this.handleClickUnfold.bind(this))
- .on('click', '.diff-line-num a', this.handleClickLineNum.bind(this));
+ .on('click', '.diff-line-num a', this.handleClickLineNum.bind(this))
+ .on('mousedown', 'td.line_content.parallel', this.handleParallelLineDown.bind(this));
isBound = true;
}
@@ -100,6 +101,18 @@ class Diff {
this.highlightSelectedLine();
}
+ handleParallelLineDown(e) {
+ const line = $(e.currentTarget);
+ const table = line.closest('table');
+
+ table.removeClass('left-side-selected right-side-selected');
+
+ const lineClass = ['left-side', 'right-side'].filter(name => line.hasClass(name))[0];
+ if (lineClass) {
+ table.addClass(`${lineClass}-selected`);
+ }
+ }
+
diffViewType() {
return $('.inline-parallel-buttons a.active').data('view-type');
}
diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss
index e4bd783c8bc..fb23343b966 100644
--- a/app/assets/stylesheets/pages/diff.scss
+++ b/app/assets/stylesheets/pages/diff.scss
@@ -77,6 +77,18 @@
word-wrap: break-word;
}
}
+
+ &.left-side-selected {
+ td.line_content.parallel.right-side {
+ @include user-select(none);
+ }
+ }
+
+ &.right-side-selected {
+ td.line_content.parallel.left-side {
+ @include user-select(none);
+ }
+ }
}
tr.line_holder.parallel {
diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb
index 28f591a4e22..4e4a66e8a02 100644
--- a/app/helpers/diff_helper.rb
+++ b/app/helpers/diff_helper.rb
@@ -33,19 +33,21 @@ module DiffHelper
end
def diff_match_line(old_pos, new_pos, text: '', view: :inline, bottom: false)
- content = content_tag :td, text, class: "line_content match #{view == :inline ? '' : view}"
- cls = ['diff-line-num', 'unfold', 'js-unfold']
- cls << 'js-unfold-bottom' if bottom
+ content_line_class = %w[line_content match]
+ content_line_class << 'parallel' if view == :parallel
+
+ line_num_class = %w[diff-line-num unfold js-unfold]
+ line_num_class << 'js-unfold-bottom' if bottom
html = ''
if old_pos
- html << content_tag(:td, '...', class: cls + ['old_line'], data: { linenumber: old_pos })
- html << content unless view == :inline
+ html << content_tag(:td, '...', class: [*line_num_class, 'old_line'], data: { linenumber: old_pos })
+ html << content_tag(:td, text, class: [*content_line_class, 'left-side']) if view == :parallel
end
if new_pos
- html << content_tag(:td, '...', class: cls + ['new_line'], data: { linenumber: new_pos })
- html << content
+ html << content_tag(:td, '...', class: [*line_num_class, 'new_line'], data: { linenumber: new_pos })
+ html << content_tag(:td, text, class: [*content_line_class, ('right-side' if view == :parallel)])
end
html.html_safe
diff --git a/app/views/projects/blob/diff.html.haml b/app/views/projects/blob/diff.html.haml
index d1d448f0d4c..ea7a71792a3 100644
--- a/app/views/projects/blob/diff.html.haml
+++ b/app/views/projects/blob/diff.html.haml
@@ -5,25 +5,24 @@
= diff_match_line @form.since, @form.since, text: @match_line, view: diff_view
- @lines.each_with_index do |line, index|
- - line_new = index + @form.since
- - line_old = line_new - @form.offset
- - line_content = capture do
- %td.line_content.noteable_line{ class: line_class }==#{' ' * @form.indent}#{line}
- %tr.line_holder.diff-expanded{ id: line_old, class: line_class }
+ - line_number_new = index + @form.since
+ - line_number_old = line_number_new - @form.offset
+ - line[0, 0] = ' ' * @form.indent
+ %tr.line_holder.diff-expanded{ id: line_number_old, class: line_class }
- case diff_view
- when :inline
- %td.old_line.diff-line-num{ data: { linenumber: line_old } }
- %a{ href: "#", data: { linenumber: line_old }, disabled: true }
- %td.new_line.diff-line-num{ data: { linenumber: line_new } }
- %a{ href: "#", data: { linenumber: line_new }, disabled: true }
- = line_content
+ %td.old_line.diff-line-num{ data: { linenumber: line_number_old } }
+ %a{ href: "#", data: { linenumber: line_number_old }, disabled: true }
+ %td.new_line.diff-line-num{ data: { linenumber: line_number_new } }
+ %a{ href: "#", data: { linenumber: line_number_new }, disabled: true }
+ %td.line_content.noteable_line{ class: line_class }= line
- when :parallel
- %td.old_line.diff-line-num{ data: { linenumber: line_old } }
- %a{ href: "##{line_old}", data: { linenumber: line_old }, disabled: true }
- = line_content
- %td.new_line.diff-line-num{ data: { linenumber: line_new } }
- %a{ href: "##{line_new}", data: { linenumber: line_new }, disabled: true }
- = line_content
+ %td.old_line.diff-line-num{ data: { linenumber: line_number_old } }
+ %a{ href: "##{line_number_old}", data: { linenumber: line_number_old }, disabled: true }
+ %td.line_content.noteable_line.left-side{ class: line_class }= line
+ %td.new_line.diff-line-num{ data: { linenumber: line_number_new } }
+ %a{ href: "##{line_number_new}", data: { linenumber: line_number_new }, disabled: true }
+ %td.line_content.noteable_line.right-side{ class: line_class }= line
- if @form.unfold? && @form.bottom? && @form.to < @blob.lines.size
%tr.line_holder{ id: @form.to, class: line_class }
diff --git a/app/views/projects/diffs/_parallel_view.html.haml b/app/views/projects/diffs/_parallel_view.html.haml
index 56d63250714..1f0ca211074 100644
--- a/app/views/projects/diffs/_parallel_view.html.haml
+++ b/app/views/projects/diffs/_parallel_view.html.haml
@@ -14,20 +14,20 @@
= diff_match_line left.old_pos, nil, text: left.text, view: :parallel
- when 'old-nonewline', 'new-nonewline'
%td.old_line.diff-line-num
- %td.line_content.match= left.text
+ %td.line_content.match.left-side= left.text
- else
- left_line_code = diff_file.line_code(left)
- left_position = diff_file.position(left)
- %td.old_line.diff-line-num.js-avatar-container{ id: left_line_code, class: left.type, data: { linenumber: left.old_pos } }
+ %td.old_line.diff-line-num.js-avatar-container{ class: left.type, data: { linenumber: left.old_pos } }
= add_diff_note_button(left_line_code, left_position, 'old')
%a{ href: "##{left_line_code}", data: { linenumber: left.old_pos } }
- discussion_left = discussions_left.try(:first)
- if discussion_left && discussion_left.resolvable?
%diff-note-avatars{ "discussion-id" => discussion_left.id }
- %td.line_content.parallel.noteable_line{ class: left.type }= diff_line_content(left.text)
+ %td.line_content.parallel.noteable_line.left-side{ id: left_line_code, class: left.type }= diff_line_content(left.text)
- else
%td.old_line.diff-line-num.empty-cell
- %td.line_content.parallel
+ %td.line_content.parallel.left-side
- if right
- case right.type
@@ -35,20 +35,20 @@
= diff_match_line nil, right.new_pos, text: left.text, view: :parallel
- when 'old-nonewline', 'new-nonewline'
%td.new_line.diff-line-num
- %td.line_content.match= right.text
+ %td.line_content.match.right-side= right.text
- else
- right_line_code = diff_file.line_code(right)
- right_position = diff_file.position(right)
- %td.new_line.diff-line-num.js-avatar-container{ id: right_line_code, class: right.type, data: { linenumber: right.new_pos } }
+ %td.new_line.diff-line-num.js-avatar-container{ class: right.type, data: { linenumber: right.new_pos } }
= add_diff_note_button(right_line_code, right_position, 'new')
%a{ href: "##{right_line_code}", data: { linenumber: right.new_pos } }
- discussion_right = discussions_right.try(:first)
- if discussion_right && discussion_right.resolvable?
%diff-note-avatars{ "discussion-id" => discussion_right.id }
- %td.line_content.parallel.noteable_line{ class: right.type }= diff_line_content(right.text)
+ %td.line_content.parallel.noteable_line.right-side{ id: right_line_code, class: right.type }= diff_line_content(right.text)
- else
%td.old_line.diff-line-num.empty-cell
- %td.line_content.parallel
+ %td.line_content.parallel.right-side
- if discussions_left || discussions_right
= render "discussions/parallel_diff_discussion", discussions_left: discussions_left, discussions_right: discussions_right
diff --git a/changelogs/unreleased/dm-copy-parallel-diff.yml b/changelogs/unreleased/dm-copy-parallel-diff.yml
new file mode 100644
index 00000000000..96a65007661
--- /dev/null
+++ b/changelogs/unreleased/dm-copy-parallel-diff.yml
@@ -0,0 +1,5 @@
+---
+title: Only copy old/new code when selecting left/right side of parallel diff
+merge_request:
+author:
+type: added
diff --git a/features/steps/shared/diff_note.rb b/features/steps/shared/diff_note.rb
index 2c59ec5bb06..c872bd6f861 100644
--- a/features/steps/shared/diff_note.rb
+++ b/features/steps/shared/diff_note.rb
@@ -232,7 +232,7 @@ module SharedDiffNote
end
def click_parallel_diff_line(code, line_type)
- find(".line_holder.parallel .diff-line-num[id='#{code}']").trigger 'mouseover'
+ find(".line_holder.parallel td[id='#{code}']").find(:xpath, 'preceding-sibling::*[1][self::td]').trigger 'mouseover'
find(".line_holder.parallel button[data-line-code='#{code}']").trigger 'click'
end
end
diff --git a/spec/features/copy_as_gfm_spec.rb b/spec/features/copy_as_gfm_spec.rb
index dfeba722ac6..ebcd0ba0dcd 100644
--- a/spec/features/copy_as_gfm_spec.rb
+++ b/spec/features/copy_as_gfm_spec.rb
@@ -446,7 +446,7 @@ describe 'Copy as GFM', js: true do
def verify(label, *gfms)
aggregate_failures(label) do
gfms.each do |gfm|
- html = gfm_to_html(gfm)
+ html = gfm_to_html(gfm).gsub(/\A&#x000A;|&#x000A;\z/, '')
output_gfm = html_to_gfm(html)
expect(output_gfm.strip).to eq(gfm.strip)
end
@@ -463,42 +463,98 @@ describe 'Copy as GFM', js: true do
let(:project) { create(:project, :repository) }
context 'from a diff' do
- before do
- visit project_commit_path(project, sample_commit.id)
- end
+ shared_examples 'copying code from a diff' do
+ context 'selecting one word of text' do
+ it 'copies as inline code' do
+ verify(
+ '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"] .line .no',
- context 'selecting one word of text' do
- it 'copies as inline code' do
- verify(
- '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"] .line .no',
+ '`RuntimeError`',
- '`RuntimeError`'
- )
+ target: '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]'
+ )
+ end
end
- end
- context 'selecting one line of text' do
- it 'copies as inline code' do
- verify(
- '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"] .line',
+ context 'selecting one line of text' do
+ it 'copies as inline code' do
+ verify(
+ '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]',
- '`raise RuntimeError, "System commands must be given as an array of strings"`'
- )
+ '`raise RuntimeError, "System commands must be given as an array of strings"`',
+
+ target: '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]'
+ )
+ end
+ end
+
+ context 'selecting multiple lines of text' do
+ it 'copies as a code block' do
+ verify(
+ '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10"]',
+
+ <<-GFM.strip_heredoc,
+ ```ruby
+ raise RuntimeError, "System commands must be given as an array of strings"
+ end
+ ```
+ GFM
+
+ target: '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]'
+ )
+ end
end
end
- context 'selecting multiple lines of text' do
- it 'copies as a code block' do
- verify(
- '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10"]',
+ context 'inline diff' do
+ before do
+ visit project_commit_path(project, sample_commit.id, view: 'inline')
+ end
- <<-GFM.strip_heredoc,
- ```ruby
- raise RuntimeError, "System commands must be given as an array of strings"
- end
- ```
- GFM
- )
+ it_behaves_like 'copying code from a diff'
+ end
+
+ context 'parallel diff' do
+ before do
+ visit project_commit_path(project, sample_commit.id, view: 'parallel')
+ end
+
+ it_behaves_like 'copying code from a diff'
+
+ context 'selecting code on the left' do
+ it 'copies as a code block' do
+ verify(
+ '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10"]',
+
+ <<-GFM.strip_heredoc,
+ ```ruby
+ unless cmd.is_a?(Array)
+ raise "System commands must be given as an array of strings"
+ end
+ ```
+ GFM
+
+ target: '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8"].left-side'
+ )
+ end
+ end
+
+ context 'selecting code on the right' do
+ it 'copies as a code block' do
+ verify(
+ '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10"]',
+
+ <<-GFM.strip_heredoc,
+ ```ruby
+ unless cmd.is_a?(Array)
+ raise RuntimeError, "System commands must be given as an array of strings"
+ end
+ ```
+ GFM
+
+ target: '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8"].right-side'
+ )
+ end
end
end
end
@@ -587,9 +643,9 @@ describe 'Copy as GFM', js: true do
end
end
- def verify(selector, gfm)
+ def verify(selector, gfm, target: nil)
html = html_for_selector(selector)
- output_gfm = html_to_gfm(html, 'transformCodeSelection')
+ output_gfm = html_to_gfm(html, 'transformCodeSelection', target: target)
expect(output_gfm.strip).to eq(gfm.strip)
end
end
@@ -605,15 +661,21 @@ describe 'Copy as GFM', js: true do
page.evaluate_script(js)
end
- def html_to_gfm(html, transformer = 'transformGFMSelection')
+ def html_to_gfm(html, transformer = 'transformGFMSelection', target: nil)
js = <<-JS.strip_heredoc
(function(html) {
var transformer = window.gl.CopyAsGFM[#{transformer.inspect}];
var node = document.createElement('div');
- node.innerHTML = html;
+ $(html).each(function() { node.appendChild(this) });
+
+ var targetSelector = #{target.to_json};
+ var target;
+ if (targetSelector) {
+ target = document.querySelector(targetSelector);
+ }
- node = transformer(node);
+ node = transformer(node, target);
if (!node) return null;
return window.gl.CopyAsGFM.nodeToGFM(node);
diff --git a/spec/features/merge_requests/diff_notes_avatars_spec.rb b/spec/features/merge_requests/diff_notes_avatars_spec.rb
index 9bcb78d5206..4766cdf716f 100644
--- a/spec/features/merge_requests/diff_notes_avatars_spec.rb
+++ b/spec/features/merge_requests/diff_notes_avatars_spec.rb
@@ -84,7 +84,7 @@ feature 'Diff note avatars', js: true do
end
it 'shows note avatar' do
- page.within find("[id='#{position.line_code(project.repository)}']") do
+ page.within find_line(position.line_code(project.repository)) do
find('.diff-notes-collapse').click
expect(page).to have_selector('img.js-diff-comment-avatar', count: 1)
@@ -92,7 +92,7 @@ feature 'Diff note avatars', js: true do
end
it 'shows comment on note avatar' do
- page.within find("[id='#{position.line_code(project.repository)}']") do
+ page.within find_line(position.line_code(project.repository)) do
find('.diff-notes-collapse').click
expect(first('img.js-diff-comment-avatar')["data-original-title"]).to eq("#{note.author.name}: #{note.note.truncate(17)}")
@@ -100,13 +100,13 @@ feature 'Diff note avatars', js: true do
end
it 'toggles comments when clicking avatar' do
- page.within find("[id='#{position.line_code(project.repository)}']") do
+ page.within find_line(position.line_code(project.repository)) do
find('.diff-notes-collapse').click
end
expect(page).to have_selector('.notes_holder', visible: false)
- page.within find("[id='#{position.line_code(project.repository)}']") do
+ page.within find_line(position.line_code(project.repository)) do
first('img.js-diff-comment-avatar').click
end
@@ -122,7 +122,7 @@ feature 'Diff note avatars', js: true do
wait_for_requests
- page.within find("[id='#{position.line_code(project.repository)}']") do
+ page.within find_line(position.line_code(project.repository)) do
expect(page).not_to have_selector('img.js-diff-comment-avatar')
end
end
@@ -138,7 +138,7 @@ feature 'Diff note avatars', js: true do
wait_for_requests
end
- page.within find("[id='#{position.line_code(project.repository)}']") do
+ page.within find_line(position.line_code(project.repository)) do
find('.diff-notes-collapse').trigger('click')
expect(page).to have_selector('img.js-diff-comment-avatar', count: 2)
@@ -158,7 +158,7 @@ feature 'Diff note avatars', js: true do
end
end
- page.within find("[id='#{position.line_code(project.repository)}']") do
+ page.within find_line(position.line_code(project.repository)) do
find('.diff-notes-collapse').trigger('click')
expect(page).to have_selector('img.js-diff-comment-avatar', count: 3)
@@ -176,7 +176,7 @@ feature 'Diff note avatars', js: true do
end
it 'shows extra comment count' do
- page.within find("[id='#{position.line_code(project.repository)}']") do
+ page.within find_line(position.line_code(project.repository)) do
find('.diff-notes-collapse').click
expect(find('.diff-comments-more-count')).to have_content '+1'
@@ -185,4 +185,10 @@ feature 'Diff note avatars', js: true do
end
end
end
+
+ def find_line(line_code)
+ line = find("[id='#{line_code}']")
+ line = line.find(:xpath, 'preceding-sibling::*[1][self::td]') if line.tag_name == 'td'
+ line
+ end
end