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

Handle 48-bit #rrrrggggbbbb hashes (fixes rainbow-identifiers-mode) #30

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Nephyrin
Copy link

There's a few different ways to write this (cond), I went for the minimal change here but it could be re-ordered to be a little clearer.

This fixes, specifically, htmlize-ing code with the rainbow identifiers package, which emits these colors.

These aren't valid in HTML so we shouldn't be passing them
through. (color-values) will read these, so just fall through to that
if we encounter one.
@@ -958,16 +958,18 @@ If no rgb.txt file is found, return nil."
;; specifying any color. Hence (htmlize-color-to-rgb nil)
;; returns nil.
)
((string-match "\\`#" color)
((and (string-match "\\`#" color) (= (length color) 7))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we just convert #rrrrggggbbbb to #rrggbb?

I think the idea behind this code is to handle color names like "fuchsia" without calling color-values which (at some point) didn't work when Emacs was run from a TTY. Perhaps the whole logic of parsing rgb.txt can just be removed.

Copy link
Author

@Nephyrin Nephyrin Jun 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have any background on the history there, but sticking the (not (string-match ...) on the rgb.txt condition means only strings that start with # but are not the expected length fall through to color-values, which should handle them (though maybe older emacs had some caveats there?)

As a side-effect, actually, this will cause #111 12-bit colors to go to color-values too, and expand to 24-bit. Maybe that's okay for consistency? Or we could allow lengths of 7-or-4.

I'm happy to tweak this however you think it should go -- a replace-regexp-in-string to just truncate the least significant bits down from the 48-bit ones would work, too.

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.

2 participants