From 5cb5539e06e44563eaaaf5c662236089edd06168 Mon Sep 17 00:00:00 2001 From: Anna Bargiel Date: Thu, 16 Nov 2023 09:26:55 +0100 Subject: [PATCH 1/4] Refactor usage of Redcarpet We were missing tests to check the conversion from Markdown to HTML. While writing the missing tests, I found out that some implementations for using Redcarpet didn't work. We had added some Redcarpet extensions, [1] but they did not load in the application when we used them. To be more specific, the Redcarpet extensions were not enabled in the application where we were using them. This was the reason for refactoring the code. The tests that check for correct conversion of Markdown to HTML with Redcarpet pass. [1]: https://www.writesoftwarewell.com/how-to-render-markdown-views-in-rails/ --- config/initializers/markdown.rb | 38 +----- lib/handlers/markdown_handler.rb | 12 ++ lib/markdown.rb | 28 +++++ spec/lib/markdown_spec.rb | 191 +++++++++++++++++++++++++++++++ 4 files changed, 233 insertions(+), 36 deletions(-) create mode 100644 lib/handlers/markdown_handler.rb create mode 100644 lib/markdown.rb create mode 100644 spec/lib/markdown_spec.rb diff --git a/config/initializers/markdown.rb b/config/initializers/markdown.rb index e75d53c6..eb72c29c 100644 --- a/config/initializers/markdown.rb +++ b/config/initializers/markdown.rb @@ -1,40 +1,6 @@ -require 'redcarpet' - -module ActionView - module Template::Handlers - # Rails template handler for Markdown - class Markdown - class_attribute :default_format - self.default_format = Mime[:html] - - # @param template [ActionView::Template] - # @return [String] Ruby code that when evaluated will return the rendered - # content - def call(_template, body) - @markdown ||= Redcarpet::Markdown.new(renderer, params) - "#{@markdown.render(body).inspect}.html_safe" - end - - private - - def params - { - autolink: true, fenced_code_blocks: true, - space_after_headers: true, tables: true - } - end - - def renderer - options = { - link_attributes: { rel: 'nofollow' } - } - Redcarpet::Render::HTML.new(options) - end - end - end -end +require 'handlers/markdown_handler' ActionView::Template.register_template_handler( :md, - ActionView::Template::Handlers::Markdown.new + Handlers::MarkdownHandler.new ) diff --git a/lib/handlers/markdown_handler.rb b/lib/handlers/markdown_handler.rb new file mode 100644 index 00000000..fba4bbae --- /dev/null +++ b/lib/handlers/markdown_handler.rb @@ -0,0 +1,12 @@ +require 'markdown' + +module Handlers + class MarkdownHandler + class_attribute :default_format + self.default_format = Mime[:html] + + def call(_template, body) + Markdown.new(body).to_html.inspect + end + end +end diff --git a/lib/markdown.rb b/lib/markdown.rb new file mode 100644 index 00000000..45ed0064 --- /dev/null +++ b/lib/markdown.rb @@ -0,0 +1,28 @@ +require 'redcarpet' + +class Markdown + EXTENSIONS = { + autolink: true, + fenced_code_blocks: true, + space_after_headers: true, + strikethrough: true, + tables: true + }.freeze + + def initialize(input) + @input = input + end + + def to_html + @markdown = Redcarpet::Markdown.new(renderer, EXTENSIONS) + @markdown.render(input) + end + + private + + attr_reader :input + + def renderer + Redcarpet::Render::HTML.new(link_attributes: { rel: 'nofollow' }) + end +end diff --git a/spec/lib/markdown_spec.rb b/spec/lib/markdown_spec.rb new file mode 100644 index 00000000..d64d12ba --- /dev/null +++ b/spec/lib/markdown_spec.rb @@ -0,0 +1,191 @@ +require 'spec_helper' +require 'markdown' + +RSpec.describe Markdown do + describe '#to_html' do + it 'returns the HTML code that corresponds to the important text in Markdown' do + important_text_in_markdown = '**Hello** world' + expected_result_in_html = "

Hello world

\n" + markdown = described_class.new(important_text_in_markdown) + + expect(markdown.to_html).to eq(expected_result_in_html) + end + + it 'returns the HTML code that corresponds to the emphasized text in Markdown' do + emphasized_text_in_markdown = 'Hello *world*' + expected_result_in_html = "

Hello world

\n" + markdown = described_class.new(emphasized_text_in_markdown) + + expect(markdown.to_html).to eq(expected_result_in_html) + end + + it 'returns the HTML code that corresponds to the striked text in Markdown' do + strikethrough_in_markdown = 'Hello ~~World~~' + expected_result_in_html = "

Hello World

