Skip to content

maintaining original sys.stdin #59

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

Merged
merged 1 commit into from
Nov 1, 2018
Merged

maintaining original sys.stdin #59

merged 1 commit into from
Nov 1, 2018

Conversation

kzidane
Copy link
Member

@kzidane kzidane commented Oct 31, 2018

No description provided.

Copy link
Member

@dmalan dmalan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What bug does this fix, just to be clear?

@@ -35,18 +35,21 @@ class Reader:
https://bugs.python.org/issue24402
"""

def __init__(self, f):
self.f = f
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's f here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can/should we rename? Feels a bit magical.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was trying to be consistent with what you have here

self.f = f
. Can be any object (presumably file) passed to the constructor here. We're instantiating a few lines later and passing it sys.stdin, so in the instance that we're using self.f is original sys.stdin.

@kzidane
Copy link
Member Author

kzidane commented Oct 31, 2018

Fixes this bug per the email thread:

screenshot 2018-10-29 14 09 04

@kzidane
Copy link
Member Author

kzidane commented Oct 31, 2018

Which happens when using IDLE on Windows at least.

Copy link
Contributor

@glennholloway glennholloway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prior to this change, get_string was hanging on both macOS and Linux when used under IDLE. This fix resolves that problem. With it, get_string seems to work fine under IDLE on both platforms.

Copy link
Member

@dmalan dmalan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, gotcha, nice fix. Thanks!

@dmalan dmalan merged commit 0c2c6c1 into develop Nov 1, 2018
@dmalan dmalan deleted the idle branch November 1, 2018 02:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants