braver / vscode-solarized

Solarized for VS Code
MIT License
32 stars 10 forks source link

Punctuation color isn't consistent between themes #7

Open viqofirdaus opened 6 years ago

viqofirdaus commented 6 years ago

Hi, I'm wondering if this is intended? Punctuation like curly braces in light theme are in black color different with the text one, while in dark theme both are in the same color

image

braver commented 6 years ago

That’s probably not by design. 🧐

DavidRGriswold commented 5 years ago

This is the same issue as #9 and will be fixed shortly.

yingbo commented 5 years ago

Why the issue was closed without a fix? It should be closed right after "will be fixed shortly".

DavidRGriswold commented 5 years ago

The issue has been fixed. Though in the process I introduced another bug, naturally, which I will be reporting and working on fixing shortly.

DavidRGriswold commented 5 years ago

Okay, I am reopening this issue. Though part of the problem was indeed fixed with the bug in #9, part of it was not. Specifically, it seems like the two themes do, by design, have some color differences around comments and punctuation.

Light theme (this is the original version, not the themed workbench version) image

Dark Theme (original version) image

Notice that the punctuation and comments almost switch places in terms of lightness/darkness. I am NOT super-familiar with the theory behind the solarized color scheme (I just like the general palette enough to help with this project) so I can't really speak to this; @braver is this intentional? Does it match the sublime project?

DavidRGriswold commented 5 years ago

However, if the original issue was simply that the text and punctuation were different colors FROM EACH OTHER, then that should be fixed in the next bugfix.

DavidRGriswold commented 5 years ago

Hmm, more differences. In javascript file, in the light version punctuation and variables are the same color image

While in the dark version, punctuation and COMMENTS are the same color image

Again, not sure if this is by design?

braver commented 5 years ago

No, that’s not by design. It was probably caused by me working on the light theme and then not completely updating the dark theme, or the other way around. The color schemes for Sublime are a good reference. Since Sublime allows the usage of variables I was able to find and resolve the inconsistencies there. The current version should be a good guide for how it should be: https://github.com/braver/Solarized, see also this commit: https://github.com/braver/Solarized/commit/dbc409c85752ce99123934f6312f4c5b906dbcbb

DavidRGriswold commented 5 years ago

Hmm. I am looking at your sublime theme and figuring out the best way to make everything consistent to the VS Code theme. I think I'm going to implement my own non-high-tech way of keeping track of variables, so that if you add some tokens to the sublime version I can easily insert them in the VS Code one.

I did find one seeming inconsistency in your sublime theme - in the light theme highlight and invisibles are the same color (base1) but in the dark one they are different colors (base01 for invisibles, base1 for highlight). My guess is that the highlight in light themes should be base01 instead.

DavidRGriswold commented 5 years ago

Okay, i got a little obsessed. I just pushed a whole new branch called "testing-sublime-version" that pulled your full sublime tokens into both of the existing themes, and also created a new workbench theme that is much more solarized-pure and allows for true mirroring of the light/dark as solarized theoretically intends, even in the workbench. It is CERTAINLY not ready for release, as it has not been tested thoroughly, so I'm not sure what the best next steps are here.

braver commented 5 years ago

Sweet! Good catch for that inconsistency too. I should throw them in a compare again and check. I’m also really intrigued by your PR, so I guess I need to fire up VS Code again and check.