chriswailes / RLTK

The Ruby Language Toolkit
http://chriswailes.github.io/RLTK/
University of Illinois/NCSA Open Source License
453 stars 35 forks source link

Add Rubinius compatibility #27

Closed benedikt closed 10 years ago

benedikt commented 11 years ago

This pull request adds Rubinius compatibility (fixes issue #19). The only problem that prevented RLTK from running on Rubinus, was the different behavior of Array#each in Rubinius. While it is possible to append new elements to the array while enumerating it in MRI, the enumeration would stop after the original number of elements in Rubinus.

This pull request changes the behavior of RLTK::Parser::State#each to properly enumerate all items, including those added while enumerating.

Additionally it adds RLTK::Parser#each_state to enable the same behavior for the @states array.

Please let my know, if you want me to adjust my implementation in any way.

Thanks a lot for the RLTK gem. It's been a pleasure to work with it!

chriswailes commented 10 years ago

I've been working on updating RLTK and beginning active development again. Before I merge this patch I want to make sure this is still an issue. If you can confirm it, then I'll merge it for the upcoming 3.0.0 release.

benedikt commented 10 years ago

Hi Chris,

I talked about this with members of the #rubinius channel on Freenode a while back. According to @brixen, the behavior of #each when the collection gets modified during the iteration is not defined in Ruby, so it's dangerous to rely on the current behavior of MRI. It might change in the future.

This pull request changes RLTK so it does not rely on #each also iterating over items that get added during the iteration.

chriswailes commented 10 years ago

Done and done. Thanks. I'm going to wait a couple of days before I publish RLTK 3.0, so if you want to test it out on Rubinius to make sure it works now, I'd love some additional feedback.

benedikt commented 10 years ago

Thanks a lot for merging this, Chris! I'm looking forward to the 3.0 release.

I wasn't able to run the RLTK test suite for some reason (neither with MRI, nor with Rubinius; see below). However I tried running my project's tests, both on MRI and Rubinius using RLTK 3.0 and it worked fine on both.

$ bundle exec rake test --trace
rake aborted!
undefined local variable or method `print_warning' for main:Object
/Users/benedikt/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/gems/filigree-0.2.0/lib/filigree/request_file.rb:27:in `rescue in request_file'
/Users/benedikt/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/gems/filigree-0.2.0/lib/filigree/request_file.rb:23:in `request_file'
/Users/benedikt/Workspace/Ruby/RLTK/Rakefile:81:in `<top (required)>'
/Users/benedikt/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/gems/rake-0.9.2.2/lib/rake/rake_module.rb:25:in `load'
/Users/benedikt/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/gems/rake-0.9.2.2/lib/rake/rake_module.rb:25:in `load_rakefile'
/Users/benedikt/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/gems/rake-0.9.2.2/lib/rake/application.rb:501:in `raw_load_rakefile'
/Users/benedikt/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/gems/rake-0.9.2.2/lib/rake/application.rb:82:in `block in load_rakefile'
/Users/benedikt/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/gems/rake-0.9.2.2/lib/rake/application.rb:133:in `standard_exception_handling'
/Users/benedikt/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/gems/rake-0.9.2.2/lib/rake/application.rb:81:in `load_rakefile'
/Users/benedikt/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/gems/rake-0.9.2.2/lib/rake/application.rb:65:in `block in run'
/Users/benedikt/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/gems/rake-0.9.2.2/lib/rake/application.rb:133:in `standard_exception_handling'
/Users/benedikt/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/gems/rake-0.9.2.2/lib/rake/application.rb:63:in `run'
/Users/benedikt/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/gems/rake-0.9.2.2/bin/rake:33:in `<top (required)>'
/Users/benedikt/.rbenv/versions/2.0.0/bin/rake:23:in `load'
/Users/benedikt/.rbenv/versions/2.0.0/bin/rake:23:in `<main>'
chriswailes commented 10 years ago

Sorry, that was my fault. I apparently didn't test a code path in Filigree. I've fixed it now.

Did you re-run bundle install? It was supposed to print out a warning that rubygems-tasks wasn't installed.

benedikt commented 10 years ago

Yes, I did run bundle install. I just pulled the current revision, but it doesn't work yet. Is the commit pushed already?

chriswailes commented 10 years ago

I just pushed the updated Filigree gem. You should see it now. I'm not sure why rubygems-tasks isn't being found.

benedikt commented 10 years ago

Ah, I see! After running bundle update, everything worked fine. The tests run on both MRI and Rubinius. There's just a problem with SimpleCov in Rubinius, but looking at the backtrace, it's not related to RLTK.

Maybe adding Gemfile.lock to .gitignore would be an option to prevent confusion in the future?

chriswailes commented 10 years ago

Will do. Thanks for the testing.

While I have your attention, do you have any features you would really like to see in the 3.0 release of RLTK?

benedikt commented 10 years ago

I'm quite new to working with parser generators, so I really don't have an idea what's possible and what's not.

One thing that might already be possible, but I couldn't figure out, is how to handle errors while parsing. At the moment, I'm just getting a "String not in language" exception when I try to parse an invalid document. If there would be a way to know what exactly caused this, it would be very helpful. If this is already possible, then a bit more documentation about that would be nice, because I have no idea how to build that :)

chriswailes commented 10 years ago

What you are really looking for are error productions. These allow the parser to gracefully handle bad input. If you place them in the right spot (which is an art as much as it is a science) you will be able to recover from missing semi-colons, un-terminated strings, and other such common errors. The documentation doesn't cover all of the complexities of using them, but you should be able to find better explanations on the web with some searching.

I can also try and make the "String not in language" exception a bit more informative, and tell you where the parsing broke down.

benedikt commented 10 years ago

I'll have a deeper look into error productions then :) Thank you!