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

Incorrect call to fflush() inside write_console() #3688

Closed
dimztimz opened this issue Oct 25, 2023 · 1 comment · Fixed by #3689
Closed

Incorrect call to fflush() inside write_console() #3688

dimztimz opened this issue Oct 25, 2023 · 1 comment · Fixed by #3689

Comments

@dimztimz
Copy link
Contributor

In this very recent change 3b7f58a which originates from this PR #3668, the added call to fflush is incorrect and may brake the std::ostream overloads of print(). The intention was to implement the paper https://wg21.link/P2539/ in FMT here.

The correct behavior is to remove fflush from write_console() and call it before calling write_console(), but only from the C stream overloads of print, i.e. print(FILE*,...). For the ostream overloads one should call ostream::flush().

dimztimz added a commit to dimztimz/fmt that referenced this issue Oct 25, 2023
This change correctly implements https://wg21.link/P2539/ for both
C streams and C++ iostreams.

Fixes fmtlib#3688.
dimztimz added a commit to dimztimz/fmt that referenced this issue Oct 25, 2023
This change correctly implements https://wg21.link/P2539/ for both
C streams and C++ iostreams.

Fixes fmtlib#3688.
dimztimz added a commit to dimztimz/fmt that referenced this issue Oct 25, 2023
This change correctly implements https://wg21.link/P2539/ for both
C streams and C++ iostreams.

Fixes fmtlib#3688.
dimztimz added a commit to dimztimz/fmt that referenced this issue Oct 25, 2023
This change correctly implements https://wg21.link/P2539/ for both
C streams and C++ iostreams.

Fixes fmtlib#3688.
@vitaut
Copy link
Contributor

vitaut commented Oct 25, 2023

It is not incorrect, just unnecessary. A PR to remove it would be welcome.

@vitaut vitaut closed this as completed Oct 25, 2023
vitaut pushed a commit that referenced this issue Oct 25, 2023
This change correctly implements https://wg21.link/P2539/ for both
C streams and C++ iostreams.

Fixes #3688.
@vitaut vitaut removed the question label Oct 25, 2023
ckerr pushed a commit to transmission/fmt that referenced this issue Nov 7, 2023
This change correctly implements https://wg21.link/P2539/ for both
C streams and C++ iostreams.

Fixes fmtlib#3688.
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 a pull request may close this issue.

2 participants