JRuby 1.7

[jeg2]

JRuby 1.7

[mnzaki]

JRuby moved from jline1 to jline2.7 on April 17 (jruby/jruby@bb7dc31)
I suppose highline is expecting jline1

[JEG2]

I would love to see this fixed. Anyone who know's jline and offers up a patch is my hero.

[mnzaki]

I was just beginning work on a patch. Quick clarification: what's up with all the disableEcho and enableEcho for JRuby and not for other platforms? A lot of the calls (gets, readline, getbyte, etc) on $stdin/$stdout work for JRuby just as well, and I don't see why all the extra logic (and echo disable/re-enable) is needed.

[mnzaki]

Here's an initial patch, mnzaki/highline@721bc1e6194d7a79b19
I'm going to try to refactor a few parts then issue a pull request.

[mnzaki]

I have pushed more changes to the jruby-1.7 branch, which clean up the terminal code a bit. This needs more testing of course. I've tested ncurses, stty and jline.

stty and jline seem fine.

ncurses breaks the console if the process gets a SIGSTOP (ctrl-z on the terminal) and SIGCONT ("fg").
Also, after ending the process the terminal does not handle scroll events properly anymore. Usually when in secondary screen mode, the scroll events simply produce up and down keypresses, and when secondary screen mode is terminated, the scroll events are captured again. It seems like we are not closing down ncurses properly or something else is wrong.

I'd recommend giving 'jline' higher priority than 'ncurses' mode.

If you wish, you can just pull the first commit which makes minimal changes (specific to JRuby) and does not touch anything else.

[JEG2]

Wow, thanks for working on this.

I agree with you assessment. Let's bump line over ncurses.

[mnzaki]

I need the JRuby functionality, so it's only natural that I work on it :-D

The ncurses terminal breakage is actually a JRuby bug, and an important one IMO. JRuby doesn't seem to run the 'ensure' blocks if the process gets a SIGINT, so NCurses::endwin is never called if we die suddenly. I'll be reporting this. MRI handles this correctly, and ncurses works well there (sort of; I still don't like the occasional screen blank).

I've rearranged the choices to give jline (which is much more reliable) a higher priority, and I removed the comment about needing ncurses for jruby.

BTW, all these changes will probably break highline horribly for users of jruby before 1.5 (and possible 1.6)

[JEG2]

Hmm, we probably need an option for older JRuby versions. Thoughts?

[mnzaki]

Not sure. You could keep the changes on a different branch for a couple months then merge into master later. The JLine change in JRuby has been since mid April, that's about 2 months ago. I'm guessing there'll be considerably more people using the new JRuby in a month or two.

[tomdz]

You could distinguish the jruby versions using JRUBY_VERSION, and keep the jline1 code around for < 1.7.

[JEG2]

I think I prefer tomdz's approach. HighLine is a general purpose library and we're going to need to support it for a while yet.

[mnzaki]

That sounds better. What do you think of the rest of the changes in the branch though?
I'm off on vacation for a week, I might work on this when I'm back if you don't.

[JEG2]

It's looking good to me. I think you're on the right track.

[nirvdrum]

If it helps any, I've tried @mnzaki's branch on JRuby 1.7.0-preview2 in the context of Capistrano and everything has been working great.

[JEG2]

Great. So we just need to sort out the backwards compatibility issues and these changes are a go.

[nirvdrum]

@tomdz's solution makes sense. Otherwise, you could try to require one, rescue the exception, and then try to require the other.

[JEG2]

Yeah, we just need someone to get the patch ready.

[markmandel]

@mnzaki - Just tested your branch as well. Worked a charm (was having exactly the same issue). Thanks for putting in the effort.

[mnzaki]

Apologies for the delay, but my organization has moved away from JRuby for now.
I have update the branch and added back support for 1.6
I have done some basic testing under JRuby 1.6 and 1.7, and MRI 1.9. More testing is probably needed to make sure I didn't break anything.

I will create a pull request now.

[JEG2]

I've merged the changes. Thank you.

Anyone else willing to double-check this stuff before I release a new gem?

[ryanb]

@JEG2 I had the error mentioned in this issue and using the master branch fixed it. I will be mentioning this fix in a RailsCasts episode today but it would be nice to get a release soon. Thanks!

[JEG2]

I have pushed 1.6.14 with this fix. Thanks all!