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

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

Merged
merged 1 commit into from May 8, 2017

Conversation

Projects
None yet
2 participants
@headius
Member

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

Clean up structure of io/console and avoid stty on Windows.
Fixes #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

This comment has been minimized.

Show comment
Hide comment
@headius

headius May 5, 2017

Member

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.
Member

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

This comment has been minimized.

Show comment
Hide comment
@kares

kares May 6, 2017

Member

👍 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).

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

This comment has been minimized.

Show comment
Hide comment
@headius

headius May 6, 2017

Member

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.

Member

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

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details

@headius headius deleted the headius:cleanup_io_console branch May 8, 2017

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