Commit d0d3ae0f authored by Douwe Maan's avatar Douwe Maan Committed by Yorick Peterse

Merge branch 'adambutler/gitlab-ce-feature/support-diff-of-issue-title-rename'

# Conflicts:
#	app/services/system_note_service.rb
parent f2e46514
......@@ -9,6 +9,8 @@ v 8.8.0 (unreleased)
- Use a case-insensitive comparison in sanitizing URI schemes
- Toggle sign-up confirmation emails in application settings
- Make it possible to prevent tagged runner from picking untagged jobs
- Added `InlineDiffFilter` to the markdown parser. (Adam Butler)
- Added inline diff styling for `change_title` system notes. (Adam Butler)
- Project#open_branches has been cleaned up and no longer loads entire records into memory.
- Escape HTML in commit titles in system note messages
- Fix creation of Ci::Commit object which can lead to pending, failed in some scenarios
......
......@@ -36,22 +36,6 @@
}
}
.filename {
&.old {
display: inline-block;
span.idiff {
background-color: #f8cbcb;
}
}
&.new {
display: inline-block;
span.idiff {
background-color: #a6f3a6;
}
}
}
a:not(.btn) {
color: $gl-dark-link-color;
}
......
......@@ -269,3 +269,11 @@ h1, h2, h3, h4 {
text-align: right;
}
}
.idiff.deletion {
background: $line-removed-dark;
}
.idiff.addition {
background: $line-added-dark;
}
......@@ -178,6 +178,7 @@ $table-border-gray: #f0f0f0;
$line-target-blue: #eaf3fc;
$line-select-yellow: #fcf8e7;
$line-select-yellow-dark: #f0e2bd;
/*
* Fonts
*/
......
......@@ -2,8 +2,8 @@ module DiffHelper
def mark_inline_diffs(old_line, new_line)
old_diffs, new_diffs = Gitlab::Diff::InlineDiff.new(old_line, new_line).inline_diffs
marked_old_line = Gitlab::Diff::InlineDiffMarker.new(old_line).mark(old_diffs)
marked_new_line = Gitlab::Diff::InlineDiffMarker.new(new_line).mark(new_diffs)
marked_old_line = Gitlab::Diff::InlineDiffMarker.new(old_line).mark(old_diffs, mode: :deletion)
marked_new_line = Gitlab::Diff::InlineDiffMarker.new(new_line).mark(new_diffs, mode: :addition)
[marked_old_line, marked_new_line]
end
......
......@@ -169,7 +169,14 @@ class SystemNoteService
#
# Returns the created Note object
def self.change_title(noteable, project, author, old_title)
body = "Title changed from **#{old_title}** to **#{noteable.title}**"
new_title = noteable.title.dup
old_diffs, new_diffs = Gitlab::Diff::InlineDiff.new(old_title, new_title).inline_diffs
marked_old_title = Gitlab::Diff::InlineDiffMarker.new(old_title).mark(old_diffs, mode: :deletion, markdown: true)
marked_new_title = Gitlab::Diff::InlineDiffMarker.new(new_title).mark(new_diffs, mode: :addition, markdown: true)
body = "Changed title: **#{marked_old_title}** → **#{marked_new_title}**"
create_note(noteable: noteable, project: project, author: author, note: body)
end
......
......@@ -11,11 +11,9 @@
= link_to "#diff-#{i}" do
- if diff_file.renamed_file
- old_path, new_path = mark_inline_diffs(diff_file.old_path, diff_file.new_path)
.filename.old
= old_path
= old_path
→
.filename.new
= new_path
= new_path
- else
%span
= diff_file.new_path
......
......@@ -8,6 +8,7 @@
* [Multiple underscores in words](#multiple-underscores-in-words)
* [URL auto-linking](#url-auto-linking)
* [Code and Syntax Highlighting](#code-and-syntax-highlighting)
* [Inline Diff](#inline-diff)
* [Emoji](#emoji)
* [Special GitLab references](#special-gitlab-references)
* [Task lists](#task-lists)
......@@ -153,6 +154,19 @@ s = "There is no highlighting for this."
But let's throw in a <b>tag</b>.
```
## Inline Diff
With inline diffs tags you can display {+ additions +} or [- deletions -].
The wrapping tags can be either curly braces or square brackets [+ additions +] or {- deletions -}.
However the wrapping tags cannot be mixed as such:
- {+ additions +]
- [+ additions +}
- {- deletions -]
- [- deletions -}
## Emoji
Sometimes you want to :monkey: around a bit and add some :star2: to your :speech_balloon:. Well we have a gift for you:
......
module Banzai
module Filter
class InlineDiffFilter < HTML::Pipeline::Filter
IGNORED_ANCESTOR_TAGS = %w(pre code tt).to_set
def call
search_text_nodes(doc).each do |node|
next if has_ancestor?(node, IGNORED_ANCESTOR_TAGS)
content = node.to_html
content = content.gsub(/(?:\[\-(.*?)\-\]|\{\-(.*?)\-\})/, '<span class="idiff left right deletion">\1\2</span>')
content = content.gsub(/(?:\[\+(.*?)\+\]|\{\+(.*?)\+\})/, '<span class="idiff left right addition">\1\2</span>')
next if html == content
node.replace(content)
end
doc
end
end
end
end
......@@ -23,7 +23,8 @@ module Banzai
Filter::LabelReferenceFilter,
Filter::MilestoneReferenceFilter,
Filter::TaskListFilter
Filter::TaskListFilter,
Filter::InlineDiffFilter
]
end
......
module Gitlab
module Diff
class InlineDiffMarker
MARKDOWN_SYMBOLS = {
addition: "+",
deletion: "-"
}
attr_accessor :raw_line, :rich_line
def initialize(raw_line, rich_line = raw_line)
......@@ -8,7 +13,7 @@ module Gitlab
@rich_line = ERB::Util.html_escape(rich_line)
end
def mark(line_inline_diffs)
def mark(line_inline_diffs, mode: nil, markdown: false)
return rich_line unless line_inline_diffs
marker_ranges = []
......@@ -20,13 +25,22 @@ module Gitlab
end
offset = 0
# Mark each range
marker_ranges.each_with_index do |range, i|
class_names = ["idiff"]
class_names << "left" if i == 0
class_names << "right" if i == marker_ranges.length - 1
offset = insert_around_range(rich_line, range, "<span class='#{class_names.join(" ")}'>", "</span>", offset)
# Mark each range
marker_ranges.each_with_index do |range, index|
before_content =
if markdown
"{#{MARKDOWN_SYMBOLS[mode]}"
else
"<span class='#{html_class_names(marker_ranges, mode, index)}'>"
end
after_content =
if markdown
"#{MARKDOWN_SYMBOLS[mode]}}"
else
"</span>"
end
offset = insert_around_range(rich_line, range, before_content, after_content, offset)
end
rich_line.html_safe
......@@ -34,6 +48,14 @@ module Gitlab
private
def html_class_names(marker_ranges, mode, index)
class_names = ["idiff"]
class_names << "left" if index == 0
class_names << "right" if index == marker_ranges.length - 1
class_names << mode if mode
class_names.join(" ")
end
# Mapping of character positions in the raw line, to the rich (highlighted) line
def position_mapping
@position_mapping ||= begin
......
......@@ -278,6 +278,10 @@ describe 'GitLab Markdown', feature: true do
it 'includes GollumTagsFilter' do
expect(doc).to parse_gollum_tags
end
it 'includes InlineDiffFilter' do
expect(doc).to parse_inline_diffs
end
end
# Fake a `current_user` helper
......
......@@ -243,3 +243,16 @@ References should be parseable even inside _<%= merge_request.to_reference %>_ e
- [[link-text|http://example.com/pdfs/gollum.pdf]]
- [[images/example.jpg]]
- [[http://example.com/images/example.jpg]]
### Inline Diffs
With inline diffs tags you can display {+ additions +} or [- deletions -].
The wrapping tags can be either curly braces or square brackets [+ additions +] or {- deletions -}.
However the wrapping tags can not be mixed as such -
- {+ additions +]
- [+ additions +}
- {- delletions -]
- [- delletions -}
......@@ -93,9 +93,9 @@ describe DiffHelper do
it "returns strings with marked inline diffs" do
marked_old_line, marked_new_line = mark_inline_diffs(old_line, new_line)
expect(marked_old_line).to eq("abc <span class='idiff left right'>&#39;def&#39;</span>")
expect(marked_old_line).to eq("abc <span class='idiff left right deletion'>&#39;def&#39;</span>")
expect(marked_old_line).to be_html_safe
expect(marked_new_line).to eq("abc <span class='idiff left right'>&quot;def&quot;</span>")
expect(marked_new_line).to eq("abc <span class='idiff left right addition'>&quot;def&quot;</span>")
expect(marked_new_line).to be_html_safe
end
end
......
require 'spec_helper'
describe Banzai::Filter::InlineDiffFilter, lib: true do
include FilterSpecHelper
it 'adds inline diff span tags for deletions when using square brackets' do
doc = "START [-something deleted-] END"
expect(filter(doc).to_html).to eq('START <span class="idiff left right deletion">something deleted</span> END')
end
it 'adds inline diff span tags for deletions when using curley braces' do
doc = "START {-something deleted-} END"
expect(filter(doc).to_html).to eq('START <span class="idiff left right deletion">something deleted</span> END')
end
it 'does not add inline diff span tags when a closing tag is not provided' do
doc = "START [- END"
expect(filter(doc).to_html).to eq(doc)
end
it 'adds inline span tags for additions when using square brackets' do
doc = "START [+something added+] END"
expect(filter(doc).to_html).to eq('START <span class="idiff left right addition">something added</span> END')
end
it 'adds inline span tags for additions when using curley braces' do
doc = "START {+something added+} END"
expect(filter(doc).to_html).to eq('START <span class="idiff left right addition">something added</span> END')
end
it 'does not add inline diff span tags when a closing addition tag is not provided' do
doc = "START {+ END"
expect(filter(doc).to_html).to eq(doc)
end
it 'does not add inline diff span tags when the tags do not match' do
examples = [
"{+ additions +]",
"[+ additions +}",
"{- delletions -]",
"[- delletions -}"
]
examples.each do |doc|
expect(filter(doc).to_html).to eq(doc)
end
end
it 'prevents user-land html being injected' do
doc = "START {+&lt;script&gt;alert('I steal cookies')&lt;/script&gt;+} END"
expect(filter(doc).to_html).to eq("START <span class=\"idiff left right addition\">&lt;script&gt;alert('I steal cookies')&lt;/script&gt;</span> END")
end
it 'preserves content inside pre tags' do
doc = "<pre>START {+something added+} END</pre>"
expect(filter(doc).to_html).to eq(doc)
end
it 'preserves content inside code tags' do
doc = "<code>START {+something added+} END</code>"
expect(filter(doc).to_html).to eq(doc)
end
it 'preserves content inside tt tags' do
doc = "<tt>START {+something added+} END</tt>"
expect(filter(doc).to_html).to eq(doc)
end
end
......@@ -75,10 +75,10 @@ describe Issues::UpdateService, services: true do
end
it 'creates system note about title change' do
note = find_note('Title changed')
note = find_note('Changed title:')
expect(note).not_to be_nil
expect(note.note).to eq 'Title changed from **Old title** to **New title**'
expect(note.note).to eq 'Changed title: **{-Old-} title** → **{+New+} title**'
end
it 'creates system note about confidentiality change' do
......
......@@ -92,10 +92,10 @@ describe MergeRequests::UpdateService, services: true do
end
it 'creates system note about title change' do
note = find_note('Title changed')
note = find_note('Changed title:')
expect(note).not_to be_nil
expect(note.note).to eq 'Title changed from **Old title** to **New title**'
expect(note.note).to eq 'Changed title: **{-Old-} title** → **{+New+} title**'
end
it 'creates system note about branch change' do
......
......@@ -241,7 +241,7 @@ describe SystemNoteService, services: true do
it 'sets the note text' do
expect(subject.note).
to eq "Title changed from **Old title** to **#{noteable.title}**"
to eq "Changed title: **{-Old title-}** → **{+#{noteable.title}+}**"
end
end
end
......
......@@ -168,6 +168,16 @@ module MarkdownMatchers
expect(actual).to have_selector('input[checked]', count: 3)
end
end
# InlineDiffFilter
matcher :parse_inline_diffs do
set_default_markdown_messages
match do |actual|
expect(actual).to have_selector('span.idiff.addition', count: 2)
expect(actual).to have_selector('span.idiff.deletion', count: 2)
end
end
end
# Monkeypatch the matcher DSL so that we can reduce some noisy duplication for
......
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