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

File/IO readlines supporting new params limit, and open mode #650

merged 2 commits into from Apr 24, 2013


None yet
2 participants
Copy link

commented Apr 23, 2013

It fixes 12 failures, and 9 errors of the rubyspec test core/io/readlines_spec.rb
Let me know if I need to change anything.


This comment has been minimized.

Copy link

commented Apr 23, 2013

@josedonizetti This patch looks good but there is one problem with it. The @JRubyMethod annotations for both of those method bindings will work in both 1.8 and 1.9 mode. The new argument support you are adding should only work for 1.9 mode.

Could you refactor this to support both modes (You can see an example of this splitting in RubyIO.sysopen()/RubyIO.sysopen19())? Mostly this just means having 4 Java methods 2 for 1.8 and 2 for 1.9 with usually a common method which can perform the work. Be aware due to backwards compat that the original Java method names for these need to keep their names and the same method signature. The 1.9 versions can be labelled readlines19().


This comment has been minimized.

Copy link
Member Author

commented Apr 24, 2013

@enebo With this last commit, the support is exclusive for 1.9 not affecting 1.8 at all. Also I've fixed all the specs related to IO.readlines but two. One because of enconding, when reading the files at the time of comparing the characters are all corrupted and the other because JRuby don't yet support IO.popen("-") we get this error on the 1.8 as well. Let me know if anything. Thanks!

enebo added a commit that referenced this pull request Apr 24, 2013

Merge pull request #650 from josedonizetti/io_readlines
File/IO readlines supporting new params limit, and open mode (thanks jose!)

@enebo enebo merged commit a09d139 into jruby:master Apr 24, 2013

1 check failed

default The Travis build failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.