search

sharkdp / lscolors

A Rust library and tool to colorize paths using LS_COLORS
Apache License 2.0
265 stars 18 forks source link

Crossterm conversion using incorrect colors #40

Closed meain closed 2 years ago

meain commented 2 years ago

I am still not 100% certain on this and so I thought I would start a discussion around this. I believe we incorrectly converting to the bright variants in crossterm instead of the dark ones. For example 34 should be matched to 4 in crossterm which would be DarkBlue and not Blue.

https://github.com/sharkdp/lscolors/blob/c6e191b91f637058ba932a4f7b065f2e19d8a1ba/src/style.rs#L58-L63

sharkdp commented 2 years ago

Thank you very much. This should be fixed. Ideally, we should have tests for both terminal library backends (that would assert that the output actually matches the correct escape sequene). I don't use the crossterm backend myself, so I hadn't noticed this.

meain commented 2 years ago

As of now crossterm does not seem to support 8/16 color terminals. The minimum you need is 256 colors(related issue). So, even if we are to change it to the dark varients(which we should still do), we will still get different output as ansi-term by default assumes 8 colors. For example, LS_COLORS value of 34 would become 1;34 in ansi-term and 38;5;4 in crossterm. The biggest difference that I have run into with this automatic conversion is that most terminals don't respect "use bright colors for bold text" if we are using 256 color pallete.