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

Loosen Pygments Dependency #1015

Merged
merged 3 commits into from May 5, 2013

Conversation

Projects
None yet
5 participants
@parkr
Member

parkr commented May 4, 2013

We bundle pygments.rb with Jekyll, but it doesn't make sense to require it if pygments is set to false or when it's not explicitly needed. This shuffles around the requiring to be just where it's needed.

Fixes #1006.

module Jekyll
module Logger
class Logger < Logger

This comment has been minimized.

@parkr

parkr May 4, 2013

Member

Without this line, I was getting ridiculous errors from Redcarpet tests:

TypeError: Logger is not a class
    /Users/parker/.rbenv/versions/1.9.3-p362/lib/ruby/gems/1.9.1/gems/pygments.rb-0.4.2/lib/pygments/popen.rb:5:in `require'
    /Users/parker/.rbenv/versions/1.9.3-p362/lib/ruby/gems/1.9.1/gems/pygments.rb-0.4.2/lib/pygments/popen.rb:5:in `<top (required)>'
    /Users/parker/.rbenv/versions/1.9.3-p362/lib/ruby/gems/1.9.1/gems/pygments.rb-0.4.2/lib/pygments.rb:1:in `require'
    /Users/parker/.rbenv/versions/1.9.3-p362/lib/ruby/gems/1.9.1/gems/pygments.rb-0.4.2/lib/pygments.rb:1:in `<top (required)>'
    /Users/parker/code/jekyll/lib/jekyll/converters/markdown/redcarpet_parser.rb:44:in `require'
    /Users/parker/code/jekyll/lib/jekyll/converters/markdown/redcarpet_parser.rb:44:in `load_pygments'
    /Users/parker/code/jekyll/lib/jekyll/converters/markdown/redcarpet_parser.rb:7:in `initialize'
    /Users/parker/code/jekyll/lib/jekyll/converters/markdown.rb:13:in `new'
    /Users/parker/code/jekyll/lib/jekyll/converters/markdown.rb:13:in `setup'
    /Users/parker/code/jekyll/lib/jekyll/converters/markdown.rb:38:in `convert'
    /Users/parker/code/jekyll/test/test_redcarpet.rb:30:in `block (2 levels) in <class:TestRedcarpet>'
    /Users/parker/.rbenv/versions/1.9.3-p362/lib/ruby/gems/1.9.1/gems/shoulda-context-1.0.2/lib/shoulda/context/context.rb:398:in `call'
    /Users/parker/.rbenv/versions/1.9.3-p362/lib/ruby/gems/1.9.1/gems/shoulda-context-1.0.2/lib/shoulda/context/context.rb:398:in `block in create_test_from_should_hash'
    /Users/parker/.rbenv/versions/1.9.3-p362/lib/ruby/gems/1.9.1/gems/mocha-0.10.5/lib/mocha/integration/mini_test/version_230_to_262.rb:28:in `run'
@mattr-

This comment has been minimized.

Member

mattr- commented May 5, 2013

Redcarpet is a rather fickle thing. ;-)

👍

@AlexanderEkdahl

This comment has been minimized.

Contributor

AlexanderEkdahl commented May 5, 2013

Awesome! I'm all for loosening dependencies. How about classifier? Since it depends on a C-extension.

parkr added a commit that referenced this pull request May 5, 2013

@parkr parkr merged commit 1b994a9 into master May 5, 2013

1 check passed

default The Travis build passed
Details

@parkr parkr deleted the loose-pygments-dep branch May 5, 2013

parkr added a commit that referenced this pull request May 5, 2013

@parkr parkr referenced this pull request May 5, 2013

Closed

Hard dependency on Pygments #1006

@hendricius

This comment has been minimized.

hendricius commented on ca888ca May 14, 2013

Causes an error for me:

/Users/hendricius/Dropbox/Sites/forked/jekyll/lib/jekyll/logger.rb:4:in `<module:Jekyll>': superclass mismatch for class Logger (TypeError)
from /Users/hendricius/Dropbox/Sites/forked/jekyll/lib/jekyll/logger.rb:3:in `<top (required)>'
from /Users/hendricius/Dropbox/Sites/forked/jekyll/lib/jekyll.rb:31:in `require'
from /Users/hendricius/Dropbox/Sites/forked/jekyll/lib/jekyll.rb:31:in `<top (required)>'

This comment has been minimized.

hendricius replied May 14, 2013

Not sure, maybe an issue with me having a Gem that defines logger? Renaming it to "Pony" or something else resolved the error.

This comment has been minimized.

Member

parkr replied May 14, 2013

Running which version of ruby?

This comment has been minimized.

hendricius replied May 14, 2013

More diagnosing:

screen shot 2013-05-14 at 12 43 55 pm

This comment has been minimized.

hendricius replied May 14, 2013

ruby 1.9.3p392 (2013-02-22 revision 39386) [x86_64-darwin12.3.0]

renaming it to:

module Jekyll
 class Logger
    # Public: Print a jekyll message to stdout
    #

resolves the issue too. not sure if it is correct though.

cheers to berlin.

This comment has been minimized.

hendricius replied May 14, 2013

This comment has been minimized.

Member

mattr- replied May 14, 2013

This comment has been minimized.

hendricius replied May 14, 2013

This comment has been minimized.

Member

parkr replied May 14, 2013

How 'bout Stevenson? He's the author of Jekyll & Hyde ;)

This comment has been minimized.

Member

parkr replied May 14, 2013

Renamed: #1106

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