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
Overhaul io gate structure #666
base: master
Are you sure you want to change the base?
Conversation
978f779
to
af6d3ff
Compare
The readline test failures are caused by I'm not sure what causes the windows failure though and I don't have a windows machine. Any help would be appreciated 🙏 |
test/reline/helper.rb
Outdated
@@ -22,29 +22,24 @@ module Reline | |||
class <<self | |||
def test_mode(ansi: false) | |||
@original_iogate = IOGate | |||
remove_const('IOGate') | |||
const_set('IOGate', ansi ? Reline::ANSI : Reline::GeneralIO) | |||
@original_screen_size = @original_iogate.get_screen_size |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this @original_screen_size
and IOGate.set_screen_size(*@original_screen_size)
is not needed because we don't modify @original_iogate
anymore.
(perhaps this is causing windows test failure)
Looks good 👍 For #659, I think separate pull request is better |
af6d3ff
to
e7855d7
Compare
class Reline::Dumb < Reline::IO | ||
RESET_COLOR = '' # Do not send color reset sequence | ||
|
||
def initialize(encoding: Encoding.default_external) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think default value of encoding should be nil, or we need to check RUBY_PLATFORM.
If @encoding
is always set, we can't get into elsif RUBY_PLATFORM =~ /mswin|mingw/
branch in def encoding
and this seems to be the reason CI (irb) is failing. (but I can't reproduce the failing in my local environment...)
CI / readline can be passed by setting |
I think the current IO gate design has a few non-functional issues:
Reline::IOGate
, like in tests, we need to reset it back through redefining the methods, which is pretty hacky.This PR addresses the issues by:
GeneralIO
toDumb
to better reflect its role