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

[#1487] ensure background color is preserved #1865

Merged
merged 2 commits into from
Oct 24, 2024

Conversation

imrichardcole
Copy link
Contributor

@imrichardcole imrichardcole commented Oct 23, 2024

As raised in issue 1487, the color of the background on Windows was assumed to be black. This means that when printing results in, for example, green, you ended up with green on black regardless of the original background color.

This update retains the background colors when printing.

This naturally raises another topic of, does the output color want to be determined by the background color when printing? I.e we would now end up printing green text on a green background if that were the case. Maybe this can be captured in another request if it's desirable.

@imrichardcole
Copy link
Contributor Author

image

@imrichardcole
Copy link
Contributor Author

image

@imrichardcole
Copy link
Contributor Author

image

@dmah42
Copy link
Member

dmah42 commented Oct 23, 2024

clang-format fixes needed but also #1867 probably needed.

@imrichardcole imrichardcole force-pushed the task/fix-console-colours branch from ea30cad to b93d585 Compare October 23, 2024 09:18
@imrichardcole
Copy link
Contributor Author

Sorry, I need to get myself setup locally with clang-tidy so I don't put pressure on these actions prematurely

@dmah42
Copy link
Member

dmah42 commented Oct 23, 2024

Sorry, I need to get myself setup locally with clang-tidy so I don't put pressure on these actions prematurely

i do have it set up locally and i still get it wrong. it's fine, it's what the actions are there for.

@imrichardcole imrichardcole force-pushed the task/fix-console-colours branch from b93d585 to 7fe76d7 Compare October 23, 2024 09:25
dmah42
dmah42 previously approved these changes Oct 23, 2024
dmah42
dmah42 previously approved these changes Oct 23, 2024
@dmah42
Copy link
Member

dmah42 commented Oct 23, 2024

-                                             FOREGROUND_INTENSITY | 
+                                             FOREGROUND_INTENSITY |

i'm struggling to see the difference... is it one more space on the second one?

@imrichardcole
Copy link
Contributor Author

Yes, I'm struggling to get clang-format-lint to be happy.

@dmah42
Copy link
Member

dmah42 commented Oct 24, 2024

took a closer look. on that line above there's an extra space after the |

@dmah42 dmah42 merged commit 4e3f2d8 into google:main Oct 24, 2024
80 checks passed
@dmah42
Copy link
Member

dmah42 commented Oct 24, 2024

thank you!

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.

2 participants