summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBrett Walker <bwalker@gitlab.com>2018-12-20 20:22:27 -0600
committerBrett Walker <bwalker@gitlab.com>2019-01-21 13:45:16 -0600
commit45a04f93747a128588268395071f00d0af70acd7 (patch)
tree308c01c9537fa30f72f6a8649ce699a8045fe652
parentc141d0afb15366beb1cae8a240faf6aaeb632214 (diff)
downloadgitlab-ce-45a04f93747a128588268395071f00d0af70acd7.tar.gz
Enable CommonMark source line position information
This adds 'data-sourcepos' to tags, indicating which line of markdown it came from. Sets the stage for intelligently manipulating specific lines of markdown.
-rw-r--r--lib/banzai/filter/markdown_engines/common_mark.rb15
-rw-r--r--lib/banzai/filter/markdown_engines/redcarpet.rb2
-rw-r--r--lib/banzai/filter/markdown_filter.rb2
-rw-r--r--lib/banzai/filter/sanitization_filter.rb3
-rw-r--r--lib/banzai/filter/spaced_link_filter.rb2
-rw-r--r--lib/banzai/pipeline/atom_pipeline.rb3
-rw-r--r--lib/gitlab/gfm/reference_rewriter.rb2
-rw-r--r--spec/helpers/issuables_helper_spec.rb2
-rw-r--r--spec/lib/banzai/filter/markdown_filter_spec.rb40
-rw-r--r--spec/lib/banzai/filter/sanitization_filter_spec.rb7
-rw-r--r--spec/lib/banzai/pipeline/description_pipeline_spec.rb2
-rw-r--r--spec/models/concerns/cache_markdown_field_spec.rb17
-rw-r--r--spec/models/concerns/cacheable_attributes_spec.rb2
-rw-r--r--spec/models/concerns/redactable_spec.rb2
-rw-r--r--spec/requests/api/markdown_spec.rb2
-rw-r--r--spec/serializers/group_child_entity_spec.rb2
-rw-r--r--spec/support/helpers/markdown_feature.rb2
17 files changed, 85 insertions, 22 deletions
diff --git a/lib/banzai/filter/markdown_engines/common_mark.rb b/lib/banzai/filter/markdown_engines/common_mark.rb
index e52c0d15b31..81fdf0b2652 100644
--- a/lib/banzai/filter/markdown_engines/common_mark.rb
+++ b/lib/banzai/filter/markdown_engines/common_mark.rb
@@ -32,8 +32,13 @@ module Banzai
:DEFAULT # default rendering system. Nothing special.
].freeze
- def initialize
- @renderer = Banzai::Renderer::CommonMark::HTML.new(options: RENDER_OPTIONS)
+ RENDER_OPTIONS_SOURCEPOS = RENDER_OPTIONS + [
+ :SOURCEPOS # enable embedding of source position information
+ ].freeze
+
+ def initialize(context)
+ @context = context
+ @renderer = Banzai::Renderer::CommonMark::HTML.new(options: render_options)
end
def render(text)
@@ -41,6 +46,12 @@ module Banzai
@renderer.render(doc)
end
+
+ private
+
+ def render_options
+ @context&.dig(:no_sourcepos) ? RENDER_OPTIONS : RENDER_OPTIONS_SOURCEPOS
+ end
end
end
end
diff --git a/lib/banzai/filter/markdown_engines/redcarpet.rb b/lib/banzai/filter/markdown_engines/redcarpet.rb
index ec150d041ff..5b3f75096b1 100644
--- a/lib/banzai/filter/markdown_engines/redcarpet.rb
+++ b/lib/banzai/filter/markdown_engines/redcarpet.rb
@@ -20,7 +20,7 @@ module Banzai
tables: true
}.freeze
- def initialize
+ def initialize(context = nil)
html_renderer = Banzai::Renderer::Redcarpet::HTML.new
@renderer = ::Redcarpet::Markdown.new(html_renderer, OPTIONS)
end
diff --git a/lib/banzai/filter/markdown_filter.rb b/lib/banzai/filter/markdown_filter.rb
index cdf758472c1..242e39f5495 100644
--- a/lib/banzai/filter/markdown_filter.rb
+++ b/lib/banzai/filter/markdown_filter.rb
@@ -6,7 +6,7 @@ module Banzai
def initialize(text, context = nil, result = nil)
super(text, context, result)
- @renderer = renderer(context[:markdown_engine]).new
+ @renderer = renderer(context[:markdown_engine]).new(context)
@text = @text.delete("\r")
end
diff --git a/lib/banzai/filter/sanitization_filter.rb b/lib/banzai/filter/sanitization_filter.rb
index edc053638a8..a4a06eae7b7 100644
--- a/lib/banzai/filter/sanitization_filter.rb
+++ b/lib/banzai/filter/sanitization_filter.rb
@@ -41,6 +41,9 @@ module Banzai
whitelist[:elements].push('abbr')
whitelist[:attributes]['abbr'] = %w(title)
+ # Allow the 'data-sourcepos' from CommonMark on all elements
+ whitelist[:attributes][:all].push('data-sourcepos')
+
# Disallow `name` attribute globally, allow on `a`
whitelist[:attributes][:all].delete('name')
whitelist[:attributes]['a'].push('name')
diff --git a/lib/banzai/filter/spaced_link_filter.rb b/lib/banzai/filter/spaced_link_filter.rb
index c6a3a763c23..de76e3f9df4 100644
--- a/lib/banzai/filter/spaced_link_filter.rb
+++ b/lib/banzai/filter/spaced_link_filter.rb
@@ -73,7 +73,7 @@ module Banzai
html = Banzai::Filter::MarkdownFilter.call(transform_markdown(match), context)
# link is wrapped in a <p>, so strip that off
- html.sub('<p>', '').chomp('</p>')
+ html.sub(/<p[^>]*>/, '').chomp('</p>')
end
def spaced_link_filter(text)
diff --git a/lib/banzai/pipeline/atom_pipeline.rb b/lib/banzai/pipeline/atom_pipeline.rb
index 13a342351b6..c632910585d 100644
--- a/lib/banzai/pipeline/atom_pipeline.rb
+++ b/lib/banzai/pipeline/atom_pipeline.rb
@@ -6,7 +6,8 @@ module Banzai
def self.transform_context(context)
super(context).merge(
only_path: false,
- xhtml: true
+ xhtml: true,
+ no_sourcepos: true
)
end
end
diff --git a/lib/gitlab/gfm/reference_rewriter.rb b/lib/gitlab/gfm/reference_rewriter.rb
index 08d7db49ad7..4d82acd9d87 100644
--- a/lib/gitlab/gfm/reference_rewriter.rb
+++ b/lib/gitlab/gfm/reference_rewriter.rb
@@ -93,7 +93,7 @@ module Gitlab
end
def markdown(text)
- Banzai.render(text, project: @source_parent, no_original_data: true)
+ Banzai.render(text, project: @source_parent, no_original_data: true, no_sourcepos: true)
end
end
end
diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb
index 81231cca085..963d3029830 100644
--- a/spec/helpers/issuables_helper_spec.rb
+++ b/spec/helpers/issuables_helper_spec.rb
@@ -193,7 +193,7 @@ describe IssuablesHelper do
projectNamespace: @project.namespace.path,
initialTitleHtml: issue.title,
initialTitleText: issue.title,
- initialDescriptionHtml: '<p dir="auto">issue text</p>',
+ initialDescriptionHtml: '<p data-sourcepos="1:1-1:10" dir="auto">issue text</p>',
initialDescriptionText: 'issue text',
initialTaskStatus: '0 of 0 tasks completed'
}
diff --git a/spec/lib/banzai/filter/markdown_filter_spec.rb b/spec/lib/banzai/filter/markdown_filter_spec.rb
index cf49249756a..62f68b5f0cb 100644
--- a/spec/lib/banzai/filter/markdown_filter_spec.rb
+++ b/spec/lib/banzai/filter/markdown_filter_spec.rb
@@ -32,19 +32,19 @@ describe Banzai::Filter::MarkdownFilter do
it 'adds language to lang attribute when specified' do
result = filter("```html\nsome code\n```")
- expect(result).to start_with("<pre><code lang=\"html\">")
+ expect(result).to start_with('<pre data-sourcepos="1:1-3:3"><code lang="html">')
end
it 'does not add language to lang attribute when not specified' do
result = filter("```\nsome code\n```")
- expect(result).to start_with("<pre><code>")
+ expect(result).to start_with('<pre data-sourcepos="1:1-3:3"><code>')
end
it 'works with utf8 chars in language' do
result = filter("```日\nsome code\n```")
- expect(result).to start_with("<pre><code lang=\"日\">")
+ expect(result).to start_with('<pre data-sourcepos="1:1-3:3"><code lang="日">')
end
end
@@ -67,6 +67,38 @@ describe Banzai::Filter::MarkdownFilter do
end
end
+ describe 'source line position' do
+ context 'using CommonMark' do
+ before do
+ stub_const('Banzai::Filter::MarkdownFilter::DEFAULT_ENGINE', :common_mark)
+ end
+
+ it 'defaults to add data-sourcepos' do
+ result = filter('test')
+
+ expect(result).to eq '<p data-sourcepos="1:1-1:4">test</p>'
+ end
+
+ it 'disables data-sourcepos' do
+ result = filter('test', { no_sourcepos: true })
+
+ expect(result).to eq '<p>test</p>'
+ end
+ end
+
+ context 'using Redcarpet' do
+ before do
+ stub_const('Banzai::Filter::MarkdownFilter::DEFAULT_ENGINE', :redcarpet)
+ end
+
+ it 'does not support data-sourcepos' do
+ result = filter('test')
+
+ expect(result).to eq '<p>test</p>'
+ end
+ end
+ end
+
describe 'footnotes in tables' do
it 'processes footnotes in table cells' do
text = <<-MD.strip_heredoc
@@ -79,7 +111,7 @@ describe Banzai::Filter::MarkdownFilter do
result = filter(text)
- expect(result).to include('<td>foot <sup')
+ expect(result).to include('<td data-sourcepos="3:2-3:12">foot <sup')
expect(result).to include('<section class="footnotes">')
end
end
diff --git a/spec/lib/banzai/filter/sanitization_filter_spec.rb b/spec/lib/banzai/filter/sanitization_filter_spec.rb
index 836af18e0b6..f2a5d7b2c9f 100644
--- a/spec/lib/banzai/filter/sanitization_filter_spec.rb
+++ b/spec/lib/banzai/filter/sanitization_filter_spec.rb
@@ -301,6 +301,13 @@ describe Banzai::Filter::SanitizationFilter do
expect(act.to_html).to eq exp
end
+ it 'allows the `data-sourcepos` attribute globally' do
+ exp = %q{<p data-sourcepos="1:1-1:10">foo/bar.md</p>}
+ act = filter(exp)
+
+ expect(act.to_html).to eq exp
+ end
+
describe 'footnotes' do
it 'allows correct footnote id property on links' do
exp = %q{<a href="#fn1" id="fnref1">foo/bar.md</a>}
diff --git a/spec/lib/banzai/pipeline/description_pipeline_spec.rb b/spec/lib/banzai/pipeline/description_pipeline_spec.rb
index 8cce1b96698..1f9d53c6345 100644
--- a/spec/lib/banzai/pipeline/description_pipeline_spec.rb
+++ b/spec/lib/banzai/pipeline/description_pipeline_spec.rb
@@ -8,7 +8,7 @@ describe Banzai::Pipeline::DescriptionPipeline do
output = described_class.to_html(html, project: spy)
- output.gsub!(%r{\A<p dir="auto">(.*)</p>(.*)\z}, '\1\2') if unwrap
+ output.gsub!(%r{\A<p #{MarkdownFeature::SOURCEPOS_REGEX} dir="auto">(.*)</p>(.*)\z}, '\1\2') if unwrap
output
end
diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb
index f8d50e89d40..da5f184af06 100644
--- a/spec/models/concerns/cache_markdown_field_spec.rb
+++ b/spec/models/concerns/cache_markdown_field_spec.rb
@@ -67,10 +67,11 @@ describe CacheMarkdownField do
end
let(:markdown) { '`Foo`' }
- let(:html) { '<p dir="auto"><code>Foo</code></p>' }
+ let(:html) { '<p data-sourcepos="1:1-1:5" dir="auto"><code>Foo</code></p>' }
let(:updated_markdown) { '`Bar`' }
- let(:updated_html) { '<p dir="auto"><code>Bar</code></p>' }
+ let(:updated_html) { '<p data-sourcepos="1:1-1:5" dir="auto"><code>Bar</code></p>' }
+ let(:updated_redcarpet_html) { '<p dir="auto"><code>Bar</code></p>' }
let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION) }
@@ -95,13 +96,16 @@ describe CacheMarkdownField do
context 'a changed markdown field' do
shared_examples 'with cache version' do |cache_version|
let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) }
+ let(:updated_version_html) do
+ cache_version == CacheMarkdownField::CACHE_REDCARPET_VERSION ? updated_redcarpet_html : updated_html
+ end
before do
thing.foo = updated_markdown
thing.save
end
- it { expect(thing.foo_html).to eq(updated_html) }
+ it { expect(thing.foo_html).to eq(updated_version_html) }
it { expect(thing.cached_markdown_version).to eq(cache_version) }
end
@@ -268,6 +272,9 @@ describe CacheMarkdownField do
describe '#refresh_markdown_cache!' do
shared_examples 'with cache version' do |cache_version|
let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) }
+ let(:updated_version_html) do
+ cache_version == CacheMarkdownField::CACHE_REDCARPET_VERSION ? updated_redcarpet_html : updated_html
+ end
before do
thing.foo = updated_markdown
@@ -276,7 +283,7 @@ describe CacheMarkdownField do
it 'fills all html fields' do
thing.refresh_markdown_cache!
- expect(thing.foo_html).to eq(updated_html)
+ expect(thing.foo_html).to eq(updated_version_html)
expect(thing.foo_html_changed?).to be_truthy
expect(thing.baz_html_changed?).to be_truthy
end
@@ -291,7 +298,7 @@ describe CacheMarkdownField do
it 'saves the changes using #update_columns' do
expect(thing).to receive(:persisted?).and_return(true)
expect(thing).to receive(:update_columns)
- .with("foo_html" => updated_html, "baz_html" => "", "cached_markdown_version" => cache_version)
+ .with("foo_html" => updated_version_html, "baz_html" => "", "cached_markdown_version" => cache_version)
thing.refresh_markdown_cache!
end
diff --git a/spec/models/concerns/cacheable_attributes_spec.rb b/spec/models/concerns/cacheable_attributes_spec.rb
index 689e7d3058f..9bc381694d6 100644
--- a/spec/models/concerns/cacheable_attributes_spec.rb
+++ b/spec/models/concerns/cacheable_attributes_spec.rb
@@ -177,7 +177,7 @@ describe CacheableAttributes do
cache_record = Appearance.current
expect(cache_record.description).to eq('**Hello**')
- expect(cache_record.description_html).to eq('<p dir="auto"><strong>Hello</strong></p>')
+ expect(cache_record.description_html).to eq('<p data-sourcepos="1:1-1:9" dir="auto"><strong>Hello</strong></p>')
end
end
end
diff --git a/spec/models/concerns/redactable_spec.rb b/spec/models/concerns/redactable_spec.rb
index 7d320edd492..fe17970d75f 100644
--- a/spec/models/concerns/redactable_spec.rb
+++ b/spec/models/concerns/redactable_spec.rb
@@ -35,7 +35,7 @@ describe Redactable do
expected = 'some text /sent_notifications/REDACTED/unsubscribe more text'
expect(model[field]).to eq expected
- expect(model["#{field}_html"]).to eq "<p dir=\"auto\">#{expected}</p>"
+ expect(model["#{field}_html"]).to eq "<p data-sourcepos=\"1:1-1:60\" dir=\"auto\">#{expected}</p>"
end
end
diff --git a/spec/requests/api/markdown_spec.rb b/spec/requests/api/markdown_spec.rb
index e82ef002d32..386679d9dfd 100644
--- a/spec/requests/api/markdown_spec.rb
+++ b/spec/requests/api/markdown_spec.rb
@@ -15,7 +15,7 @@ describe API::Markdown do
expect(response).to have_http_status(201)
expect(response.headers["Content-Type"]).to eq("application/json")
expect(json_response).to be_a(Hash)
- expect(json_response["html"]).to eq("<p>#{text}</p>")
+ expect(json_response["html"]).to eq("<p data-sourcepos=\"1:1-1:28\">#{text}</p>")
end
end
diff --git a/spec/serializers/group_child_entity_spec.rb b/spec/serializers/group_child_entity_spec.rb
index dbc40bddc30..7f2ddc9546a 100644
--- a/spec/serializers/group_child_entity_spec.rb
+++ b/spec/serializers/group_child_entity_spec.rb
@@ -102,7 +102,7 @@ describe GroupChildEntity do
let(:description) { ':smile:' }
it 'has the correct markdown_description' do
- expect(json[:markdown_description]).to eq('<p dir="auto"><gl-emoji title="smiling face with open mouth and smiling eyes" data-name="smile" data-unicode-version="6.0">😄</gl-emoji></p>')
+ expect(json[:markdown_description]).to eq('<p data-sourcepos="1:1-1:7" dir="auto"><gl-emoji title="smiling face with open mouth and smiling eyes" data-name="smile" data-unicode-version="6.0">😄</gl-emoji></p>')
end
end
diff --git a/spec/support/helpers/markdown_feature.rb b/spec/support/helpers/markdown_feature.rb
index 96401379cf0..e60e5ee6783 100644
--- a/spec/support/helpers/markdown_feature.rb
+++ b/spec/support/helpers/markdown_feature.rb
@@ -10,6 +10,8 @@
class MarkdownFeature
include FactoryBot::Syntax::Methods
+ SOURCEPOS_REGEX = 'data-sourcepos="\d*:\d*-\d*:\d*"'.freeze
+
attr_reader :fixture_path
def initialize(fixture_path = Rails.root.join('spec/fixtures/markdown.md.erb'))