[WIP] Add shim that works for both Rouge 1 and Rouge 2

[parkr]

This PR aims to allow Rouge v1 or v2 to be used with Jekyll. This will allow users to choose to stay on the old version or to upgrade if possible/they want to.

In #5230, we learned from @jneen that we cannot rely on the same classes if folks want to upgrade to Rouge 2.

I have attempted to create a helper utility which creates the necessary Rouge formatter that we need to process our files.

While this is still a work in progress, I would appreciate any general opinions on the API design. I think I may have misinterpreted @jneen's comments in #5230.

/cc https://github.com/github/pages/issues/1359

[DirtyF]

I made some quick testing:

Given I upgrade Jekyll's repository to rouge 2.0.7
Then code snippets are not properly rendered and<pre class="highlight"><code> ... </code></pre> are missing

[Crunch09]

Hey 👋 the main issue i encountered when working on #5230 was that it didn't seem possible to rebuilt the exact same output we had when using Rouge 1.7 and i was also not sure we even want to have the same exact output, that's why i couldn't continue (#5230 (comment)).

In my opinion it would be best to have the input of some users who actually want to use the new features of Rouge 2 with Jekyll and then decide on a custom formatter (based on a provided formatter from Rouge v2 as @jneen suggested).

@parkr Still happy to help if you need any form of support on implementing this

[parkr]

In my opinion it would be best to have the input of some users who actually want to use the new features of Rouge 2 with Jekyll and then decide on a custom formatter (based on a provided formatter from Rouge v2 as @jneen suggested).

Thanks! So it sounds like all people want from Rouge v2 are the extra formatters – the Ruby API doesn't really matter. The reason we can't upgrade is that some people have written their own formatters so we can't block them from using Rouge 1.

I wonder: if they're writing their own formatters or monkey-patching Jekyll to modify how it uses Rouge, then maybe we can get away with allowing Rouge 1 & 2 in the gemspec, upgrading to Rouge 2 in our code?

@envygeeks I seem to remember you mentioning that you'd done some customization with Rouge. Can you help us understand what changes we can make to allow Rouge 2 that wouldn't affect you? If you're monkey-patching, for example, then we could just change the methods you have money-patched and be totally in the clear as long as our gemspec allows either version.

[allanbowe]

We are currently developing a lexer as a contribution to Rouge, which we'd like to then use automatically on github pages, HOWEVER - it appears that this development will be in vain unless this pull request is merged.

What are the blockers?

[Crunch09]

@allanbowe Hey, you can see the main issues mentioned in the posts above.

Seeing as we don't make progress with this as it is right now, i'd propose we settle for one custom formatter (while still supporting Rouge 1) and allow users to use their own formatter. Based on hopefully more feedback once this is out we can then improve the new formatter.

[jneen]

Hi! This has gone on for a really long time. Rouge 2.0 was released a year ago, and 1.x is no longer supported. I'm getting issues opened on Rouge about bugs that were fixed ages ago because Jekyll is still on an ancient version :\

cf. github/pages-gem#436 rouge-ruby/rouge#698

cc @gfx @dblessing @selivan

[jneen]

@Crunch09 It is possible to write a wrapper class that uses either Rouge::Formatters::HTML or Rouge::Formatters::HTMLLegacy depending on the existence of the latter, which exists in 2.x and not in 1.x. Then Jekyll should be able to unpin its rouge version number and allow it to be selected downstream.

[jneen]

Also, I'm not clear on the incompatibility story for users who wrote their own formatters - if they wrote their own, how are they affected by the existing ones changing?

[Crunch09]

@jneen Thank you for your reply! I'm not a maintainer for Jekyll though, so i think it would be best if @parkr could maybe list what needs to be done before we can release it?
Otherwise i'd be happy to continue on the weekend on either #5230 or help with this PR based on your comments here and @pathawks comment in #5230.

[jneen]

I think the best approach is to use HTMLLegacy if it exists, and otherwise fall back to HTML. They should have the same behavior. Following that, I would unpin the rouge version in the gemspec, since this gem would then work for any version of rouge.

[parkr]

#6150 supersedes this.

[parkr]

#6150 points to this PR.

[Crunch09]

@DirtyF It shouldn't, that's one of the reasons why we're using the HTMLLegacy formatter, but it might happen. That's why it's still possible to use rouge 1.11.
Thanks for your example though, it should be possible to fix this one also for the new rouge version. I could have a look tomorrow.

[DirtyF]

@Crunch09 I have a hard time understanding why HTMLLegacy does not generate the same output as in Rouge 1.x.

[parkr]

My understanding is that HTMLLegacy simply provides the same Ruby API, but it uses the new HTML-based formatters internally.

[Crunch09]

@DirtyF I think @parkr is right. From the rouge readme:

HTMLLegacy is a backwards-compatibility class intended for users of rouge 1.x,
with options that were supported then.

If users want to be sure there are absolutely zero breaking changes in their site they can (according to this PR) still use the old rouge version with upcoming minor versions of jekyll.
If they want to use the new rouge version instead, there might be some changes but IMHO that should be expected if they do a major version update.

[parkr]

Just tested on Jekyll's own website, <pre class="highlight"><code> ... </code></pre> markup is still missing, so it breaks the rendering as you can see in the screenshot

I'm still concerned about this ☝️

[pathawks]

Just tested on Jekyll's own website, <pre class="highlight"><code> ... </code></pre> markup is still missing, so it breaks the rendering as you can see in the screenshot

I don't really understand what this has to do with Rouge; isn't that stuff added on our end anyway?

[DirtyF]

@pathawks I didn't know that!

[Crunch09]

Could you please try it again with rouge 2.1.1 @DirtyF which has been released today? It seems to have fixed the issue for me locally.

[DirtyF]

@Crunch09 tested this branch on two different websites (jekyll docs and one of mine) and still no <pre> and <code> tags in the generated HTML, as a result code blocks are not properly rendered.

[pathawks]

@DirtyF Odd that the test is passing, but it isn't working for you.

[Crunch09]

@DirtyF Sorry, my mistake. Ran it without bundle exec so was not using this branch :( Also still failing for me.
@pathawks I think the code you posted will only be added when using the highlight tag. The mentioned code is only a raw tag though.

[DirtyF]

Odd that the test is passing

yeah maybe we don't have the proper rendering tests here?
On the other site I tested, the source is a pure Markdown code block, no rawor highlightLiquid tags are involved.

[Crunch09]

@DirtyF Found the issue. (This time hopefully for real 🙄 )
TL;DR: The kramdown parser is not yet released in a Rouge 2 compatible form.

Support for Rouge 2 has been added but is yet to be released.
Example:

```liquid
{% raw %}
{{ site.baseurl }}
{% endraw %}
```

Produces in Rouge 1.11 with the current kramdown release 1.13.2:

<div class="language-liquid highlighter-rouge"><pre class="highlight"><code>
<span class="p">{{ </span><!-- ... --> %}</span>
</code></pre>
</div>

Produces in Rouge 2.10 with the current kramdown release 1.13.2 (as noted by DirtyF):

<div class="language-liquid highlighter-rouge">
<span class="p">{{ </span><!-- ... --> %}</span>
</div>

Produces in Rouge 2.10 with the unreleased kramdown version (current master):

<div class="language-liquid highlighter-rouge"><div class="highlight"><pre class="highlight"><code>
<span class="p">{{ </span><!-- ... --> %}</span>
</code></pre></div></div>

The last code block adds an additional div compared to Rouge 1.11 but it will look the same as in the old Rouge version.

To try it out locally (with jekyll docs):

• The kramdown repo has to be cloned. The version on github can't be used directly in the Gemfile as it doesn't include a .gemspec.
• After cloning run:

rake install
rake gemspec 

• Now reference the local version in Jekyll's Gemfile and run bundle
• Remove {% feed_meta %} from docs/_includes/top.html, otherwise the docs site will not built. (still investigating why)
• Serve the docs

I could submit a failing PR for this behavior but not sure it'll be worth it as it'll only pass once a new kramdown version has been released.

[Crunch09]

Asked now if we could have a new kramdown release: gettalong/kramdown#439

[pathawks]

I could have sworn that at one point we were overriding kramdown's built-in highlighting so that it would be as if the user had used the {% highlight %} tag instead. Fever dream?

[Crunch09]

I don't know what the release plan for jekyll is but given that 3.5.1 has been released and this is in the 3.6 milestone and all tests are passing, would it make sense to merge this into master?
My assumption is that at least some people would test their site with the latest master regularly and we might get helpful feedback on the rouge 2 integration. What do you think?

[DirtyF]

@Crunch09 I'm all for it, just waitin' on @parkr's call on this.

[parkr]

This is the main drive for v3.6.0. I've branched off a 3.5-stable so we can do patch releases for 3.5 so master is now for 3.6. Let's get this merged.

@jekyllbot: merge +minor