- Notifications
You must be signed in to change notification settings - Fork5.4k
When reading from stdin, put a wrapper around the IO object#13944
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Open
tenderlove wants to merge2 commits intoruby:masterChoose a base branch fromtenderlove:eof-wip
base:master
Could not load branches
Branch not found:{{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline, and old review comments may become outdated.
+60 −11
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
Checking the return value of `fgets` isn't good enough to determinewhether the input stream has reached EOF or not. `fgets` could have runin to an error and returned NULL. Additionally, we can't just check thebuffer contents for `\n` because `fgets` could have read the maximumbuffer size (which may not have ended with a `\n`).I want to use this callback to implement a fix for:https://bugs.ruby-lang.org/issues/21188
The purpose of this commit is to fix Bug #21188. We need to detect whenstdin has run in to an EOF case. Unfortunately we can't _call_ the eoffunction on IO because it will block.Here is a short script to demonstrate the issue:```rubyx = STDIN.getsputs xputs x.eof?```If you run the script, then type some characters (but _NOT_ a newline),then hit Ctrl-D twice, it will print the input string. Unfortunately,calling `eof?` will try to read from STDIN again causing us to need a3rd Ctrl-D to exit the program.Before introducing the EOF callback to Prism, the input loop lookedkind of like this:```rubyloop do str = STDIN.gets process(str) if str.nil? p :DONE endend```Which required 3 Ctrl-D to exit. If we naively changed it to somethinglike this:```rubyloop do str = STDIN.gets process(str) if str.eof? p :DONE endend```It would still require 3 Ctrl-D because `eof?` would block. In thispatch, we're wrapping the IO object, checking the buffer for a newlineand length, and then using that to simulate a non-blocking eof? method.[Bug #21188]
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The purpose of this commit is to fix Bug #21188. We need to detect when
stdin has run in to an EOF case. Unfortunately we can'tcall the eof
function on IO because it will block.
Here is a short script to demonstrate the issue:
If you run the script, then type some characters (butNOT a newline),
then hit Ctrl-D twice, it will print the input string. Unfortunately,
calling
eof?
will try to read from STDIN again causing us to need a3rd Ctrl-D to exit the program.
Before introducing the EOF callback to Prism, the input loop looked
kind of like this:
Which required 3 Ctrl-D to exit. If we naively changed it to something
like this:
It would still require 3 Ctrl-D because
eof?
would block. In thispatch, we're wrapping the IO object, checking the buffer for a newline
and length, and then using that to simulate a non-blocking eof? method.
[Bug #21188]