Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Rouge 2] use Rouge::Formatters::HTMLLegacy and fix tests #6150

Merged
merged 3 commits into from Jun 20, 2017
Merged

[Rouge 2] use Rouge::Formatters::HTMLLegacy and fix tests #6150

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
48b8da6
use `Rouge::Formatters::HTMLLegacy` and fix tests
Crunch09 Jun 18, 2017
198ef89
set default rouge version to 2.1.0
Crunch09 Jun 18, 2017
d3c3ec4
allow more than just 2.1.x versions by default
Crunch09 Jun 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion jekyll.gemspec
View file
Open in desktop
Expand Up @@ -38,6 +38,6 @@ Gem::Specification.new do |s|
s.add_runtime_dependency("liquid", "~> 4.0")
s.add_runtime_dependency("mercenary", "~> 0.3.3")
s.add_runtime_dependency("pathutil", "~> 0.9")
s.add_runtime_dependency("rouge", "~> #{ENV["ROUGE_VERSION"] || "1.7"}")
s.add_runtime_dependency("rouge", "~> #{ENV["ROUGE_VERSION"] || "2.1.0"}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work with 2.0 & 2.2, or is only 2.1.x compatible? Usually we'd just use 2 digits here, e.g. 2.0.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 It should also work with any 2.x version, i'll update!

s.add_runtime_dependency("safe_yaml", "~> 1.0")
end
10 changes: 5 additions & 5 deletions lib/jekyll/utils/rouge.rb
View file
Open in desktop
Expand Up @@ -4,16 +4,16 @@ module Rouge

def self.html_formatter(*args)
Jekyll::External.require_with_graceful_fail("rouge")
html_formatter = ::Rouge::Formatters::HTML.new(*args)
return html_formatter if old_api?

::Rouge::Formatters::HTMLPygments.new(html_formatter)
if old_api?
::Rouge::Formatters::HTML.new(*args)
else
::Rouge::Formatters::HTMLLegacy.new(*args)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the new API look like? is this just a wrapper around the new functionality with the old API?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly. It produces (almost) the same output as rouge 1.11 while receiving the same arguments.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are all default formatters listed. IMHO line numbers (which we rely on in a couple of tests) would also be present in Rouge::Formatters::HTMLTable and Rouge::Formatters::HTMLLinewise but HTMLLegacy should be the easiest update.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does HTMLLegacy not have those features?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, sorry, this was unclear: HTMLLegacy also has line numbers and all features we currently use.
The 2.x version of the standard HTML formatter doesn't have line numbers.

end
end

def self.old_api?
::Rouge.version.to_s < "2"
end

end
end
end
5 changes: 3 additions & 2 deletions test/test_kramdown.rb
View file
Open in desktop
Expand Up @@ -62,8 +62,9 @@ class TestKramdown < JekyllUnitTest
puts "Hello World"
~~~
MARKDOWN

selector = "div.highlighter-rouge>pre.highlight>code"
div_highlight = ""
div_highlight = ">div.highlight" unless Utils::Rouge.old_api?
selector = "div.highlighter-rouge#{div_highlight}>pre.highlight>code"
refute result.css(selector).empty?
end

Expand Down
12 changes: 6 additions & 6 deletions test/test_tags.rb
View file
Open in desktop
Expand Up @@ -430,22 +430,22 @@ def highlight_block_with_opts(options_string)
EOS
end

should "should stop highlighting at boundary with rouge 1" do
should "should stop highlighting at boundary with rouge 2" do
skip "Skipped because using an older version of Rouge" if Utils::Rouge.old_api?
expected = <<-EOS
<p>This is not yet highlighted</p>
<figure class="highlight"><pre><code class="language-php" data-lang="php"><table style="border-spacing: 0"><tbody><tr><td class="gutter gl" style="text-align: right"><pre class="lineno">1</pre></td><td class="code"><pre>test<span class="w">
</span></pre></td></tr></tbody></table></code></pre></figure>
<p>This is not yet highlighted</p>\n
<figure class="highlight"><pre><code class="language-php" data-lang="php"><table class="rouge-table"><tbody><tr><td class="gutter gl"><pre class="lineno">1
</pre></td><td class="code"><pre><span class="nx">test</span></pre></td></tr></tbody></table></code></pre></figure>\n
<p>This should not be highlighted, right?</p>
EOS
assert_match(expected, @result)
end

should "should stop highlighting at boundary with rouge 2" do
should "should stop highlighting at boundary with rouge 1" do
skip "Skipped because using a newer version of Rouge" unless Utils::Rouge.old_api?
expected = <<-EOS
<p>This is not yet highlighted</p>\n
<figure class="highlight"><pre><code class="language-php" data-lang="php"><table style="border-spacing: 0"><tbody><tr><td class="gutter gl" style="text-align: right"><pre class="lineno">1</pre></td><td class="code"><pre>test<span class="w">
<figure class="highlight"><pre><code class="language-php" data-lang="php"><table style="border-spacing: 0"><tbody><tr><td class="gutter gl" style="text-align: right"><pre class="lineno">1</pre></td><td class="code"><pre>test<span class="w">\n
</span></pre></td></tr></tbody></table></code></pre></figure>\n
<p>This should not be highlighted, right?</p>
EOS
Expand Down