From 4340dd3eeb6fdda83b729c16cba29239b8ed9f43 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 27 Aug 2015 13:09:01 -0700 Subject: [PATCH] Decouple Gitlab::Markdown from the GitlabMarkdownHelper This module is now the sole source of knowledge for *how* we render Markdown (and GFM). --- app/helpers/gitlab_markdown_helper.rb | 34 +++------- lib/gitlab/markdown.rb | 62 ++++++++++++++++--- .../markdown/syntax_highlight_filter.rb | 38 ++++++++++++ lib/gitlab/reference_extractor.rb | 8 +-- lib/redcarpet/render/gitlab_html.rb | 45 -------------- spec/features/markdown_spec.rb | 2 +- spec/helpers/gitlab_markdown_helper_spec.rb | 22 ++----- 7 files changed, 111 insertions(+), 100 deletions(-) create mode 100644 lib/gitlab/markdown/syntax_highlight_filter.rb delete mode 100644 lib/redcarpet/render/gitlab_html.rb diff --git a/app/helpers/gitlab_markdown_helper.rb b/app/helpers/gitlab_markdown_helper.rb index 114730eb948..b7fec9fc1d9 100644 --- a/app/helpers/gitlab_markdown_helper.rb +++ b/app/helpers/gitlab_markdown_helper.rb @@ -1,7 +1,6 @@ require 'nokogiri' module GitlabMarkdownHelper - include Gitlab::Markdown include PreferencesHelper # Use this in places where you would normally use link_to(gfm(...), ...). @@ -22,7 +21,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' @@ -42,29 +41,16 @@ module GitlabMarkdownHelper 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, options={}) - unless @markdown && options == @options - @options = options - - # see https://github.com/vmg/redcarpet#darling-i-packed-you-a-couple-renderers-for-lunch - rend = Redcarpet::Render::GitlabHTML.new(self, options) - - # see https://github.com/vmg/redcarpet#and-its-like-really-simple-to-use - @markdown = Redcarpet::Markdown.new(rend, MARKDOWN_OPTIONS) - end + def markdown(text, context = {}) + context.merge!( + current_user: current_user, + project: @project, + project_wiki: @project_wiki, + ref: @ref, + requested_path: @path + ) - @markdown.render(text).html_safe + Gitlab::Markdown.render(text, context) end def asciidoc(text) diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb index 9f6e19a09fd..de1da31a390 100644 --- a/lib/gitlab/markdown.rb +++ b/lib/gitlab/markdown.rb @@ -5,6 +5,44 @@ module Gitlab # # See the files in `lib/gitlab/markdown/` for specific processing information. module Markdown + # https://github.com/vmg/redcarpet#and-its-like-really-simple-to-use + REDCARPET_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 + + # 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) + self.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 +56,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' @@ -29,7 +68,7 @@ module Gitlab # :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 +79,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 +100,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 +121,22 @@ module Gitlab private + def self.renderer + @markdown ||= begin + renderer = Redcarpet::Render::HTML.new + Redcarpet::Markdown.new(renderer, REDCARPET_OPTIONS) + end + 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, diff --git a/lib/gitlab/markdown/syntax_highlight_filter.rb b/lib/gitlab/markdown/syntax_highlight_filter.rb new file mode 100644 index 00000000000..9f468f98aeb --- /dev/null +++ b/lib/gitlab/markdown/syntax_highlight_filter.rb @@ -0,0 +1,38 @@ +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 diff --git a/lib/gitlab/reference_extractor.rb b/lib/gitlab/reference_extractor.rb index e836b05ff25..20f4098057c 100644 --- a/lib/gitlab/reference_extractor.rb +++ b/lib/gitlab/reference_extractor.rb @@ -10,7 +10,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 +21,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 +38,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, diff --git a/lib/redcarpet/render/gitlab_html.rb b/lib/redcarpet/render/gitlab_html.rb deleted file mode 100644 index 9cb8e91d6e3..00000000000 --- a/lib/redcarpet/render/gitlab_html.rb +++ /dev/null @@ -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 diff --git a/spec/features/markdown_spec.rb b/spec/features/markdown_spec.rb index 4fe019f8342..c557a1061af 100644 --- a/spec/features/markdown_spec.rb +++ b/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) diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index a42ccb9b501..d1ca2337a9b 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -19,10 +19,10 @@ describe GitlabMarkdownHelper do @project = project end - describe "#gfm" do + describe "#markdown" 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') + expect(markdown("Fixed in #{commit.id}", project: @project)). + to have_selector('a.gfm') end describe "referencing multiple objects" do @@ -30,17 +30,17 @@ describe GitlabMarkdownHelper do 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 @@ -79,16 +79,6 @@ describe GitlabMarkdownHelper do expect(doc.css('a')[4].text).to eq ' for real' end - it 'should forward HTML options' do - actual = link_to_gfm("Fixed in #{commit.id}", commit_path, class: 'foo') - doc = Nokogiri::HTML.parse(actual) - - expect(doc.css('a')).to satisfy do |v| - # 'foo' gets added to all links - v.all? { |a| a.attr('class').match(/foo$/) } - end - end - it "escapes HTML passed in as the body" do actual = "This is a

test

- see #{issues[0].to_reference}" expect(link_to_gfm(actual, commit_path)). -- GitLab