Merge pull request #1085 from Retrospring/fix/markdown-rendering

Markdown rendering fixes

app/helpers/markdown_helper.rb

@@ -1,36 +1,42 @@
-module MarkdownHelper
+# frozen_string_literal: true
 
+module MarkdownHelper
   def markdown(content)
-    md = Redcarpet::Markdown.new(FlavoredMarkdown, MARKDOWN_OPTS)
+    renderer = FlavoredMarkdown.new(**MARKDOWN_RENDERER_OPTS)
-    Sanitize.fragment(md.render(content), EVIL_TAGS).html_safe
+    md = Redcarpet::Markdown.new(renderer, **MARKDOWN_OPTS)
+    # As the string has been sanitized we can mark it as HTML safe
+    Sanitize.fragment(md.render(content), EVIL_TAGS).strip.html_safe # rubocop:disable Rails/OutputSafety
   end
 
   def strip_markdown(content)
-    md = Redcarpet::Markdown.new(Redcarpet::Render::StripDown, MARKDOWN_OPTS)
+    renderer = Redcarpet::Render::StripDown.new
+    md = Redcarpet::Markdown.new(renderer, **MARKDOWN_OPTS)
     CGI.unescape_html(Sanitize.fragment(CGI.escape_html(md.render(content)), EVIL_TAGS)).strip
   end
 
   def twitter_markdown(content)
-    md = Redcarpet::Markdown.new(TwitteredMarkdown, MARKDOWN_OPTS)
+    renderer = TwitteredMarkdown.new
+    md = Redcarpet::Markdown.new(renderer, **MARKDOWN_OPTS)
     CGI.unescape_html(Sanitize.fragment(CGI.escape_html(md.render(content)), EVIL_TAGS)).strip
   end
 
   def question_markdown(content)
-    md = Redcarpet::Markdown.new(QuestionMarkdown.new, MARKDOWN_OPTS)
+    renderer = QuestionMarkdown.new
-    Sanitize.fragment(md.render(content), EVIL_TAGS).html_safe
+    md = Redcarpet::Markdown.new(renderer, **MARKDOWN_OPTS)
+    # As the string has been sanitized we can mark it as HTML safe
+    Sanitize.fragment(md.render(content), EVIL_TAGS).strip.html_safe # rubocop:disable Rails/OutputSafety
   end
 
   def raw_markdown(content)
-    md = Redcarpet::Markdown.new(Redcarpet::Render::HTML, RAW_MARKDOWN_OPTS)
+    renderer = Redcarpet::Render::HTML.new(**MARKDOWN_RENDERER_OPTS)
+    md = Redcarpet::Markdown.new(renderer, **MARKDOWN_OPTS)
     raw md.render content
   end
 
   def get_markdown(path, relative_to = Rails.root)
-    begin
+    File.read relative_to.join(path)
-      File.read relative_to.join(path)
+  rescue Errno::ENOENT
-    rescue Errno::ENOENT
+    "# Error reading #{relative_to.join(path)}"
-      "# Error reading #{relative_to.join(path)}"
-    end
   end
 
   def markdown_io(path, relative_to = Rails.root)

app/services/flavored_markdown.rb

@@ -18,10 +18,6 @@ class FlavoredMarkdown < Redcarpet::Render::HTML
     paragraph text
   end
 
-  def paragraph(text)
-    "<p>#{text}</p>"
-  end
-
   def raw_html(raw_html)
     Rack::Utils.escape_html raw_html
   end

app/services/question_markdown.rb

@@ -6,9 +6,7 @@ class QuestionMarkdown < Redcarpet::Render::StripDown
   include Rails.application.routes.url_helpers
   include SharedMarkers
 
-  def paragraph(text)
+  def paragraph(text) = "<p>#{text.gsub("\n", '<br>')}</p>"
-    "<p>#{text}</p>"
-  end
 
   def link(link, _title, _content)
     process_link(link)

config/initializers/redcarpet.rb

@@ -1,32 +1,28 @@
-require 'redcarpet/render_strip'
+# frozen_string_literal: true
+
+require "redcarpet/render_strip"
 
 MARKDOWN_OPTS = {
-    filter_html: true,
+  no_intra_emphasis:            true,
-    escape_html: true,
+  tables:                       true,
-    no_images: true,
+  fenced_code_blocks:           true,
-    no_styles: true,
+  autolink:                     true,
-    safe_links_only: true,
-    xhtml: false,
-    hard_wrap: true,
-    no_intra_emphasis: true,
-    tables: true,
-    fenced_code_blocks: true,
-    autolink: true,
-    disable_indented_code_blocks: true,
-    strikethrough: true,
-    superscript: false
-}
-
-RAW_MARKDOWN_OPTS = {
-  tables: true,
-  fenced_code_blocks: true,
-  autolink: true,
   disable_indented_code_blocks: true,
-  strikethrough: true,
+  strikethrough:                true,
-  superscript: false
+  superscript:                  false,
-}
+}.freeze
+
+MARKDOWN_RENDERER_OPTS = {
+  filter_html:     true,
+  escape_html:     true,
+  no_images:       true,
+  no_styles:       true,
+  safe_links_only: true,
+  xhtml:           false,
+  hard_wrap:       true,
+}.freeze
 
 ALLOWED_HOSTS_IN_MARKDOWN = [
-  APP_CONFIG['hostname'],
+  APP_CONFIG["hostname"],
-  *APP_CONFIG['allowed_hosts_in_markdown']
+  *APP_CONFIG["allowed_hosts_in_markdown"]
-]
+].freeze

config/locales/views.en.yml

@@ -567,7 +567,6 @@ en:
     formatting:
       body_html: |
         <p>%{app_name} uses <b>Markdown</b> for formatting</p>
-        <p>A blank line starts a new paragraph</p>
         <p><code>*italic text*</code> for <i>italic text</i></p>
         <p><code>**bold text**</code> for <b>bold text</b></p>
         <p><code>[link](https://example.com)</code> for <a href="https://example.com">link</a></p>

spec/helpers/markdown_helper_spec.rb

@@ -31,6 +31,10 @@ describe MarkdownHelper, type: :helper do
     it "should escape HTML tags" do
       expect(markdown("I'm <h1>a test</h1>")).to eq("<p>I'm &lt;h1&gt;a test&lt;/h1&gt;</p>")
     end
+
+    it "should turn line breaks into <br> tags" do
+      expect(markdown("Some\ntext")).to eq("<p>Some<br>\ntext</p>")
+    end
   end
 
   describe "#strip_markdown" do
@@ -70,6 +74,10 @@ describe MarkdownHelper, type: :helper do
     it "should not process invalid links" do
       expect(question_markdown("https://example.com/example.質問")).to eq("<p>https://example.com/example.質問</p>")
     end
+
+    it "should turn line breaks into <br> tags" do
+      expect(markdown("Some\ntext")).to eq("<p>Some<br>\ntext</p>")
+    end
   end
 
   describe "#raw_markdown" do