Skip to content
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

Fixed deprecated method call in list parser #30

Closed
wants to merge 1 commit into from

Conversation

@marxjohnson
Copy link

@marxjohnson marxjohnson commented Nov 27, 2012

The current code produces a deprecated warning in Ruby 1.8 and fails in Ruby 1.9 when parsing a definition list. This is due to use of a .type method. I've changed this to .class which fixes the problem.

@ghost ghost assigned gettalong Nov 28, 2012
@gettalong
Copy link
Owner

@gettalong gettalong commented Nov 28, 2012

This should be no bug, this just uses the Element#type accessor. I always run the tests under Ruby 1.8. and 1.9 and I don't get a warning or a failure.

How did you test? What Ruby versions? What OS?

@gettalong
Copy link
Owner

@gettalong gettalong commented Nov 28, 2012

Okay, after some more thinking why this could happen I have a test case which triggers a NoMethodError under Ruby 1.9:

test
: 

hallo
: test

The problem is not that there should be .class instead of .type but that the it.children is empty. Will have to investigate a bit more and then fix it.

Thanks for reporting this!

@marxjohnson
Copy link
Author

@marxjohnson marxjohnson commented Nov 28, 2012

Sorry, I should really have filed a proper bug report first, I got over-excited.

Running Ubuntu 12.04, Ruby 1.8.7, using kramdown 0.14.0 through jekyll, I get this warning:

/home/mark-work/.rvm/gems/ruby-1.8.7-p371/gems/kramdown-0.14.0/lib/kramdown/parser/kramdown/list.rb:217: warning: Object#type is deprecated; use Object#class

With Ruby 1.9.3, it crashes thusly:

   /home/mark-work/.rvm/gems/ruby-1.9.3-p327/gems/kramdown-0.14.0/lib/kramdown/parser/kramdown/list.rb:217:in   'block in parse_definition_list': undefined method 'type' for nil:NilClass (NoMethodError)
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/gems/kramdown-0.14.0/lib/kramdown/parser/kramdown/list.rb:205:in 'each'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/gems/kramdown-0.14.0/lib/kramdown/parser/kramdown/list.rb:205:in 'parse_definition_list'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/gems/kramdown-0.14.0/lib/kramdown/parser/kramdown.rb:140:in 'block (2 levels) in parse_blocks'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/gems/kramdown-0.14.0/lib/kramdown/parser/kramdown.rb:138:in 'each'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/gems/kramdown-0.14.0/lib/kramdown/parser/kramdown.rb:138:in 'any?'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/gems/kramdown-0.14.0/lib/kramdown/parser/kramdown.rb:138:in 'block in parse_blocks'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/gems/kramdown-0.14.0/lib/kramdown/parser/kramdown.rb:135:in 'catch'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/gems/kramdown-0.14.0/lib/kramdown/parser/kramdown.rb:135:in 'parse_blocks'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/gems/kramdown-0.14.0/lib/kramdown/parser/kramdown.rb:101:in 'parse'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/gems/kramdown-0.14.0/lib/kramdown/parser/base.rb:78:in 'parse'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/gems/kramdown-0.14.0/lib/kramdown/document.rb:119:in 'initialize'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/gems/jekyll-0.11.2/lib/jekyll/converters/markdown.rb:110:in 'new'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/gems/jekyll-0.11.2/lib/jekyll/converters/markdown.rb:110:in 'convert'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/gems/jekyll-0.11.2/lib/jekyll/convertible.rb:46:in 'transform'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/gems/jekyll-0.11.2/lib/jekyll/convertible.rb:84:in 'do_layout'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/gems/jekyll-0.11.2/lib/jekyll/page.rb:100:in 'render'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/gems/jekyll-0.11.2/lib/jekyll/site.rb:197:in 'block in render'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/gems/jekyll-0.11.2/lib/jekyll/site.rb:196:in 'each'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/gems/jekyll-0.11.2/lib/jekyll/site.rb:196:in 'render'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/gems/jekyll-0.11.2/lib/jekyll/site.rb:40:in 'process'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/gems/jekyll-0.11.2/bin/jekyll:250:in '<top (required)>'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/bin/jekyll:19:in 'load'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/bin/jekyll:19:in '<main>'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/bin/ruby_noexec_wrapper:14:in 'eval'
    from /home/mark-work/.rvm/gems/ruby-1.9.3-p327/bin/ruby_noexec_wrapper:14:in '<main>'

My colleague can reproduce the problem on Mac OSX with Ruby 1.9.3. The proposed patch fixes both cases.

@marxjohnson
Copy link
Author

@marxjohnson marxjohnson commented Nov 28, 2012

I can confirm that the error is caused by files containing the pattern you describe above. I've tried running one of the files through kramdown manually (without jekyll) and the error is produced.

@gettalong
Copy link
Owner

@gettalong gettalong commented Nov 28, 2012

Thanks for confirming the issue! However, as explained in my first comment, the proposed fix is a fix, but not the correct one. There is a problem in the control flow of the method - will look at it later today.

@marxjohnson
Copy link
Author

@marxjohnson marxjohnson commented Nov 29, 2012

I appreciate that now. Thanks for looking in to this.

@gettalong
Copy link
Owner

@gettalong gettalong commented Nov 30, 2012

Fixed the underlying issue and released new version 0.14.1!

@gettalong gettalong closed this Nov 30, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.