-
Notifications
You must be signed in to change notification settings - Fork 36
Colors #53
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
base: devel
Are you sure you want to change the base?
Colors #53
Conversation
Some things don't work as expected. For instance, when the terminal asks for black_red, the *background* is "red" and the *foreground* is "black". It should be the other way around. I don't understand the cause of this at the moment.
Conflicts: TerminalView.py
|
There is a bug/feature of That's why the theme background is |
When the result of converting the number to a hex value starts with a 0, then that starting digit wasn't recorded in the string. It is now.
convert_color_scheme.py
Outdated
|
|
||
| def next_color(color_text): | ||
| """Given a color string "#xxxxxy", returns its next color "#xxxxx{y+1}".""" | ||
| hex_value = int(color_text[1:], 16) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah...would be more robust to do [1:7] to ignore the alpha value.
|
What is the expected result for the default color? Isn't it showing white color now? |
|
|
||
| def distance2(a, b): | ||
| """Compute the squared distance between two 3D vectors.""" | ||
| return norm2((a[0] - b[0], a[1] - b[1], a[2] - b[2])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This Euclidean distance is very bad for yellow color.
It picks

rather than

for the yellow color.
We may need https://pypi.python.org/pypi/colormath/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I noticed this, I'll play around with other metrics 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have been playing with https://gist.github.com/randy3k/000b04de620c306988835324db09fa9b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does #ffcc66 get assigned to? Green/cyan?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, instead of generating the tmTheme file from user file. I guess we should parse the user scheme and save the settings in TerminalView.sublime-settings
something like
'terminal_color_0' : '#000000',
'terminal_color_1' : '#ff8787',
'terminal_color_2' : '#87d75f',
'terminal_color_3' : '#ffd75f',
'terminal_color_4' : '#87afff',
'terminal_color_5' : '#af87ff',
'terminal_color_6' : '#d7ff87',
'terminal_color_7' : '#bfbfbf',
'terminal_color_8' : '#000000',
'terminal_color_9' : '#ff8787',
'terminal_color_10' : '#87d75f',
'terminal_color_11' : '#ffd75f',
'terminal_color_12' : '#87afff',
'terminal_color_13' : '#af87ff',
'terminal_color_14' : '#d7ff87',
'terminal_color_15' : '#bfbfbf',
and every time when TerminalView is launched, the hidden-tmTheme will be created automatically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It also allows users to modify their colors easily.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course, we don't want to do it for every terminal view, may be only for the first terminal view after Sublime Text restarts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I set the color reference for yellow to (255/255, 200/255, 0), it does choose the yellowish color. From some experimentation I noticed that the cie94 metric has some love/hate relationship with the green component. If you lower the green component in the reference value for yellow, there's almost no perceptual difference but the distances to things that look yellowish become much, much smaller.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just checked out the cie94, good job 👍
There's also a preliminary implementation of the CIELAB 2000 metric, but it looks like it gives some wrong results for some colors, so it probably has bugs. This commit uses the 1994 version. Also, cie94 really loves the green component. This is why the reference value for the yellow color is changed from (1,1,0) to (1,200/255,0). Empirically better results.
|
How do think the idea of generating settings like and applying them to TerminalView in run_time? It will give much more controls to users. |
|
I think that's okay, but I worry for the startup time. Would that be noticeable? |
|
If we only do it once until Sublime Text restarts, it should okay. |
|
Very interesting - while personally I like the black/white scheme it is definitely nice that it can adapt to user color scheme. I'll just wait a bit with pulling this until we have 0.5 out then we can look into including this for 0.6 👍 |
|
Yeah, I'm aiming for 0.6 for this PR. |
|
A few notes after testing this: I ran into a bit of trouble since I don't have any dark blue color in my color scheme (I think). My light blue was used for cyan and magenta was used for blue (thus magenta was not visible): I still like this approach but I'm worried about it giving users an inconsistent experience. Maybe TerminalView should ship with a default color scheme similar to what we have now + a few alternatives that can be selected through settings (like maybe a light version with white background). Among the alternatives this could be one of them - a sort of auto color scheme. Another concern I have if we were to use this as default color scheme is that it doesn't set TerminalView apart from the rest of ST3. I think it's a good thing to have a somewhat different color scheme (atleast as default) since it signals to the user that they are not in a regular ST3 view (different keybindings etc.). What do you think ? |
|
I'm okay with this being an optional setting in the settings. Preferences differ from person to person and I like my terminal to look like any other view. For the magenta being black, I think the function extracted only 5 colors and added a black color to get six (see here). Furthermore it then assigned "magenta-ish" to the blue reference color because blue just so happens to come before magenta. The magenta-ish color should have been assigned to the magenta reference. I think that's a bad idea now and I'd like to change the algorithm so that it can better handle schemes with few colors. Can you put your color scheme in a gist so I can experiment with it? |
|
Color scheme: https://gist.github.com/Wramberg/4c3701579a5c18b581f3f1df4fb521b1 |
|
Just to give an update: cie2000 works now, but it gives basically the same results as cie1994. Also, your color scheme has 11 colors, so the problem is not that it has too few colors. The problem is actually that while it looks like the magenta color is black, it's actually fairly close (a distance of about 30 units, 15 units is "very close"). The problem is that it is also happens to be very close to the background color. My idea is to check the distance to the background color, and if it's "too close" to the background, change the lightness somehow to account for that. For this, I need to be able to convert from |
|
Thank you for the feature. |
|
Hi @johanson, glad you like it. I never got around to finishing this, because it needs a few things still:
Like @Wramberg I'm occupied with work these days, I'll see if I can find the time to pick this up some day again. |









This isn't done yet as it has some bugs. Pretty pictures:
Derived from "Mariana" scheme
Derived from "Blue Thunder" scheme
Derived from "Breakers" scheme
I've been fudging with getting the foreground and background colors switched because as you can see, for some reason it selects the background while it should select the foreground.