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

xterm-256color support (proof-of-concept) #46

Closed
wants to merge 2 commits into from

Conversation

jscheid
Copy link

@jscheid jscheid commented Oct 28, 2019

Terminal.app doesn't support 24-bit color, but it does support 256 colors which can make for a decent approximation, see screenshots below.

This needs more polish, for which I actually might not have time myself, but I figured someone might find it useful already as it is.

Before:
Screenshot 2019-10-28 at 21 43 36

After:
Screenshot 2019-10-28 at 22 22 09

@dandavison
Copy link
Owner

This is great @jscheid, thanks very much for working on it! I think it definitely makes sense to get this functionality into delta.

One question I have is, it looks like bat does something like this. How does what you're doing here compare to what bat is using, i.e. https://docs.rs/ansi_colours/1.0.1/ansi_colours/fn.ansi256_from_rgb.html ?

I'm no expert on the environment variables concerned. I see you're inspecting TERM, whereas bat seems to be looking at COLORTERM (also documented in bat's README). I'll have to read up on them.

In any case, I'd definitely like delta to behave nicely when users don't have 24 bit color support so thanks very much for getting it under way!

@jscheid
Copy link
Author

jscheid commented Oct 29, 2019

Ah, I was looking for an existing solution like what ansi_colours provides, I wonder why I didn't find it. It probably makes sense for you to go with that instead, if only because the colorspace crate is unpublished (and is APL, not sure if that's an issue for you.)

To answer your question: I believe my solution provides a more accurate approximation because it uses Delta E instead of operating in RGB color space, and takes gamma into account. It comes at the expense of being more compute intensive, however that wouldn't matter in an application like yours where there's a relatively small number of colors, since you can easily cache the results.

I wouldn't worry about accuracy too much though, going with ansi_colour seems like the pragmatic choice here unless it produces wildly different results. Feel free to close this PR.

You're right about COLORTERM, I was just about to fix that but since it looks like you're going a different route anyway I'll leave it as is.

Thanks for a very useful tool btw, cheers!

@dandavison
Copy link
Owner

Thanks very much @jscheid for getting the work started here. I've merged #111 which adds support for 256-color terminal environments and should serve to replace this PR.

@dandavison dandavison closed this Mar 1, 2020
@jscheid
Copy link
Author

jscheid commented Mar 2, 2020

Awesome, thanks!

@jscheid jscheid deleted the xterm-256color branch March 2, 2020 02:49
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.

None yet

2 participants