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

Allow ostream-redirection of more than 1024 characters #1479

Merged
merged 1 commit into from
Aug 29, 2018

Conversation

Quincunx271
Copy link
Contributor

Fixes #1478

In short, detail::pythonbuf::overflow() had its branch for the return statement backwards, since bool(sync()) is true if and only if sync() fails; sync() returns 0 on success. This fixes it by using sync() == 0 as the condition.

@Quincunx271
Copy link
Contributor Author

The CI errors seem to be a warning about using a deprecated numpy type (matrix), which has nothing to do with this change. All non-numpy tests seem to be passing.

@wjakob
Copy link
Member

wjakob commented Aug 28, 2018

There is something I don't understand about the original implementation, which is somewhat related to the patch here. It looks like sync() can't return any value other than zero -- so what's the purpose of the ternary expression?

@Quincunx271
Copy link
Contributor Author

If someone wrote a class which inherits from pythonbuf, sync() could return a non-zero value.

@wjakob
Copy link
Member

wjakob commented Aug 28, 2018

Ah, gotcha -- it's a virtual function in the C++ standard. This looks good to me then.

@wjakob wjakob merged commit 2cbafb0 into pybind:master Aug 29, 2018
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.

scoped_ostream_redirect only works for the first 1024 characters
2 participants