Commit a761c59a authored by hhoopes's avatar hhoopes Committed by Sean McGivern

Add keyword arguments to truncated_diff method

* Added keyword arguments to truncated_diff_lines method to allow for using highlighting or not (html templates vs. text)
* Tweaked templates for consistency and format appropriateness
parent f928dba9
...@@ -27,7 +27,7 @@ class Discussion ...@@ -27,7 +27,7 @@ class Discussion
delegate :blob, delegate :blob,
:highlighted_diff_lines, :highlighted_diff_lines,
:text_parsed_diff_lines, :diff_lines,
to: :diff_file, to: :diff_file,
allow_nil: true allow_nil: true
...@@ -164,10 +164,11 @@ class Discussion ...@@ -164,10 +164,11 @@ class Discussion
end end
# Returns an array of at most 16 highlighted lines above a diff note # Returns an array of at most 16 highlighted lines above a diff note
def truncated_diff_lines def truncated_diff_lines(highlight: true)
initial_lines = highlight ? highlighted_diff_lines : diff_lines
prev_lines = [] prev_lines = []
diff_file.diff_lines.each do |line| initial_lines.each do |line|
if line.meta? if line.meta?
prev_lines.clear prev_lines.clear
else else
......
...@@ -9,7 +9,7 @@ ...@@ -9,7 +9,7 @@
%table %table
- discussions = { discussion.original_line_code => discussion } - discussions = { discussion.original_line_code => discussion }
= render partial: "projects/diffs/line", = render partial: "projects/diffs/line",
collection: discussion.highlighted_diff_lines(discussion.truncated_diff_lines), collection: discussion.highlighted_diff_lines,
as: :line, as: :line,
locals: { diff_file: diff_file, locals: { diff_file: diff_file,
discussions: discussions, discussions: discussions,
......
<% if current_application_settings.email_author_in_body %>
<%= @note.author_name %> wrote:
<% end -%>
<%= @note.note %>
= content_for :head do = content_for :head do
= stylesheet_link_tag 'mailers/highlighted_diff_email' = stylesheet_link_tag 'mailers/highlighted_diff_email'
New comment
- if @note.diff_note? && @note.diff_file - if @note.diff_note? && @note.diff_file
on on
= link_to @note.diff_file.file_path, @target_url, class: 'details' = link_to @note.diff_file.file_path, @target_url, class: 'details'
\: \:
- if @discussion
%table %table
= render partial: "projects/diffs/line", = render partial: "projects/diffs/line",
collection: @discussion.highlighted_diff_lines(@discussion.truncated_diff_lines), collection: @discussion.truncated_diff_lines,
as: :line, as: :line,
locals: { diff_file: @note.diff_file, locals: { diff_file: @note.diff_file,
plain: true, plain: true,
email: true } email: true }
= render 'note_message' = render 'note_message'
<% if @note.diff_note? && @note.diff_file -%>
on <%= @note.diff_file.file_path -%>
<% end -%>:
<%= url %>
<%= render 'simple_diff' if @discussion -%>
<%= render 'note_message' %>
<% lines = @discussion.truncated_diff_lines %> <% @discussion.truncated_diff_lines(highlight: false).each do |line| %>
<% @discussion.text_parsed_diff_lines(lines).each do |line| %> <%= "> " + line.text %>
<%= line %>
<% end %> <% end %>
%p.details %p.details
New comment for Commit
= @commit.short_id
= render 'note_mr_or_commit_email' = render 'note_mr_or_commit_email'
New comment for Commit <%= @commit.short_id %> <% url = url_for(namespace_project_commit_url(@note.project.namespace, @note.project, id: @commit.id, anchor: "note_#{@note.id}")) %>
<%= url_for(namespace_project_commit_url(@note.project.namespace, @note.project, id: @commit.id, anchor: "note_#{@note.id}")) %> New comment for Commit <%= @commit.short_id -%>
<%= render partial: 'note_mr_or_commit_email', locals: { url: url } %>
<%= render 'simple_diff' if @discussion %>
<% if current_application_settings.email_author_in_body %>
<%= @note.author_name %> wrote:
<% end %>
<%= @note.note %>
%p.details %p.details
New comment for Merge Request
= @merge_request.to_reference
= render 'note_mr_or_commit_email' = render 'note_mr_or_commit_email'
New comment for Merge Request <%= @merge_request.to_reference %> <% url = url_for(namespace_project_merge_request_url(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request, anchor: "note_#{@note.id}")) %>
<%= url_for(namespace_project_merge_request_url(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request, anchor: "note_#{@note.id}")) %> New comment for Merge Request <%= @merge_request.to_reference -%>
<%= render partial: 'note_mr_or_commit_email', locals: { url: url }%>
<%= render 'simple_diff' if @discussion %>
<% if current_application_settings.email_author_in_body %>
<%= @note.author_name %> wrote:
<% end %>
<%= @note.note %>
...@@ -76,15 +76,8 @@ module Gitlab ...@@ -76,15 +76,8 @@ module Gitlab
@diff_lines ||= Gitlab::Diff::Parser.new.parse(raw_diff.each_line).to_a @diff_lines ||= Gitlab::Diff::Parser.new.parse(raw_diff.each_line).to_a
end end
def highlighted_diff_lines(lines = self) def highlighted_diff_lines
@highlighted_diff_lines ||= Gitlab::Diff::Highlight.new(lines, repository: self.repository).highlight @highlighted_diff_lines ||= Gitlab::Diff::Highlight.new(self, repository: self.repository).highlight
end
def text_parsed_diff_lines(lines)
@text_parsed_diff_lines ||=
lines.map do | line |
"> " + line.text
end
end end
# Array[<Hash>] with right/left keys that contains Gitlab::Diff::Line objects which text is hightlighted # Array[<Hash>] with right/left keys that contains Gitlab::Diff::Line objects which text is hightlighted
......
...@@ -697,7 +697,7 @@ describe Notify do ...@@ -697,7 +697,7 @@ describe Notify do
let(:note) { create(model, project: project, author: note_author) } let(:note) { create(model, project: project, author: note_author) }
it "includes diffs with character-level highlighting" do it "includes diffs with character-level highlighting" do
is_expected.to have_body_text /\<span class='idiff left right'>vars = {<\/span>/ is_expected.to have_body_text /<span class=\"p\">}<\/span><\/span>/
end end
it 'contains a link to the diff file' do it 'contains a link to the diff file' do
...@@ -743,9 +743,6 @@ describe Notify do ...@@ -743,9 +743,6 @@ describe Notify do
subject { Notify.note_commit_email(recipient.id, note.id) } subject { Notify.note_commit_email(recipient.id, note.id) }
it_behaves_like 'a note email on a diff', :diff_note_on_commit it_behaves_like 'a note email on a diff', :diff_note_on_commit
# it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
# let(:model) { commit }
# end
it_behaves_like 'it should show Gmail Actions View Commit link' it_behaves_like 'it should show Gmail Actions View Commit link'
it_behaves_like 'a user cannot unsubscribe through footer link' it_behaves_like 'a user cannot unsubscribe through footer link'
end end
...@@ -757,9 +754,6 @@ describe Notify do ...@@ -757,9 +754,6 @@ describe Notify do
subject { Notify.note_merge_request_email(recipient.id, note.id) } subject { Notify.note_merge_request_email(recipient.id, note.id) }
it_behaves_like 'a note email on a diff', :diff_note_on_merge_request it_behaves_like 'a note email on a diff', :diff_note_on_merge_request
# it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
# let(:model) { merge_request }
# end
it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like 'it should show Gmail Actions View Merge request link'
it_behaves_like 'an unsubscribeable thread' it_behaves_like 'an unsubscribeable thread'
end end
......
...@@ -595,7 +595,7 @@ describe Discussion, model: true do ...@@ -595,7 +595,7 @@ describe Discussion, model: true do
let(:truncated_lines) { subject.truncated_diff_lines } let(:truncated_lines) { subject.truncated_diff_lines }
context "when diff is greater than allowed number of truncated diff lines " do context "when diff is greater than allowed number of truncated diff lines " do
let(:initial_line_count) { subject.diff_file.diff_lines.count } let(:initial_line_count) { subject.diff_lines.count }
let(:truncated_line_count) { truncated_lines.count } let(:truncated_line_count) { truncated_lines.count }
it "returns fewer lines" do it "returns fewer lines" do
...@@ -606,7 +606,7 @@ describe Discussion, model: true do ...@@ -606,7 +606,7 @@ describe Discussion, model: true do
end end
context "when some diff lines are meta" do context "when some diff lines are meta" do
let(:initial_meta_lines?) { subject.diff_file.diff_lines.any?(&:meta?) } let(:initial_meta_lines?) { subject.diff_lines.any?(&:meta?) }
let(:truncated_meta_lines?) { truncated_lines.any?(&:meta?) } let(:truncated_meta_lines?) { truncated_lines.any?(&:meta?) }
it "returns no meta lines" do it "returns no meta lines" do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment