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

Crossterm Reset != Black #2

Open
joshka opened this issue Aug 17, 2023 · 3 comments
Open

Crossterm Reset != Black #2

joshka opened this issue Aug 17, 2023 · 3 comments

Comments

@joshka
Copy link

joshka commented Aug 17, 2023

Hi there. I was taking a read of the source and noticed that crossterm::Color::Reset gets translated to Black.

CC::Reset => 0.into(),

This is correct only for terminal themes that have their background color and black set to the same value (e.g. all light themes are excluded by this).

Visually, here's the output of the color example from Ratatui (note the color names are Ratatui names, not crossterm ones):

colors

I'm not sure what the right design for this should be in coolor as it seems like it would be use case dependent, but perhaps adding an explicit Reset variant to the color enum. Do you think it would be worth marking this as non-exhaustive to allow adding other color models?

@Canop
Copy link
Owner

Canop commented Aug 17, 2023

The thing is, coolor deals with colors and Reset isn't really a color.
Reset should probably be normally dealt with before calling into. Maybe with a function returning an Option<Color>.

@joshka
Copy link
Author

joshka commented Aug 17, 2023

Yeah - that was my sort of thinking too, but I wasn't sure what sorts of use cases would lead to using that method.

My interest in this particular crate was mainly piqued by wondering whether there is value to adding conversions for Ratatui colors.

@Canop
Copy link
Owner

Canop commented Aug 17, 2023

My interest in this particular crate was mainly piqued by wondering whether there is value to adding conversions for Ratatui colors.

I don't know if that would be useful but it would be clean and straightforward from what I see of ratatui Color, so go ahead if you want.

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

No branches or pull requests

2 participants