-
Notifications
You must be signed in to change notification settings - Fork 138
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
Added an "approximate_rgb" function. #30
base: master
Are you sure you want to change the base?
Conversation
This would fix #14, I presume. |
Hmm... just saw the failed tests. Seems it's working on Python 2.7 but not on 2.5 and 2.6. I will need to look into that. |
Tried to install Python 2.5 and 2.6 from source, but I am having trouble making virtualenv work with them... I am giving up for now due to time-constraints :( If I don't forget it, I will look it up later again. |
I'm fully in favor of a nearest-possible-color fallback in case of limited-color terminals, in case it helps. |
I designed a draft proposal in issues #67, please review. It would likely make some use of the given code. I am going to provide some criticism of this PR, please consider it constructive!
So, The rules of why and how these values are clamped/divided/reduced, then matched along a row/col/face, then some magic about indexes should be declared as a code comment in the implementation: so there is no need to express why such values are matched in the test: it should be desk-checkable by the given description of the implementation. I think I see three functions to be compoundly tested:
Currently this is all done in a single function. My proposal in #67 also hopes to make use of variants of these 3 functions to map to a 24-bit colorspace as well, which would require conditionally excluding some of these components. |
I fully agree with your comments. It has been a while since I looked into this code, I don't even remember writing this, and it does not even look like something I had written. Not sure where even it comes from. Right now, I don't get too much time to do some research in this, but I will try to get back to that code as soon as I can and clean things up. I hope I will get to it soon. |
Do we have any change of making this a reality? |
@ssbarnea this is rolling into next release of blessed https://github.com/jquast/blessed/blob/d7c3d52f8804d92cfb35a8286cd88fd46e633fc9/blessed/terminal.py#L670 |
Also covered 256 colour codes with test. Not 100% sure if those test are any good though.
I've stumbled across this issue again. It dropped off my radar again. I will deal with the conflicts and will change the code commend to something objective. |
024fffd
to
2979a15
Compare
Done. Should be better now. |
I've also hunted down the original implementation and added a link (for attribution). |
Also covered 256 colour codes with test. Not 100% sure if those test are
any good though.