Commit a2455196 authored by Robert Speicher's avatar Robert Speicher

Merge branch 'bw-enable-sourcepos' into 'master'

Enable CommonMark source line position information

See merge request gitlab-org/gitlab-ce!23971
parents 4f8bdb01 7bc0fbe2
...@@ -32,8 +32,13 @@ module Banzai ...@@ -32,8 +32,13 @@ module Banzai
:DEFAULT # default rendering system. Nothing special. :DEFAULT # default rendering system. Nothing special.
].freeze ].freeze
def initialize RENDER_OPTIONS_SOURCEPOS = RENDER_OPTIONS + [
@renderer = Banzai::Renderer::CommonMark::HTML.new(options: 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 end
def render(text) def render(text)
...@@ -41,6 +46,12 @@ module Banzai ...@@ -41,6 +46,12 @@ module Banzai
@renderer.render(doc) @renderer.render(doc)
end end
private
def render_options
@context[:no_sourcepos] ? RENDER_OPTIONS : RENDER_OPTIONS_SOURCEPOS
end
end end
end end
end end
......
...@@ -20,7 +20,7 @@ module Banzai ...@@ -20,7 +20,7 @@ module Banzai
tables: true tables: true
}.freeze }.freeze
def initialize def initialize(context = nil)
html_renderer = Banzai::Renderer::Redcarpet::HTML.new html_renderer = Banzai::Renderer::Redcarpet::HTML.new
@renderer = ::Redcarpet::Markdown.new(html_renderer, OPTIONS) @renderer = ::Redcarpet::Markdown.new(html_renderer, OPTIONS)
end end
......
...@@ -6,7 +6,7 @@ module Banzai ...@@ -6,7 +6,7 @@ module Banzai
def initialize(text, context = nil, result = nil) def initialize(text, context = nil, result = nil)
super(text, context, result) super(text, context, result)
@renderer = renderer(context[:markdown_engine]).new @renderer = renderer(context[:markdown_engine]).new(context)
@text = @text.delete("\r") @text = @text.delete("\r")
end end
......
...@@ -41,6 +41,9 @@ module Banzai ...@@ -41,6 +41,9 @@ module Banzai
whitelist[:elements].push('abbr') whitelist[:elements].push('abbr')
whitelist[:attributes]['abbr'] = %w(title) 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` # Disallow `name` attribute globally, allow on `a`
whitelist[:attributes][:all].delete('name') whitelist[:attributes][:all].delete('name')
whitelist[:attributes]['a'].push('name') whitelist[:attributes]['a'].push('name')
......
...@@ -73,7 +73,8 @@ module Banzai ...@@ -73,7 +73,8 @@ module Banzai
html = Banzai::Filter::MarkdownFilter.call(transform_markdown(match), context) html = Banzai::Filter::MarkdownFilter.call(transform_markdown(match), context)
# link is wrapped in a <p>, so strip that off # link is wrapped in a <p>, so strip that off
html.sub('<p>', '').chomp('</p>') p_node = Nokogiri::HTML.fragment(html).at_css('p')
p_node ? p_node.children.to_html : html
end end
def spaced_link_filter(text) def spaced_link_filter(text)
......
...@@ -6,7 +6,8 @@ module Banzai ...@@ -6,7 +6,8 @@ module Banzai
def self.transform_context(context) def self.transform_context(context)
super(context).merge( super(context).merge(
only_path: false, only_path: false,
xhtml: true xhtml: true,
no_sourcepos: true
) )
end end
end end
......
...@@ -14,6 +14,12 @@ module Banzai ...@@ -14,6 +14,12 @@ module Banzai
Filter::ExternalLinkFilter Filter::ExternalLinkFilter
] ]
end end
def self.transform_context(context)
super(context).merge(
no_sourcepos: true
)
end
end end
end end
end end
...@@ -11,7 +11,8 @@ module Banzai ...@@ -11,7 +11,8 @@ module Banzai
def self.transform_context(context) def self.transform_context(context)
super(context).merge( super(context).merge(
only_path: false only_path: false,
no_sourcepos: true
) )
end end
end end
......
...@@ -27,6 +27,12 @@ module Banzai ...@@ -27,6 +27,12 @@ module Banzai
Filter::CommitReferenceFilter Filter::CommitReferenceFilter
] ]
end end
def self.transform_context(context)
super(context).merge(
no_sourcepos: true
)
end
end end
end end
end end
...@@ -93,7 +93,7 @@ module Gitlab ...@@ -93,7 +93,7 @@ module Gitlab
end end
def markdown(text) 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 end
end end
......
...@@ -173,6 +173,7 @@ describe IssuablesHelper do ...@@ -173,6 +173,7 @@ describe IssuablesHelper do
before do before do
allow(helper).to receive(:current_user).and_return(user) allow(helper).to receive(:current_user).and_return(user)
allow(helper).to receive(:can?).and_return(true) allow(helper).to receive(:can?).and_return(true)
stub_commonmark_sourcepos_disabled
end end
it 'returns the correct json for an issue' do it 'returns the correct json for an issue' do
......
...@@ -30,21 +30,21 @@ describe Banzai::Filter::MarkdownFilter do ...@@ -30,21 +30,21 @@ describe Banzai::Filter::MarkdownFilter do
end end
it 'adds language to lang attribute when specified' do it 'adds language to lang attribute when specified' do
result = filter("```html\nsome code\n```") result = filter("```html\nsome code\n```", no_sourcepos: true)
expect(result).to start_with("<pre><code lang=\"html\">") expect(result).to start_with('<pre><code lang="html">')
end end
it 'does not add language to lang attribute when not specified' do it 'does not add language to lang attribute when not specified' do
result = filter("```\nsome code\n```") result = filter("```\nsome code\n```", no_sourcepos: true)
expect(result).to start_with("<pre><code>") expect(result).to start_with('<pre><code>')
end end
it 'works with utf8 chars in language' do it 'works with utf8 chars in language' do
result = filter("```日\nsome code\n```") result = filter("```日\nsome code\n```", no_sourcepos: true)
expect(result).to start_with("<pre><code lang=\"\">") expect(result).to start_with('<pre><code lang="日">')
end end
end end
...@@ -67,6 +67,38 @@ describe Banzai::Filter::MarkdownFilter do ...@@ -67,6 +67,38 @@ describe Banzai::Filter::MarkdownFilter do
end end
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 describe 'footnotes in tables' do
it 'processes footnotes in table cells' do it 'processes footnotes in table cells' do
text = <<-MD.strip_heredoc text = <<-MD.strip_heredoc
...@@ -77,7 +109,7 @@ describe Banzai::Filter::MarkdownFilter do ...@@ -77,7 +109,7 @@ describe Banzai::Filter::MarkdownFilter do
[^1]: a footnote [^1]: a footnote
MD MD
result = filter(text) result = filter(text, no_sourcepos: true)
expect(result).to include('<td>foot <sup') expect(result).to include('<td>foot <sup')
expect(result).to include('<section class="footnotes">') expect(result).to include('<section class="footnotes">')
......
...@@ -301,6 +301,13 @@ describe Banzai::Filter::SanitizationFilter do ...@@ -301,6 +301,13 @@ describe Banzai::Filter::SanitizationFilter do
expect(act.to_html).to eq exp expect(act.to_html).to eq exp
end 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 describe 'footnotes' do
it 'allows correct footnote id property on links' do it 'allows correct footnote id property on links' do
exp = %q{<a href="#fn1" id="fnref1">foo/bar.md</a>} exp = %q{<a href="#fn1" id="fnref1">foo/bar.md</a>}
......
...@@ -13,6 +13,10 @@ describe Banzai::Pipeline::DescriptionPipeline do ...@@ -13,6 +13,10 @@ describe Banzai::Pipeline::DescriptionPipeline do
output output
end end
before do
stub_commonmark_sourcepos_disabled
end
it 'uses a limited whitelist' do it 'uses a limited whitelist' do
doc = parse('# Description') doc = parse('# Description')
......
...@@ -54,6 +54,8 @@ describe Banzai::Pipeline::FullPipeline do ...@@ -54,6 +54,8 @@ describe Banzai::Pipeline::FullPipeline do
end end
it 'properly adds the necessary ids and classes' do it 'properly adds the necessary ids and classes' do
stub_commonmark_sourcepos_disabled
expect(html.lines.map(&:strip).join("\n")).to eq filtered_footnote expect(html.lines.map(&:strip).join("\n")).to eq filtered_footnote
end end
end end
......
...@@ -74,6 +74,10 @@ describe CacheMarkdownField do ...@@ -74,6 +74,10 @@ describe CacheMarkdownField do
let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION) } let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION) }
before do
stub_commonmark_sourcepos_disabled
end
describe '.attributes' do describe '.attributes' do
it 'excludes cache attributes' do it 'excludes cache attributes' do
expect(thing.attributes.keys.sort).to eq(%w[bar baz foo]) expect(thing.attributes.keys.sort).to eq(%w[bar baz foo])
......
...@@ -159,6 +159,10 @@ describe CacheableAttributes do ...@@ -159,6 +159,10 @@ describe CacheableAttributes do
describe 'edge cases' do describe 'edge cases' do
describe 'caching behavior', :use_clean_rails_memory_store_caching do describe 'caching behavior', :use_clean_rails_memory_store_caching do
before do
stub_commonmark_sourcepos_disabled
end
it 'retrieves upload fields properly' do it 'retrieves upload fields properly' do
ar_record = create(:appearance, :with_logo) ar_record = create(:appearance, :with_logo)
ar_record.cache! ar_record.cache!
......
require 'spec_helper' require 'spec_helper'
describe Redactable do describe Redactable do
before do
stub_commonmark_sourcepos_disabled
end
shared_examples 'model with redactable field' do shared_examples 'model with redactable field' do
it 'redacts unsubscribe token' do it 'redacts unsubscribe token' do
model[field] = 'some text /sent_notifications/00000000000000000000000000000000/unsubscribe more text' model[field] = 'some text /sent_notifications/00000000000000000000000000000000/unsubscribe more text'
......
...@@ -7,6 +7,8 @@ describe API::Markdown do ...@@ -7,6 +7,8 @@ describe API::Markdown do
let(:user) {} # No-op. It gets overwritten in the contexts below. let(:user) {} # No-op. It gets overwritten in the contexts below.
before do before do
stub_commonmark_sourcepos_disabled
post api("/markdown", user), params: params post api("/markdown", user), params: params
end end
......
...@@ -10,6 +10,7 @@ describe GroupChildEntity do ...@@ -10,6 +10,7 @@ describe GroupChildEntity do
before do before do
allow(request).to receive(:current_user).and_return(user) allow(request).to receive(:current_user).and_return(user)
stub_commonmark_sourcepos_disabled
end end
shared_examples 'group child json' do shared_examples 'group child json' do
......
...@@ -62,6 +62,12 @@ module StubGitlabCalls ...@@ -62,6 +62,12 @@ module StubGitlabCalls
end end
end end
def stub_commonmark_sourcepos_disabled
allow_any_instance_of(Banzai::Filter::MarkdownEngines::CommonMark)
.to receive(:render_options)
.and_return(Banzai::Filter::MarkdownEngines::CommonMark::RENDER_OPTIONS)
end
private private
def stub_container_registry_tag_manifest_content def stub_container_registry_tag_manifest_content
......
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