Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Possible detect and support pure-ruby Readline? #10

Closed
luislavena opened this Issue Dec 24, 2010 · 14 comments

Comments

Projects
None yet
2 participants

Hello,

First thank you for creating Bond and Ripl, been using it on Linux and OSX with extreme joy.

However, nothing is perfect, and Bond experience in Windows is not the best.

Under Windows, we replaced GNU Readline dependency by a pure-ruby readline named rb-readline:

http://github.com/luislavena/rb-readline

It tries to provide all the functionality from readline using pure Ruby, removing the need of a binary/compiler/C dependency.

Since the extension included in Bond just adds a missing element to the Ruby readline library, was wondering what about support RbReadline directly?

Having ripl and Bond work under Windows with rb-readline will make more life of lot of people better.

Thank you

Owner

cldwalker commented Dec 30, 2010

Hi Luis,

Sure. I'll need a pull request for a rb-readline adapter, perhaps as Bond::RbReadline. The module should define two methods, setup and line_buffer. The latter method should return the current line the user is typing. Once I get this, I'll tweak extconf.rb to not actually compile the c extension for windows (it still builds a dummy extension using make).

Owner

cldwalker commented Jan 8, 2011

As I have an increasing number of users with readline install issues, I ended up adding support for rb-readline locally. Only problem now is there is no rb-readline gem. Any reason why the gem doesn't exist? Am I supposed to point people to rdp-rb-readline?

Thank you.

There is no rb-readline distributed as gem as it was primarily distributed as package to replace RubyInstaller readline dependency.

The reason is no gem is that irb needs it prior loading rubygems, so having it as gem required loading RubyGems which was slow prior version 1.4.0.

Also, having "readline" as extension it always takes higher priority than gems, so it will always use the C extension.

I might be able to offer it as a gem, but that defeats the purpose of rb-readline in higher priority than anything user installed as gems.

Some users do not need RubyGems for IRB operations, so rb-readline is installed using setup.rb into site_ruby of your distribution.

rdp-rb-readline is not the official version and it could be out of sync with main development.

I'll see what I can do to offer both gem and installable package, but still don't think that will solve the Readline issues you're getting.

Owner

cldwalker commented Jan 8, 2011

There is no rb-readline distributed as gem as it was primarily distributed as package to replace RubyInstaller readline dependency.

In order to ship bond with rb-readline support, I'll need a gem of some kind . It's asking a lot from non-windows users to manually install your project.

Also, having "readline" as extension it always takes higher priority than gems, so it will always use the C extension.

To use rb-readline as a gem and avoid conflict with ruby's C extension, I'd propose a lib/rb-readline.rb which has:

   require File.join(File.dirname(__FILE__), 'readline')

If you want this as a pull request, I can do it.

I might be able to offer it as a gem, but that defeats the purpose of rb-readline in higher priority than anything user installed as gems

With the above proposal, I don't see how it defeats the purpose of rb-readline. It's just another way of using rb-readline.

I'll see what I can do to offer both gem and installable package, but still don't think that will solve the Readline issues you're getting.

Obviously, I don't expect rb-readline to solve users' readline issues. For that, I'll add a flag in extconf.rb to bypass actual compilation of readline headers. But with rb-readline support, I could offer them a way to use bond without having to depend on gnu readline. As you can see here, I'm having multiple debian users with readline issues who could use rb-readline.

If you'd like pull requests for the above proposal or releasing rb-readline as a gem, I can give them.

Cool, will work on this gem approach and integrate other changes from pull requests.

The only problem is that RbReadline do not provide a line_buffer functionality like the extension you did for C readline. Will take a look and see what I can do.

I can get the gem release code in place, but the additional code might take a bit longer.

Thank you for your input and interest on this.

Owner

cldwalker commented Jan 8, 2011

The only problem is that RbReadline do not provide a line_buffer functionality like the extension you did for C readline. Will take a look and see what I can do.

It does:

 RbReadline.instance_variable_get '@rl_line_buffer'

If you offer an accessor to this instance variable, it will make bond's rb-readline plugin cleaner :)

Hello,

I just pushed to rb-readline master the gem support and line_buffer. Would love if you can take a quick look before me releasing it as gem with 0.4.0 version.

Thank you.

Owner

cldwalker commented Jan 9, 2011

I'm not sure I understand wrapping the require with

unless $LOADED_FEATURES.any? { |f| f =~ /readline\.rb$/ }

If a user requires rb-readline twice, require will handle not loading a file twice. If a user accidently requires rb-readline after already having readline, they should be made aware of their mistake.

Only reason I bring this up is because it makes it harder for others to use rb-readline. For example, in ripl I'll need to make sure users load rb-readline before any readline-related libraries (which I have two of, lib/ripl/readline.rb and lib/bond/readline.rb), otherwise the require won't work. Either way you decide, it looks good to ship :)

Ruby 1.8.x do not require full paths, and 1.9.2 will differ readline paths between the bundled and the one of the gem.

Doing the double require will render RbReadline useless, so the wrapping is to avoid that corner case.

Certain versions of Ruby, including RubyInstaller bundle RbReadline into site_ruby, so installing the gem (as following wiki/readme instructions) could generate that double require issue.

I can change the require to be rbreadline instead, since that is the part that worries me.

Will check that instead of readline to avoid confusion with bond and ripl readline.

Owner

cldwalker commented Jan 9, 2011

You could also just wrap the require in unless defined? RbReadline

Thank you, I need to stop coding really late at night.

This looks good?

ConnorAtherton/rb-readline@0a76cc9

Owner

cldwalker commented Jan 10, 2011

Perfecto :)

Done!

Gem:

https://rubygems.org/gems/rb-readline

Changes:

ConnorAtherton/rb-readline@v0.3.0...v0.4.0

Now, where is my lovely adapter? :-D

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment