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

Make colored print handle UTF-8 #2701

Merged
merged 4 commits into from
Jan 7, 2022
Merged

Make colored print handle UTF-8 #2701

merged 4 commits into from
Jan 7, 2022

Conversation

AlexGuteniev
Copy link
Contributor

@AlexGuteniev AlexGuteniev commented Jan 4, 2022

Fixes #2681

The working flow:

  • With is_utf8() calls detail::print that specifically converts to UTF-16 and calls WriteConsoleW
  • Without is_utf8() calls fwrite_fully

The broken flow:

  • Always call fputs

The fix:

  • With is_utf8() call detail::print
  • Without is_utf8() keep calling fputs, as fwrite_fully is not exported

This is tested on Windows only. On Windows fputs does not do any magic, and color sequences are processed by WriteConsole. I'm not sure if on other platform fputs processes these ANSI escape sequences, or fwrite will also do.

@AlexGuteniev
Copy link
Contributor Author

I actually observe inconsistency that non-colored format throws I/O errors but colored directly calls fputs without checking. I'm not creating the issue yet, maybe it should be fixed within this change.

Copy link
Contributor

@vitaut vitaut left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

I actually observe inconsistency that non-colored format throws I/O errors but colored directly calls fputs without checking. I'm not creating the issue yet, maybe it should be fixed within this change.

Good catch, please add an error check here or in a separate PR.

include/fmt/color.h Outdated Show resolved Hide resolved
@AlexGuteniev AlexGuteniev requested a review from vitaut January 6, 2022 23:57
@vitaut vitaut merged commit 0102101 into fmtlib:master Jan 7, 2022
@AlexGuteniev AlexGuteniev deleted the styles branch January 7, 2022 00:24
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.

UTF8 doesn't work with text styles
2 participants