Thanks for the review! On Sun, 04 Jul 2021 12:40:59 +0300, Tomi Ollila <tomi.ollila@iki.fi> wrote: > The code looks good to me, just that these "hardcoded" color values gives > me a bit of suspicion... That's true and I do think the colors could eventually be configurable, but right now I don't know how the configuration should work. Should it be a palette, should each header have a configurable color (and should the displayed headers be configurable too) etc. It's easy to make this hugely bloated so I'll avoid going there until I have a clear idea of a good, minimal approach. > the colors look OK when background is dark (black). on light background > (white) color 33 (yellow), color 36 (cyan) and color 32 (green) are > somewhat hard to read (in decreasing hardness)... Most terminals have user-configurable colors so I don't think this is a showstopper. But as I said, yes, it would be best to make the colors configurable. > I did not try to apply this and see how those looks like when displaying > real emails (no setup on this machine for now...) Here's a screenshot: https://i.imgur.com/7dlP7lt.png I'm happy to receive any suggestions but I think it's best to stay with the original 3-bit spec (SGR codes 30-37, or in practice 31-36: red, green, yellow, blue, magenta, cyan) if the values are hardcoded. Those are most widely supported. > One option would be to first send this without color support and then > add *configurable* color support -- I don't know which way is better > but as a rewiever that would be easier to accept... That's an excellent idea! I'll post PATCH v3 later. I don't really want to try real feature detection because some terminals have weird corner cases with their ANSI sequence support. `isatty` is what other CLI tools tend to use as default AFAIK. Hannu _______________________________________________ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-leave@notmuchmail.org
