Skip to content
This repository has been archived by the owner on May 26, 2019. It is now read-only.

Swaps out Nokogiri implementation for semi custom encoder #137

Closed
wants to merge 1 commit into from
Closed

Swaps out Nokogiri implementation for semi custom encoder #137

wants to merge 1 commit into from

Conversation

danmcclain
Copy link
Contributor

There is no way to insert a clean patch into CodeRay's encoder, so I had to copy/pasta some of the implementation to inject the changes

@danmcclain
Copy link
Contributor Author

@trek Let me know what you think, feels bad, but really no better way to do it 😒

@trek
Copy link
Member

trek commented Mar 30, 2015

Why can we not PR against CoderRay to just get this into their library? I actually prefer the Nokogiri solution to this. I don't want to keep supporting a fork of this library.

@rwjblue
Copy link
Member

rwjblue commented Mar 30, 2015

This is mostly to make the custom line highlighting work, and the internal coderay structure is not really setup to make this sort of extension easy.

IMHO, the modifications of the HTML output via Nokogiri was much cleaner and easier to deal with longer term. I cannot reason about the internal coderay structure, but manipulating HTML with nokogiri is pretty simple.

@trek trek mentioned this pull request Apr 3, 2015
@danmcclain
Copy link
Contributor Author

This would be a lot of effort to bring into CodeRay, and that assumes that they'd want to accept it at all. It's a lot of hackery to get to a place to do this work. If you look, I'm still splitting the code on new lines inside of CodeRay, because it is no clear spot to put the line wrapping code (there is a begin_line method that never gets called.

The nokogiri solution is cleaner in my opinion, and CodeRay is not easy to patch the way it is currently implemented

@trek
Copy link
Member

trek commented Apr 10, 2015

Closed in favor of #144

@trek trek closed this Apr 10, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants