Merge branch 'rs-cleanup-redcarpet-renderer' into 'master'

Decouple Gitlab::Markdown from the GitlabMarkdownHelper

This module is now the sole source of knowledge for *how* we render
Markdown (and GFM).

See merge request !1223

app/helpers/gitlab_markdown_helper.rb

@@ -1,9 +1,6 @@
 require 'nokogiri'
 
 module GitlabMarkdownHelper
-  include Gitlab::Markdown
-  include PreferencesHelper
-
   # Use this in places where you would normally use link_to(gfm(...), ...).
   #
   # It solves a problem occurring with nested links (i.e.
@@ -22,7 +19,7 @@ module GitlabMarkdownHelper
                      escape_once(body)
                    end
 
-    gfm_body = gfm(escaped_body, {}, html_options)
+    gfm_body = Gitlab::Markdown.gfm(escaped_body, project: @project, current_user: current_user)
 
     fragment = Nokogiri::XML::DocumentFragment.parse(gfm_body)
     if fragment.children.size == 1 && fragment.children[0].name == 'a'
@@ -39,32 +36,38 @@ module GitlabMarkdownHelper
       end
     end
 
+    # Add any custom CSS classes to the GFM-generated reference links
+    if html_options[:class]
+      fragment.css('a.gfm').add_class(html_options[:class])
+    end
+
     fragment.to_html.html_safe
   end
 
-  MARKDOWN_OPTIONS = {
-    no_intra_emphasis:   true,
-    tables:              true,
-    fenced_code_blocks:  true,
-    strikethrough:       true,
-    lax_spacing:         true,
-    space_after_headers: true,
-    superscript:         true,
-    footnotes:           true
-  }.freeze
+  def markdown(text, context = {})
+    context.merge!(
+      current_user: current_user,
+      path:         @path,
+      project:      @project,
+      project_wiki: @project_wiki,
+      ref:          @ref
+    )
 
-  def markdown(text, options={})
-    unless @markdown && options == @options
-      @options = options
+    Gitlab::Markdown.render(text, context)
+  end
 
-      # see https://github.com/vmg/redcarpet#darling-i-packed-you-a-couple-renderers-for-lunch
-      rend = Redcarpet::Render::GitlabHTML.new(self, options)
+  # TODO (rspeicher): Remove all usages of this helper and just call `markdown`
+  # with a custom pipeline depending on the content being rendered
+  def gfm(text, options = {})
+    options.merge!(
+      current_user: current_user,
+      path:         @path,
+      project:      @project,
+      project_wiki: @project_wiki,
+      ref:          @ref
+    )
 
-      # see https://github.com/vmg/redcarpet#and-its-like-really-simple-to-use
-      @markdown = Redcarpet::Markdown.new(rend, MARKDOWN_OPTIONS)
-    end
-
-    @markdown.render(text).html_safe
+    Gitlab::Markdown.gfm(text, options)
   end
 
   def asciidoc(text)

lib/gitlab/markdown.rb

@@ -5,6 +5,32 @@ module Gitlab
   #
   # See the files in `lib/gitlab/markdown/` for specific processing information.
   module Markdown
+    # Convert a Markdown String into an HTML-safe String of HTML
+    #
+    # markdown - Markdown String
+    # context  - Hash of context options passed to our HTML Pipeline
+    #
+    # Returns an HTML-safe String
+    def self.render(markdown, context = {})
+      html = renderer.render(markdown)
+      html = gfm(html, context)
+
+      html.html_safe
+    end
+
+    # Convert a Markdown String into HTML without going through the HTML
+    # Pipeline.
+    #
+    # Note that because the pipeline is skipped, SanitizationFilter is as well.
+    # Do not output the result of this method to the user.
+    #
+    # markdown - Markdown String
+    #
+    # Returns a String
+    def self.render_without_gfm(markdown)
+      renderer.render(markdown)
+    end
+
     # Provide autoload paths for filters to prevent a circular dependency error
     autoload :AutolinkFilter,               'gitlab/markdown/autolink_filter'
     autoload :CommitRangeReferenceFilter,   'gitlab/markdown/commit_range_reference_filter'
@@ -18,6 +44,7 @@ module Gitlab
     autoload :RelativeLinkFilter,           'gitlab/markdown/relative_link_filter'
     autoload :SanitizationFilter,           'gitlab/markdown/sanitization_filter'
     autoload :SnippetReferenceFilter,       'gitlab/markdown/snippet_reference_filter'