\n" + markdown = described_class.new(strikethrough_in_markdown) + + expect(markdown.to_html).to eq(expected_result_in_html) + end + + it 'returns the HTML code the corresponds to the text containing quotation marks in Markdown' do + input = "My name is 'Bond, James Bond'" + expected_result_in_html = "

My name is 'Bond, James Bond'

\n" + markdown = described_class.new(input) + + expect(markdown.to_html).to eq(expected_result_in_html) + end + + it 'returns the HTML code that corresponds to the inline code in Markdown' do + inline_code_in_markdown = 'This is `inline code`' + expected_result_in_html = "

This is inline code

\n" + markdown = described_class.new(inline_code_in_markdown) + + expect(markdown.to_html).to eq(expected_result_in_html) + end + + it 'returns the HTML code that corresponds to the block code in Markdown' do + block_code_in_markdown = <<~BLOCK_CODE + ```ruby + puts 'Hello world' + ``` + BLOCK_CODE + expected_result_in_html = <<~HTML + 
puts 'Hello world'
+
+ HTML + markdown = described_class.new(block_code_in_markdown) + + expect(markdown.to_html).to eq(expected_result_in_html) + end + + it 'returns the HTML code that corresponds to the header in Markdown' do + header_in_markdown = '# The most important heading' + markdown = described_class.new(header_in_markdown) + + expect(markdown.to_html).to eq("

The most important heading

\n") + end + + it 'returns the HTML code that corresponds to the link in Markdown' do + link_in_markdown = 'This is the [FractalSoft](https://fractalsoft.org/)' + expected_result_in_html = <<~HTML +

This is the FractalSoft

+ HTML + markdown = described_class.new(link_in_markdown) + + expect(markdown.to_html).to eq(expected_result_in_html) + end + + it 'returns the HTML code that corresponds to the link in Markdown with www recognition' do + markdown_with_www_link = 'Visit www.fractalsoft.org' + expected_result_in_html = <<~HTML +

Visit www.fractalsoft.org

+ HTML + markdown = described_class.new(markdown_with_www_link) + + expect(markdown.to_html).to eq(expected_result_in_html) + end + + it 'returns the HTML code that corresponds to the image as link in Markdown' do + image_as_link_in_markdown = '[![My image](/path/to/image)](https://fractalsoft.org/)' + expected_result_in_html = <<~HTML +

My image

+ HTML + markdown = described_class.new(image_as_link_in_markdown) + + expect(markdown.to_html).to eq(expected_result_in_html) + end + + it 'returns the HTML code that corresponds to the link in header in Markdown' do + header_link_in_markdown = '# My Header [link](https://fractalsoft.org/)' + expected_result_in_html = <<~HTML +

My Header link

+ HTML + markdown = described_class.new(header_link_in_markdown) + + expect(markdown.to_html).to eq(expected_result_in_html) + end + + it 'returns the HTML code that corresponds to the quote in Markdown' do + quote_in_markdown = '> Be or not to be' + expected_result_in_html = "
\n

Be or not to be

\n
\n" + markdown = described_class.new(quote_in_markdown) + + expect(markdown.to_html).to eq(expected_result_in_html) + end + + it 'returns the HTML code that corresponds to the ordered list in Markdown' do + ordered_list_in_markdown = <<~ORDERED_LIST + 1. First item + 2. Second item + 3. Third item + ORDERED_LIST + expected_result_in_html = <<~HTML +
    +
  1. First item
    2. +
  2. Second item
    3. +
  3. Third item
    4. +
+ HTML + markdown = described_class.new(ordered_list_in_markdown) + + expect(markdown.to_html).to eq(expected_result_in_html) + end + + it 'returns the HTML code that corresponds to the unordered list in Markdown' do + unordered_list_in_markdown = <<~UNORDERED_LIST + - First item + - Second item + - Third item + UNORDERED_LIST + expected_result_in_html = <<~HTML + + HTML + markdown = described_class.new(unordered_list_in_markdown) + + expect(markdown.to_html).to eq(expected_result_in_html) + end + + it 'returns the HTML code that corresponds to the image in Markdown' do + image_in_markdown = '![My image](/path/to/image)' + expected_result_in_html = <<~HTML +

My image

+ HTML + markdown = described_class.new(image_in_markdown) + + expect(markdown.to_html).to eq(expected_result_in_html) + end + + it 'returns the HTML code that corresponds to the table in Markdown' do + table_in_markdown = <<~TABLE + | Syntax | Description | + |-------------|-------------| + | Header | Title | + | Paragraph | Text | + TABLE + expected_result_in_html = <<~HTML + + + + + + + + + + + + + + +
SyntaxDescription
HeaderTitle
ParagraphText
+ HTML + markdown = described_class.new(table_in_markdown) + + expect(markdown.to_html).to eq(expected_result_in_html) + end + end +end From dadccac949338dfb493f516d99d3ca5178b48601 Mon Sep 17 00:00:00 2001 From: Anna Bargiel Date: Thu, 16 Nov 2023 09:53:02 +0100 Subject: [PATCH 2/4] Add commonmarker gem > Ruby wrapper for the comrak (CommonMark parser) Rust crate [1] [1]: https://github.com/gjtorikian/commonmarker --- Gemfile | 1 + Gemfile.lock | 9 +++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Gemfile b/Gemfile index 5923ad25..17c7fdb0 100644 --- a/Gemfile +++ b/Gemfile @@ -11,6 +11,7 @@ gem 'actionpack-page_caching' gem 'active_link_to' gem 'bootstrap', '~> 5.3.2' gem 'carrierwave', '~> 3.0' +gem 'commonmarker', '~>1.0.0.pre11' gem 'draper' gem 'friendly_id', '~> 5.5' gem 'globalize' diff --git a/Gemfile.lock b/Gemfile.lock index 3f93646c..7262c0ac 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -160,6 +160,7 @@ GEM descendants_tracker (~> 0.0.1) colored (1.2) colorize (0.8.1) + commonmarker (1.0.0.pre11-x86_64-linux) concurrent-ruby (1.2.2) connection_pool (2.4.1) crass (1.0.6) @@ -281,7 +282,7 @@ GEM language_server-protocol (3.17.0.3) launchy (2.5.2) addressable (~> 2.8) - libv8-node (18.16.0.0) + libv8-node (18.16.0.0-x86_64-linux) listen (3.8.0) rb-fsevent (~> 0.10, >= 0.10.3) rb-inotify (~> 0.9, >= 0.9.10) @@ -310,7 +311,6 @@ GEM mini_histogram (0.3.1) mini_magick (4.12.0) mini_mime (1.1.5) - mini_portile2 (2.8.5) mini_racer (0.8.0) libv8-node (~> 18.16.0.0) minitest (5.20.0) @@ -329,8 +329,7 @@ GEM net-protocol net-ssh (7.2.0) nio4r (2.5.9) - nokogiri (1.15.4) - mini_portile2 (~> 2.8.2) + nokogiri (1.15.4-x86_64-linux) racc (~> 1.4) normalize-rails (8.0.1) oj (3.16.1) @@ -579,6 +578,7 @@ GEM PLATFORMS ruby + x86_64-linux DEPENDENCIES actionpack-action_caching @@ -596,6 +596,7 @@ DEPENDENCIES capybara carrierwave (~> 3.0) colored + commonmarker (~> 1.0.0.pre11) cuprite database_cleaner (~> 2.0.1) derailed_benchmarks From 03477cfdcb5017ca9a5ca7087b1f34afb824834d Mon Sep 17 00:00:00 2001 From: Anna Bargiel Date: Thu, 16 Nov 2023 11:34:20 +0100 Subject: [PATCH 3/4] Replace Redcarpet with CommonMarker We replace Redcarpet with CommonMarker [1] because the latter has more features and is faster. [2] This helps to render HTML from Markdown better. I had to refactor the tests due this change. [1]: https://github.com/gjtorikian/commonmarker/blob/v1.0.0.pre11/README.md [2]: https://github.github.com/gfm/ --- lib/markdown.rb | 19 ++++++-------- spec/lib/markdown_spec.rb | 55 +++++++++++++++++++++++++++------------ 2 files changed, 47 insertions(+), 27 deletions(-) diff --git a/lib/markdown.rb b/lib/markdown.rb index 45ed0064..ac74922a 100644 --- a/lib/markdown.rb +++ b/lib/markdown.rb @@ -1,12 +1,8 @@ -require 'redcarpet' +require 'commonmarker' class Markdown - EXTENSIONS = { - autolink: true, - fenced_code_blocks: true, - space_after_headers: true, - strikethrough: true, - tables: true + RENDER = { + hardbreaks: false }.freeze def initialize(input) @@ -14,15 +10,16 @@ def initialize(input) end def to_html - @markdown = Redcarpet::Markdown.new(renderer, EXTENSIONS) - @markdown.render(input) + Commonmarker.to_html(input, options:) end private attr_reader :input - def renderer - Redcarpet::Render::HTML.new(link_attributes: { rel: 'nofollow' }) + def options + { + render: RENDER + } end end diff --git a/spec/lib/markdown_spec.rb b/spec/lib/markdown_spec.rb index d64d12ba..7e771ea8 100644 --- a/spec/lib/markdown_spec.rb +++ b/spec/lib/markdown_spec.rb @@ -29,7 +29,7 @@ it 'returns the HTML code the corresponds to the text containing quotation marks in Markdown' do input = "My name is 'Bond, James Bond'" - expected_result_in_html = "

My name is 'Bond, James Bond'

\n" + expected_result_in_html = "

My name is 'Bond, James Bond'

\n" markdown = described_class.new(input) expect(markdown.to_html).to eq(expected_result_in_html) @@ -49,10 +49,12 @@ puts 'Hello world' ``` BLOCK_CODE - expected_result_in_html = <<~HTML - 
puts 'Hello world'
-
- HTML + expected_result_in_html = '
' \
+                                '' \
+                                'puts ' \
+                                ''' \
+                                'Hello world' \
+                                "'\n
\n" markdown = described_class.new(block_code_in_markdown) expect(markdown.to_html).to eq(expected_result_in_html) @@ -60,15 +62,18 @@ it 'returns the HTML code that corresponds to the header in Markdown' do header_in_markdown = '# The most important heading' + expected_result_in_html = '

' \ + "The most important heading

\n" markdown = described_class.new(header_in_markdown) - expect(markdown.to_html).to eq("

The most important heading

\n") + expect(markdown.to_html).to eq(expected_result_in_html) end it 'returns the HTML code that corresponds to the link in Markdown' do link_in_markdown = 'This is the [FractalSoft](https://fractalsoft.org/)' expected_result_in_html = <<~HTML -

This is the FractalSoft

+

This is the FractalSoft

HTML markdown = described_class.new(link_in_markdown) @@ -78,7 +83,7 @@ it 'returns the HTML code that corresponds to the link in Markdown with www recognition' do markdown_with_www_link = 'Visit www.fractalsoft.org' expected_result_in_html = <<~HTML -

Visit www.fractalsoft.org

+

Visit www.fractalsoft.org

HTML markdown = described_class.new(markdown_with_www_link) @@ -88,7 +93,7 @@ it 'returns the HTML code that corresponds to the image as link in Markdown' do image_as_link_in_markdown = '[![My image](/path/to/image)](https://fractalsoft.org/)' expected_result_in_html = <<~HTML -

My image

+

My image

HTML markdown = described_class.new(image_as_link_in_markdown) @@ -97,9 +102,8 @@ it 'returns the HTML code that corresponds to the link in header in Markdown' do header_link_in_markdown = '# My Header [link](https://fractalsoft.org/)' - expected_result_in_html = <<~HTML -

My Header link

- HTML + expected_result_in_html = '

' \ + "My Header link

\n" markdown = described_class.new(header_link_in_markdown) expect(markdown.to_html).to eq(expected_result_in_html) @@ -152,7 +156,7 @@ it 'returns the HTML code that corresponds to the image in Markdown' do image_in_markdown = '![My image](/path/to/image)' expected_result_in_html = <<~HTML -

My image

+

My image

HTML markdown = described_class.new(image_in_markdown) @@ -167,12 +171,14 @@ | Paragraph | Text | TABLE expected_result_in_html = <<~HTML - +
+ - + + @@ -181,11 +187,28 @@ -
Syntax Description
Header TitleParagraph Text
+ + HTML markdown = described_class.new(table_in_markdown) expect(markdown.to_html).to eq(expected_result_in_html) end + + it 'returns the HTML code that corresponds to the task list in Markdown' do + task_list_in_markdown = <<~TASK_LIST + - [ ] first to do + - [x] second to do + TASK_LIST + expected_result_in_html = <<~HTML +
    +
  • first to do
    • +
  • second to do
    • +
+ HTML + markdown = described_class.new(task_list_in_markdown) + + expect(markdown.to_html).to eq(expected_result_in_html) + end end end From aa89b81eac7f89713961a0cccb11c24d326a5c3f Mon Sep 17 00:00:00 2001 From: Anna Bargiel Date: Thu, 16 Nov 2023 11:42:27 +0100 Subject: [PATCH 4/4] Remove Redcarpet Since Redcarpet [1] is not used anywhere, I have removed it. [1]: https://github.com/vmg/redcarpet --- Gemfile | 1 - Gemfile.lock | 1 - 2 files changed, 2 deletions(-) diff --git a/Gemfile b/Gemfile index 17c7fdb0..60500f6d 100644 --- a/Gemfile +++ b/Gemfile @@ -28,7 +28,6 @@ gem 'oj' # Fast JSON parser and object serializer gem 'pg' # Ruby interface to PostgreSQL RDBMS gem 'puma' # Ruby web server built for concurrency gem 'rails-i18n' -gem 'redcarpet' gem 'route_translator' # Manage translations of routes gem 'sass-rails', '~> 6.0' gem 'simple_form', '~> 5.3' diff --git a/Gemfile.lock b/Gemfile.lock index 7262c0ac..221023e1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -628,7 +628,6 @@ DEPENDENCIES rails-controller-testing rails-i18n rails_best_practices - redcarpet reek route_translator rspec-rails (~> 6.0.3)