Enable CommonMark source line position informationbw-enable-sourcepos

This adds 'data-sourcepos' to tags, indicating which
line of markdown it came from.  Sets the stage for
intelligently manipulating specific lines of markdown.

18 files changed, 86 insertions, 23 deletions

diff --git a/app/models/concerns/cache_markdown_field.rb b/app/models/concerns/cache_markdown_field.rb
index a8c9e54f00c..73a27326f6c 100644
--- a/app/models/concerns/cache_markdown_field.rb
+++ b/app/models/concerns/cache_markdown_field.rb
@@ -15,7 +15,7 @@ module CacheMarkdownField
   # Increment this number every time the renderer changes its output
   CACHE_REDCARPET_VERSION         = 3
   CACHE_COMMONMARK_VERSION_START  = 10
-  CACHE_COMMONMARK_VERSION        = 12
+  CACHE_COMMONMARK_VERSION        = 13
 
   # changes to these attributes cause the cache to be invalidates
   INVALIDATED_BY = %w[author project].freeze
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 8ba09290e6d..e411eca9182 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 0b3c2390304..6526d848bfa 100644
--- a/spec/lib/banzai/filter/sanitization_filter_spec.rb
+++ b/spec/lib/banzai/filter/sanitization_filter_spec.rb
@@ -300,5 +300,12 @@ 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
   end
 end
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'))