Pygments.rb #5

Merged
merged 1 commit into from May 15, 2012

2 participants

@smgt

I have changed the pygments wrapper from Albino to pygments.rb. This makes the highlighting about 10-15 times faster (according to pygments.rb README). I also changed the settings variable to remote_coloring to make it easier to change syntax highlighters in the feature. BUT this will also break backwards compatibility and a major version bump should be recommended.

@zzak
Owner

Thanks, I'll have to checkout pygments.rb. Can you use it on heroku?

@smgt

Albino used to power GitHubs syntax highlighting. But a long time ago they built and changed to pygments.rb to gain more speed. Reloading the python env on each request as Albino does slows down things some what.

Haven't tried pygments.rb on heroku, does albino work?

@zzak
Owner

Albino won't work on Heroku due to lack of pygmentize, so we fall back to the pygments api

@smgt

Pygments.rb have pygments included. So if there is Python there is pygments if you use pygments.rb. Found a test implementation for deploying pygments.rb to Heroku. One thing to get it to work is that you need to specify python2.6 or else it will crash on Heroku it seems.

@smgt

Removed remote pygments generation since pygments.rb works with Heroku.

@zzak
Owner

Looks good, think you could provide a test or two?

I'm hesitant on the python2.6 dependency, anyway to test this?

@namelessjon namelessjon commented on the diff May 11, 2012
lib/glorify/renderer.rb
def initialize(options={})
- @use_albino = options.fetch(:use_albino, true)
+ python = ENV['GLORIFY_PYTHON'] || 'python'
+ RubyPython.configure :python_exe => python

Do you need to configure every time you make a Renderer? Could this be moved somewhere more globally and just done once?

@smgt
smgt added a note May 12, 2012

No, default you don't need to do anything. But when deploying to heroku you can set a environment variable to overwrite which version of python to use. Since you need to run python 2.6 on Heroku to make this work.

$ heroku config:add GLORIFY_PYTHON=python2.6

That was meant more in the sense of "Does this need to be here, run every time you make a renderer? i.e. once per page view" Though I guess configuration won't be that expensive anyway.

@smgt
smgt added a note May 12, 2012

You could move it out of Glorify::Renderer if you like and set the configuration when you initialize a Glorify::Renderer instead. But I don't know how much the gain would be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@namelessjon namelessjon and 1 other commented on an outdated diff May 11, 2012
lib/glorify/renderer.rb
super
end
def block_code(code, lang)
- if use_albino
- Albino.colorize(code, lang)
- else
- Net::HTTP.post_form(URI.parse('http://pygments.appspot.com/'),
- {'code'=>code, 'lang'=>lang}).body
- end
+ Pygments.highlight(code, :lexar => lang, :options => {:encoding => "utf-8"})

I'm pretty sure this should be :lexer not :lexar

@smgt
smgt added a note May 12, 2012

Me to!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@smgt

You can try the changes out with this gist. https://gist.github.com/2665138

@smgt

I will remove the readme fix when I get home. It should not be in this PR.

@zzak
Owner

@simon I think this is good. Could you squash down the commits and maybe add some tests?

@namelessjon Any objections before I merge this and release a 0.1.0?

@smgt

I can squash it but I will not be able to add tests in the near future.

@smgt smgt Change dep from albino to pygmentize
I also changed the settings option to something more general. If
you would like to change syntax highlighter in the feature there
will be no need to change options variable.

 - Oneline options set
 - Remove remote pygments generation.
 - Make it possible to configure python version with env vars
 - Catch lexer errors
 - Remove Gemfile.lock and add it to gitignore
 - Remove albino reference from README.md
e93938f
@zzak
Owner

@simon Looks good, thanks!

I can add in the tests before the release, ping me if you want to help.

@zzak zzak merged commit 96c7ef2 into zzak:master May 15, 2012
@smgt

That would be awesome!

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