-
Notifications
You must be signed in to change notification settings - Fork 83
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
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 🙏 |
Looks good 👍 For #659, I think separate pull request is better |
af6d3ff
to
e7855d7
Compare
CI / readline can be passed by setting |
b9f9bbe
to
196d621
Compare
1. Move IO related classes to `lib/reline/io/` directory. 2. Rename `GeneralIO` to `Dumb`. 3. Use IO classes as instances instead of classes.
196d621
to
d786203
Compare
@tompng Thanks for all the suggestions! I applied all of them and all tests are now passing 😄 |
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.
Looks good 👍
(just one change for @in_bracketed_paste
)
Co-authored-by: tomoya ishida <tomoyapenguin@gmail.com>
@tompng Thanks again for the help! |
(ruby/reline#666) * Overhaul IO gate structure 1. Move IO related classes to `lib/reline/io/` directory. 2. Rename `GeneralIO` to `Dumb`. 3. Use IO classes as instances instead of classes. * Update lib/reline/io/ansi.rb Co-authored-by: tomoya ishida <tomoyapenguin@gmail.com> --------- ruby/reline@dc1518e1ac Co-authored-by: tomoya ishida <tomoyapenguin@gmail.com>
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