Skip to content
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

Fix newline handling in stdout and stderr #686

Merged
merged 2 commits into from May 28, 2017
Merged

Conversation

@ata2001
Copy link
Contributor

ata2001 commented May 27, 2017

Fixes #658.

def send_to_stdout(self, output):
"""Send unicode string to Repl stdout"""
if not output: return

This comment has been minimized.

@sebastinas

sebastinas May 28, 2017

Contributor

Please add break the line after :.

@@ -1157,9 +1165,16 @@ def send_to_stderr(self, error):
Must be able to handle FmtStrs because interpreter pass in
tracebacks already formatted."""
if not error: return

This comment has been minimized.

@sebastinas

sebastinas May 28, 2017

Contributor

Please add break the line after :.

lines = output.split('\n')
if output == len(output) * '\n':
# If the string consist only of newline characters,
# str.split returns one more empty strings.

This comment has been minimized.

@sebastinas

sebastinas May 28, 2017

Contributor

Since at this point split was already called, I'd feel that if all(len(line) == 0 for line in lines): would be nicer.

This comment has been minimized.

@ata2001

ata2001 May 28, 2017

Author Contributor

What about if all(not line for line in lines):?

if error == len(error) * '\n':
# If the string consist only of newline characters,
# str.split returns one more empty strings.
lines = lines[:-1]

This comment has been minimized.

@sebastinas

sebastinas May 28, 2017

Contributor

Similar to the other splitcomment.

@sebastinas sebastinas merged commit 997117a into bpython:master May 28, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.