+    autoload :SyntaxHighlightFilter,        'gitlab/markdown/syntax_highlight_filter'
     autoload :TableOfContentsFilter,        'gitlab/markdown/table_of_contents_filter'
     autoload :TaskListFilter,               'gitlab/markdown/task_list_filter'
     autoload :UserReferenceFilter,          'gitlab/markdown/user_reference_filter'
@@ -28,8 +55,7 @@ module Gitlab
     # options      - A Hash of options used to customize output (default: {}):
     #                :xhtml               - output XHTML instead of HTML
     #                :reference_only_path - Use relative path for reference links
-    # html_options - extra options for the reference links as given to link_to
-    def gfm(text, options = {}, html_options = {})
+    def self.gfm(text, options = {})
       return text if text.nil?
 
       # Duplicate the string so we don't alter the original, then call to_str
@@ -40,8 +66,8 @@ module Gitlab
       options.reverse_merge!(
         xhtml:                false,
         reference_only_path:  true,
-        project:              @project,
-        current_user:         current_user
+        project:              options[:project],
+        current_user:         options[:current_user]
       )
 
       @pipeline ||= HTML::Pipeline.new(filters)
@@ -61,12 +87,11 @@ module Gitlab
         current_user:    options[:current_user],
         only_path:       options[:reference_only_path],
         project:         options[:project],
-        reference_class: html_options[:class],
 
         # RelativeLinkFilter
-        ref:            @ref,
-        requested_path: @path,
-        project_wiki:   @project_wiki
+        ref:            options[:ref],
+        requested_path: options[:path],
+        project_wiki:   options[:project_wiki]
       }
 
       result = @pipeline.call(text, context)
@@ -83,14 +108,36 @@ module Gitlab
 
     private
 
+    def self.renderer
+      @markdown ||= begin
+        renderer = Redcarpet::Render::HTML.new
+        Redcarpet::Markdown.new(renderer, redcarpet_options)
+      end
+    end
+
+    def self.redcarpet_options
+      # https://github.com/vmg/redcarpet#and-its-like-really-simple-to-use
+      @redcarpet_options ||= {
+        fenced_code_blocks:  true,
+        footnotes:           true,
+        lax_spacing:         true,
+        no_intra_emphasis:   true,
+        space_after_headers: true,
+        strikethrough:       true,
+        superscript:         true,
+        tables:              true
+      }.freeze
+    end
+
     # Filters used in our pipeline
     #
     # SanitizationFilter should come first so that all generated reference HTML
     # goes through untouched.
     #
     # See https://github.com/jch/html-pipeline#filters for more filters.
-    def filters
+    def self.filters
       [
+        Gitlab::Markdown::SyntaxHighlightFilter,
         Gitlab::Markdown::SanitizationFilter,
 
         Gitlab::Markdown::RelativeLinkFilter,

lib/gitlab/markdown/autolink_filter.rb

@@ -1,3 +1,4 @@
+require 'gitlab/markdown'
 require 'html/pipeline/filter'
 require 'uri'
 

lib/gitlab/markdown/commit_range_reference_filter.rb

@@ -1,3 +1,5 @@
+require 'gitlab/markdown'
+
 module Gitlab
   module Markdown
     # HTML filter that replaces commit range references with links.

lib/gitlab/markdown/commit_reference_filter.rb

@@ -1,3 +1,5 @@
+require 'gitlab/markdown'
+
 module Gitlab
   module Markdown
     # HTML filter that replaces commit references with links.

lib/gitlab/markdown/cross_project_reference.rb

@@ -1,3 +1,5 @@
+require 'gitlab/markdown'
+
 module Gitlab
   module Markdown
     # Common methods for ReferenceFilters that support an optional cross-project

lib/gitlab/markdown/emoji_filter.rb

@@ -1,6 +1,7 @@
+require 'action_controller'
+require 'gitlab/markdown'
 require 'gitlab_emoji'
 require 'html/pipeline/filter'
-require 'action_controller'
 
 module Gitlab
   module Markdown

lib/gitlab/markdown/external_issue_reference_filter.rb

@@ -1,3 +1,5 @@
+require 'gitlab/markdown'
+
 module Gitlab
   module Markdown
     # HTML filter that replaces external issue tracker references with links.

lib/gitlab/markdown/external_link_filter.rb

@@ -1,3 +1,4 @@
+require 'gitlab/markdown'
 require 'html/pipeline/filter'
 
 module Gitlab

lib/gitlab/markdown/issue_reference_filter.rb

@@ -1,3 +1,5 @@
+require 'gitlab/markdown'
+
 module Gitlab
   module Markdown
     # HTML filter that replaces issue references with links. References to

lib/gitlab/markdown/label_reference_filter.rb

@@ -1,3 +1,5 @@
+require 'gitlab/markdown'
+
 module Gitlab
   module Markdown
     # HTML filter that replaces label references with links.

lib/gitlab/markdown/merge_request_reference_filter.rb

@@ -1,3 +1,5 @@
+require 'gitlab/markdown'
+
 module Gitlab
   module Markdown
     # HTML filter that replaces merge request references with links. References

lib/gitlab/markdown/reference_filter.rb

@@ -1,4 +1,5 @@
 require 'active_support/core_ext/string/output_safety'
+require 'gitlab/markdown'
 require 'html/pipeline/filter'
 
 module Gitlab
@@ -9,7 +10,6 @@ module Gitlab
     #
     # Context options:
     #   :project (required) - Current project, ignored if reference is cross-project.
-    #   :reference_class    - Custom CSS class added to reference links.
     #   :only_path          - Generate path-only links.
     #
     # Results:
@@ -70,7 +70,7 @@ module Gitlab
       end
 
       def reference_class(type)
-        "gfm gfm-#{type} #{context[:reference_class]}".strip
+        "gfm gfm-#{type}"
       end
 
       # Iterate through the document's text nodes, yielding the current node's

lib/gitlab/markdown/relative_link_filter.rb

@@ -1,3 +1,4 @@
+require 'gitlab/markdown'
 require 'html/pipeline/filter'
 require 'uri'
 

lib/gitlab/markdown/sanitization_filter.rb

@@ -1,3 +1,4 @@
+require 'gitlab/markdown'
 require 'html/pipeline/filter'
 require 'html/pipeline/sanitization_filter'
 

lib/gitlab/markdown/snippet_reference_filter.rb

@@ -1,3 +1,5 @@
+require 'gitlab/markdown'
+
 module Gitlab
   module Markdown
     # HTML filter that replaces snippet references with links. References to

lib/gitlab/markdown/syntax_highlight_filter.rb

@@ -0,0 +1,39 @@
+require 'gitlab/markdown'
+require 'html/pipeline/filter'
+require 'rouge/plugins/redcarpet'
+
+module Gitlab
+  module Markdown
+    # HTML Filter to highlight fenced code blocks
+    #
+    class SyntaxHighlightFilter < HTML::Pipeline::Filter
+      include Rouge::Plugins::Redcarpet
+
+      def call
+        doc.search('pre > code').each do |node|
+          highlight_node(node)
+        end
+
+        doc
+      end
+
+      def highlight_node(node)
+        language = node.attr('class')
+        code     = node.text
+
+        highlighted = block_code(code, language)
+
+        # Replace the parent `pre` element with the entire highlighted block
+        node.parent.replace(highlighted)
+      end
+
+      private
+
+      # Override Rouge::Plugins::Redcarpet#rouge_formatter
+      def rouge_formatter(lexer)
+        Rouge::Formatters::HTMLGitlab.new(
+          cssclass: "code highlight js-syntax-highlight #{lexer.tag}")
+      end
+    end
+  end
+end

lib/gitlab/markdown/table_of_contents_filter.rb

@@ -1,3 +1,4 @@
+require 'gitlab/markdown'
 require 'html/pipeline/filter'
 
 module Gitlab

lib/gitlab/markdown/task_list_filter.rb

@@ -1,3 +1,4 @@
+require 'gitlab/markdown'
 require 'task_list/filter'
 
 module Gitlab

lib/gitlab/markdown/user_reference_filter.rb

@@ -1,3 +1,5 @@
+require 'gitlab/markdown'
+
 module Gitlab
   module Markdown
     # HTML filter that replaces user or group references with links.

lib/gitlab/reference_extractor.rb

@@ -1,3 +1,5 @@
+require 'gitlab/markdown'
+
 module Gitlab
   # Extract possible GFM references from an arbitrary String for further processing.
   class ReferenceExtractor
@@ -10,7 +12,7 @@ module Gitlab
 
     def analyze(text)
       references.clear
-      @text = markdown.render(text.dup)
+      @text = Gitlab::Markdown.render_without_gfm(text)
     end
 
     %i(user label issue merge_request snippet commit commit_range).each do |type|
@@ -21,10 +23,6 @@ module Gitlab
 
     private
 
-    def markdown
-      @markdown ||= Redcarpet::Markdown.new(Redcarpet::Render::HTML, GitlabMarkdownHelper::MARKDOWN_OPTIONS)
-    end
-
     def references
       @references ||= Hash.new do |references, type|
         type = type.to_sym
@@ -42,7 +40,7 @@ module Gitlab
     # Returns the results Array for the requested filter type
     def pipeline_result(filter_type)
       klass  = filter_type.to_s.camelize + 'ReferenceFilter'
-      filter = "Gitlab::Markdown::#{klass}".constantize
+      filter = Gitlab::Markdown.const_get(klass)
 
       context = {
         project: project,

lib/redcarpet/render/gitlab_html.rb

@@ -1,45 +0,0 @@
-require 'active_support/core_ext/string/output_safety'
-
-class Redcarpet::Render::GitlabHTML < Redcarpet::Render::HTML
-  attr_reader :template
-  alias_method :h, :template
-
-  def initialize(template, options = {})
-    @template = template
-    @options = options.dup
-
-    @options.reverse_merge!(
-      # Handled further down the line by Gitlab::Markdown::SanitizationFilter
-      escape_html: false,
-      project: @template.instance_variable_get("@project")
-    )
-
-    super(options)
-  end
-
-  def normal_text(text)
-    ERB::Util.html_escape_once(text)
-  end
-
-  # Stolen from Rouge::Plugins::Redcarpet as this module is not required
-  # from Rouge's gem root.
-  def block_code(code, language)
-    lexer = Rouge::Lexer.find_fancy(language, code) || Rouge::Lexers::PlainText
-
-    # XXX HACK: Redcarpet strips hard tabs out of code blocks,
-    # so we assume you're not using leading spaces that aren't tabs,
-    # and just replace them here.
-    if lexer.tag == 'make'
-      code.gsub!(/^    /, "\t")
-    end
-
-    formatter = Rouge::Formatters::HTMLGitlab.new(
-      cssclass: "code highlight js-syntax-highlight #{lexer.tag}"
-    )
-    formatter.format(lexer.lex(code))
-  end
-
-  def postprocess(full_document)
-    h.gfm(full_document, @options)
-  end
-end

spec/features/gitlab_flavored_markdown_spec.rb

@@ -77,7 +77,7 @@ describe "GitLab Flavored Markdown", feature: true do
     it "should render details in issues#show" do
       visit namespace_project_issue_path(project.namespace, project, @issue)
 
-      expect(page).to have_link("@#{fred.username}")
+      expect(page).to have_link(fred.to_reference)
     end
   end
 

spec/features/markdown_spec.rb

@@ -179,7 +179,7 @@ describe 'GitLab Markdown', feature: true do
     before(:all) do
       @feat = MarkdownFeature.new
 
-      # `gfm` helper depends on a `@project` variable
+      # `markdown` helper expects a `@project` variable
       @project = @feat.project
 
       @html = markdown(@feat.raw_markdown)

spec/helpers/gitlab_markdown_helper_spec.rb

@@ -19,28 +19,23 @@ describe GitlabMarkdownHelper do
     @project = project
   end
 
-  describe "#gfm" do
-    it "should forward HTML options to links" do
-      expect(gfm("Fixed in #{commit.id}", { project: @project }, class: 'foo')).
-        to have_selector('a.gfm.foo')
-    end
-
+  describe "#markdown" do
     describe "referencing multiple objects" do
       let(:actual) { "#{merge_request.to_reference} -> #{commit.to_reference} -> #{issue.to_reference}" }
 
       it "should link to the merge request" do
         expected = namespace_project_merge_request_path(project.namespace, project, merge_request)
-        expect(gfm(actual)).to match(expected)
+        expect(markdown(actual)).to match(expected)
       end
 
       it "should link to the commit" do
         expected = namespace_project_commit_path(project.namespace, project, commit)
-        expect(gfm(actual)).to match(expected)
+        expect(markdown(actual)).to match(expected)
       end
 
       it "should link to the issue" do
         expected = namespace_project_issue_path(project.namespace, project, issue)
-        expect(gfm(actual)).to match(expected)
+        expect(markdown(actual)).to match(expected)
       end
     end
   end

spec/lib/gitlab/markdown/commit_range_reference_filter_spec.rb

@@ -75,11 +75,6 @@ module Gitlab::Markdown
         expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-commit_range'
       end
 
-      it 'includes an optional custom class' do
-        doc = filter("See #{reference}", reference_class: 'custom')
-        expect(doc.css('a').first.attr('class')).to include 'custom'
-      end
-
       it 'includes a data-project-id attribute' do
         doc = filter("See #{reference}")
         link = doc.css('a').first

spec/lib/gitlab/markdown/commit_reference_filter_spec.rb

@@ -71,11 +71,6 @@ module Gitlab::Markdown
         expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-commit'
       end
 
-      it 'includes an optional custom class' do
-        doc = filter("See #{reference}", reference_class: 'custom')
-        expect(doc.css('a').first.attr('class')).to include 'custom'
-      end
-
       it 'includes a data-project-id attribute' do
         doc = filter("See #{reference}")
         link = doc.css('a').first

spec/lib/gitlab/markdown/external_issue_reference_filter_spec.rb

@@ -68,11 +68,6 @@ module Gitlab::Markdown
         expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-issue'
       end
 
-      it 'includes an optional custom class' do
-        doc = filter("Issue #{reference}", reference_class: 'custom')
-        expect(doc.css('a').first.attr('class')).to include 'custom'
-      end
-
       it 'supports an :only_path context' do
         doc = filter("Issue #{reference}", only_path: true)
         link = doc.css('a').first.attr('href')

spec/lib/gitlab/markdown/issue_reference_filter_spec.rb

@@ -68,11 +68,6 @@ module Gitlab::Markdown
         expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-issue'
       end
 
-      it 'includes an optional custom class' do
-        doc = filter("Issue #{reference}", reference_class: 'custom')
-        expect(doc.css('a').first.attr('class')).to include 'custom'
-      end
-
       it 'includes a data-project-id attribute' do
         doc = filter("Issue #{reference}")
         link = doc.css('a').first

spec/lib/gitlab/markdown/label_reference_filter_spec.rb

@@ -25,11 +25,6 @@ module Gitlab::Markdown
       expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-label'
     end
 
-    it 'includes an optional custom class' do
-      doc = filter("Label #{reference}", reference_class: 'custom')
-      expect(doc.css('a').first.attr('class')).to include 'custom'
-    end
-
     it 'includes a data-project-id attribute' do
       doc = filter("Label #{reference}")
       link = doc.css('a').first

spec/lib/gitlab/markdown/merge_request_reference_filter_spec.rb

@@ -56,11 +56,6 @@ module Gitlab::Markdown
         expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-merge_request'
       end
 
-      it 'includes an optional custom class' do
-        doc = filter("Merge #{reference}", reference_class: 'custom')
-        expect(doc.css('a').first.attr('class')).to include 'custom'
-      end
-
       it 'includes a data-project-id attribute' do
         doc = filter("Merge #{reference}")
         link = doc.css('a').first

spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb

@@ -55,11 +55,6 @@ module Gitlab::Markdown
         expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-snippet'
       end
 
-      it 'includes an optional custom class' do
-        doc = filter("Snippet #{reference}", reference_class: 'custom')
-        expect(doc.css('a').first.attr('class')).to include 'custom'
-      end
-
       it 'includes a data-project-id attribute' do
         doc = filter("Snippet #{reference}")
         link = doc.css('a').first

spec/lib/gitlab/markdown/user_reference_filter_spec.rb

@@ -130,11 +130,6 @@ module Gitlab::Markdown
       expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-project_member'
     end
 
-    it 'includes an optional custom class' do
-      doc = filter("Hey #{reference}", reference_class: 'custom')
-      expect(doc.css('a').first.attr('class')).to include 'custom'
-    end
-
     it 'supports an :only_path context' do
       doc = filter("Hey #{reference}", only_path: true)
       link = doc.css('a').first.attr('href')