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.

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

Avoid duplicated output using highlight tags #2264

Merged
merged 1 commit into from Apr 27, 2014

Conversation

Projects
None yet
3 participants
@robin850
Contributor

robin850 commented Apr 27, 2014

Hello,

This tiny patch fixes a bug while using Rouge with highlight tags. The output was duplicated since the output variable in the Liquid tag definition was equal to the highlighter's prefix value and the << method changes its receiver.

Therefore, we should simply define an empty string and append the prefix if it is present.

I noticed that this is only happening with posts (not index.html for instance).

I did not manage to write a failing test though so I wrote a feature (see this gist ; let me know if I'm doing something wrong). The assertion is also a bit tricky/brittle ; let me know if you have a better idea to ensure that there is only a single .highlight div.

There are also other pull requests about this area of the code (i.e. #2221 and #2154) but I don't know which one you are going to ship and Jekyll 2.0 is already at the release candidate step so I take the liberty to send the proper fix here.

Have a nice day.

Avoid duplicated output using highlight tags
While using Rouge and an `highlight` tag, the output was duplicated
since the `output` variable in the Liquid tag definition was equal to
the highlighter's prefix value and the `<<` method changes its receiver.

Therefore, we should simply define an empty string and append the prefix
if it is present.
@parkr

This comment has been minimized.

Member

parkr commented Apr 27, 2014

Failing test seems to be unrelated. I'm 👍.

@parkr parkr merged commit 3a61088 into jekyll:master Apr 27, 2014

1 check failed

continuous-integration/travis-ci The Travis CI build failed
Details

parkr added a commit that referenced this pull request Apr 27, 2014

@parkr

This comment has been minimized.

Member

parkr commented Apr 27, 2014

Thank you so much, @robin850! ❤️

@robin850

This comment has been minimized.

Contributor

robin850 commented Apr 27, 2014

My pleasure. Thanks for merging ! ❤️

@robin850 robin850 deleted the robin850:duplicated-output-rouge branch Apr 27, 2014

@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.