resolves #99 add support for JRuby

pygments / pygments.rb

💎 Ruby wrapper for Pygments syntax highlighter

MIT License

572 stars 141 forks source link

resolves #99 add support for JRuby #162

Closed mojavelinux closed 7 years ago

mojavelinux commented 8 years ago

When RUBY_ENGINE == 'jruby', do the following:

map popen4 function to Open.popen3
map Yajl to MultiJson and set MultiJson engine to json_gem
update gemspec to build -java gem when running under JRuby
drop posix/spawn and yaml dependencies and add multi_json

Additionally, add rake and test-unit as development dependencies so rake and tests can be run via Bundler, respectively.

mojavelinux commented 8 years ago

Please note of the following facts:

This patch does not change how pygments.rb functions under MRI in any way.
All the tests run successfully under JRuby 9.1.2.0
Installation and requiring of the library works exactly the same on all platforms (Ruby automatically selects the right version to install and load).
This gem must be released using JRuby (it will automatically upload the platform-specific variant).

mojavelinux commented 8 years ago

I recognize that Rouge is now the preferred gem over pygments.rb. However, there are still integrations which rely on this gem that haven't had a chance to make the switch. Support for JRuby would really help in this transition period.

gfx commented 7 years ago

This gem depends on the very POSIX process model. If you want to execute pygments for JRuby, it should be better that you call pygmentize(1) command directly.

Or, consider https://github.com/jneen/rouge, which is written in pure Ruby.

mojavelinux commented 7 years ago

Seriously?!? Come on. This fix is critical for compatibility across Ruby platforms (esp during the transition to Rouge, which can't happen overnight). I spent a lot of time to figure this solution out and I think it is very reasonable. Can you please respect that effort and not close the door on JRuby? I urge you to reconsider.

gfx commented 7 years ago

Hmm, okay, will try to get compatibility with JRuby and keep it as much as possible.

However, dynamic dependencies in gemspec are useless. Can you fix some points?

Make POSIX::Spawn optional and add a configuring point to select POSIX::Spawn or pure Ruby open4
Always use MutiJson

mojavelinux commented 7 years ago

Thank you. I'd be glad to update the patch. Stay tuned.

gfx commented 7 years ago

The idea of this PR is included in #170. Thanks to the suggestion.