-
Notifications
You must be signed in to change notification settings - Fork 281
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
Improve color detection across platforms #885
Conversation
7bbdbd9
to
b8ae856
Compare
Contains fix for #883 to unblock testing. Tested in Windows Terminal with bash (WSL) and pwsh.exe (WT uses conpty) and cmd.exe (which uses conhost). |
can you rebase now main is stable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I'd suggest adding a small amount of details to the commit message body instead of just linking to the issue as this makes it easier for people to review what's changed in a release.
Something like:
`style::available_color_count()` now:
- checks support_ansi on windows
- uses COLORTERM environment variable with fallback to TERM
- supports 24bit / truecolor (reported as u16::MAX)
Would it be worthwhile to add unit tests for this? Perhaps using https://crates.io/crates/temp-env?
Fixes crossterm-rs#882 by improving `style::available_color_count()`: - Checks ANSI support on Windows. - Uses the COLORTERM environment variable and falls back to the TERM environment variable. - Supports "xterm-24bit" and "truecolor" values which return `u16::MAX`.
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice thanks for the PR
Fixes #882 by improving
style::available_color_count()
:u16::MAX
.