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.

Sign up for GitHub

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

Jump to bottom

Clean up structure of io/console and avoid stty on Windows. #4590

Merged
merged 1 commit into from
May 8, 2017
Merged

Clean up structure of io/console and avoid stty on Windows. #4590

merged 1 commit into from
May 8, 2017
+342 −279

Conversation

headius
Copy link
Member

@headius headius commented May 5, 2017

This should fix #3989.

I have restructured io/console to be more modular and now force Windows to always use the stubbed version...never the stty version.

Other platforms should work as they did before, but the code is a bit easier to follow now.

@headius headius added this to the JRuby 1.7.27 milestone May 5, 2017
@headius
Clean up structure of io/console and avoid stty on Windows.
43a6732 
Fixes jruby#3989.

* Restructure the different impls of console into their own files.
* Always use stubbed version on Windows.
* Cascade from native to stty to stubbed on other platforms.
@headius headius force-pushed the cleanup_io_console branch from bd5964f to 43a6732 Compare May 5, 2017 20:23
@headius headius mentioned this pull request May 5, 2017
Closed
@headius
Copy link
Member Author

headius commented May 5, 2017

Request review from @kares, @enebo. This is almost structurally identical to the old one, but the old logic was hard to follow and had unusual guards (like the ttymode defined check) that I did not carry over.

I believe the new logic is cleaned and more appropriate:

  1. If Windows, use stubbed always.
  2. If Linux or BSD, attempt to use native; if that fails, continue with (3)
  3. Attempt to use stty. If that fails, fall back to stubbed.

@kares
Copy link
Member

kares commented May 6, 2017

👍 such a cleanup would be good to have on 9K (pretty much all in one file)
although I am not sure how deep its modified compared to the stock version for MRI (due stdlib imports).

@headius
Copy link
Member Author

headius commented May 6, 2017

In MRI, io/console is written entirely in C. Part of this reorg is to get files like linux_console.rb out of the io load namespace. I realize I have added a new namespace with JRuby-specific files, but I think it is isolated pretty well.

I will get this into 9k as well since they're both from the same sources.

@headius headius merged commit 59d305e into jruby:jruby-1_7 May 8, 2017
@headius headius deleted the cleanup_io_console branch May 8, 2017 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
JRuby 1.7.27
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@headius @kares