highlight: fix problem with linenos and rouge. #3436

Merged
merged 2 commits into from Feb 10, 2015

Conversation

Projects
None yet
4 participants
@parkr
Member

parkr commented Feb 10, 2015

Found by @EdMcBane in #3435

The strange regexp we were doing to replace the <pre><code></pre></code>
bits in the Pygments output were wreaking havoc on Rouge output
because Rouge uses <pre>'s to wrap line numbers.

To be consistent, the output from render_* should not include
the wrapping <div> and <pre> tags. It should just be what was
inside. We can then wrap it in our own custom tags without using
any regular expressions, as God intended. Death to regular
expressions and HTML manipulation!

parkr added some commits Feb 10, 2015

highlight: fix problem with linenos and rouge.
Found by @EdMcBane in #3435

The strange regexp we were doing to replace the <pre><code></pre></code>
bits in the Pygments output were wreaking havoc on Rouge output
because Rouge uses <pre>'s to wrap line numbers.

To be consistent, the output from render_* should *not* include
the wrapping <div> and <pre> tags. It should just be what was
inside. We can then wrap it in our own custom tags without using
any regular expressions, as God intended. Death to regular
expressions and HTML manipulation!
highlight: duplicate tests for pygments for rouge
Ensure that the output we get for pygments will match
that we get for rouge in all cases except line numbers.

@parkr parkr added the fix label Feb 10, 2015

@parkr parkr merged commit b81f6ed into master Feb 10, 2015

2 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci The Travis CI build passed
Details

@parkr parkr deleted the fix-highlight-madness branch Feb 10, 2015

parkr added a commit that referenced this pull request Feb 10, 2015

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Mar 5, 2015

Member

This broke my site.

I had a plugin that was overriding add_code_tag to replace <div class="highlight"> with some custom tags. Now that <div class="highlight"> isn't in code, add_code_tag needs to be sure to add it.

Overall, I think this is much better than how it was being done in the past, but some documentation would be nice. I was pulling my hair out trying to track down the problem.

Member

pathawks commented Mar 5, 2015

This broke my site.

I had a plugin that was overriding add_code_tag to replace <div class="highlight"> with some custom tags. Now that <div class="highlight"> isn't in code, add_code_tag needs to be sure to add it.

Overall, I think this is much better than how it was being done in the past, but some documentation would be nice. I was pulling my hair out trying to track down the problem.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Mar 5, 2015

Member

@pathawks This probably isn't a huge concern as I can't imagine lots of people rewriting/reworking the code highlighting. I'm glad you (seemingly quickly) figured out what was wrong and fixed it. I don't think this change is appropriate for the Upgrading doc, but it's in the History 8-)

If you write a blog post about it, Google will probably put you front and center for anyone else having this problem :)

Member

parkr commented Mar 5, 2015

@pathawks This probably isn't a huge concern as I can't imagine lots of people rewriting/reworking the code highlighting. I'm glad you (seemingly quickly) figured out what was wrong and fixed it. I don't think this change is appropriate for the Upgrading doc, but it's in the History 8-)

If you write a blog post about it, Google will probably put you front and center for anyone else having this problem :)

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Mar 5, 2015

Contributor

Good thing this is beta not stable 😼

Contributor

envygeeks commented Mar 5, 2015

Good thing this is beta not stable 😼

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Mar 5, 2015

Member

If you write a blog post about it, Google will probably put you front and center for anyone else having this problem :)

👍

Good thing this is beta not stable

Yup, that's why I'm testing and reporting things that break 😃

Member

pathawks commented Mar 5, 2015

If you write a blog post about it, Google will probably put you front and center for anyone else having this problem :)

👍

Good thing this is beta not stable

Yup, that's why I'm testing and reporting things that break 😃

@asl97 asl97 referenced this pull request Feb 28, 2016

Closed

Faulty linenos #3049

@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.