Commit 45a04f93 authored by Brett Walker's avatar Brett Walker

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.
parent c141d0af
...@@ -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&.dig(: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,7 @@ module Banzai ...@@ -73,7 +73,7 @@ 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>') html.sub(/<p[^>]*>/, '').chomp('</p>')
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
......
...@@ -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
......
...@@ -193,7 +193,7 @@ describe IssuablesHelper do ...@@ -193,7 +193,7 @@ describe IssuablesHelper do
projectNamespace: @project.namespace.path, projectNamespace: @project.namespace.path,
initialTitleHtml: issue.title, initialTitleHtml: issue.title,
initialTitleText: 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', initialDescriptionText: 'issue text',
initialTaskStatus: '0 of 0 tasks completed' initialTaskStatus: '0 of 0 tasks completed'
} }
......
...@@ -32,19 +32,19 @@ describe Banzai::Filter::MarkdownFilter do ...@@ -32,19 +32,19 @@ describe Banzai::Filter::MarkdownFilter do
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```")
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 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```")
expect(result).to start_with("<pre><code>") expect(result).to start_with('<pre data-sourcepos="1:1-3:3"><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```")
expect(result).to start_with("<pre><code lang=\"\">") expect(result).to start_with('<pre data-sourcepos="1:1-3:3"><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
...@@ -79,7 +111,7 @@ describe Banzai::Filter::MarkdownFilter do ...@@ -79,7 +111,7 @@ describe Banzai::Filter::MarkdownFilter do
result = filter(text) 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">') expect(result).to include('<section class="footnotes">')
end end
end end
......
...@@ -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>}
......
...@@ -8,7 +8,7 @@ describe Banzai::Pipeline::DescriptionPipeline do ...@@ -8,7 +8,7 @@ describe Banzai::Pipeline::DescriptionPipeline do
output = described_class.to_html(html, project: spy) 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 output
end end
......
...@@ -67,10 +67,11 @@ describe CacheMarkdownField do ...@@ -67,10 +67,11 @@ describe CacheMarkdownField do
end end
let(:markdown) { '`Foo`' } 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_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) } let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION) }
...@@ -95,13 +96,16 @@ describe CacheMarkdownField do ...@@ -95,13 +96,16 @@ describe CacheMarkdownField do
context 'a changed markdown field' do context 'a changed markdown field' do
shared_examples 'with cache version' do |cache_version| shared_examples 'with cache version' do |cache_version|
let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: 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 before do
thing.foo = updated_markdown thing.foo = updated_markdown
thing.save thing.save
end 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) } it { expect(thing.cached_markdown_version).to eq(cache_version) }
end end
...@@ -268,6 +272,9 @@ describe CacheMarkdownField do ...@@ -268,6 +272,9 @@ describe CacheMarkdownField do
describe '#refresh_markdown_cache!' do describe '#refresh_markdown_cache!' do
shared_examples 'with cache version' do |cache_version| shared_examples 'with cache version' do |cache_version|
let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: 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 before do
thing.foo = updated_markdown thing.foo = updated_markdown
...@@ -276,7 +283,7 @@ describe CacheMarkdownField do ...@@ -276,7 +283,7 @@ describe CacheMarkdownField do
it 'fills all html fields' do it 'fills all html fields' do
thing.refresh_markdown_cache! 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.foo_html_changed?).to be_truthy
expect(thing.baz_html_changed?).to be_truthy expect(thing.baz_html_changed?).to be_truthy
end end
...@@ -291,7 +298,7 @@ describe CacheMarkdownField do ...@@ -291,7 +298,7 @@ describe CacheMarkdownField do
it 'saves the changes using #update_columns' do it 'saves the changes using #update_columns' do
expect(thing).to receive(:persisted?).and_return(true) expect(thing).to receive(:persisted?).and_return(true)
expect(thing).to receive(:update_columns) 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! thing.refresh_markdown_cache!
end end
......
...@@ -177,7 +177,7 @@ describe CacheableAttributes do ...@@ -177,7 +177,7 @@ describe CacheableAttributes do
cache_record = Appearance.current cache_record = Appearance.current
expect(cache_record.description).to eq('**Hello**') 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 end
end end
......
...@@ -35,7 +35,7 @@ describe Redactable do ...@@ -35,7 +35,7 @@ describe Redactable do
expected = 'some text /sent_notifications/REDACTED/unsubscribe more text' expected = 'some text /sent_notifications/REDACTED/unsubscribe more text'
expect(model[field]).to eq expected 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
end end
......
...@@ -15,7 +15,7 @@ describe API::Markdown do ...@@ -15,7 +15,7 @@ describe API::Markdown do
expect(response).to have_http_status(201) expect(response).to have_http_status(201)
expect(response.headers["Content-Type"]).to eq("application/json") expect(response.headers["Content-Type"]).to eq("application/json")
expect(json_response).to be_a(Hash) 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
end end
......
...@@ -102,7 +102,7 @@ describe GroupChildEntity do ...@@ -102,7 +102,7 @@ describe GroupChildEntity do
let(:description) { ':smile:' } let(:description) { ':smile:' }
it 'has the correct markdown_description' do 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
end end
......
...@@ -10,6 +10,8 @@ ...@@ -10,6 +10,8 @@
class MarkdownFeature class MarkdownFeature
include FactoryBot::Syntax::Methods include FactoryBot::Syntax::Methods
SOURCEPOS_REGEX = 'data-sourcepos="\d*:\d*-\d*:\d*"'.freeze
attr_reader :fixture_path attr_reader :fixture_path
def initialize(fixture_path = Rails.root.join('spec/fixtures/markdown.md.erb')) def initialize(fixture_path = Rails.root.join('spec/fixtures/markdown.md.erb'))
......
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