search

braver / vscode-solarized

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

Colors different for TypeScript #5

Closed davidchl closed 6 years ago

davidchl commented 6 years ago

I love this theme but when we switched from JavaScript to TypeScript, some of the colors seemed off. Here's an example:

Javascript: image

TypeScript: image

It would be great if the TypeScript colors were the same as the JavaScript colors.

Thanks!

braver commented 6 years ago

This isn’t something the color scheme can do, the syntaxes should be changed.

DavidRGriswold commented 6 years ago

I know you closed this issue, @braver , but I might investigate a bit more, since it seems the default Solarized Light theme does have the same color scheme for JS and TS

DavidRGriswold commented 6 years ago

So it seems this is caused by the default "variable" being blue while the default "storage" is also blue. In other words, what it shows in the typescript screenshot. The code has a special syntax command for javascript "readwrite" variables, which is what is giving the gray color in the first screenshot.

I get the same effect using "Java" syntax, since it also simply uses the default "variable" token rather than the special variable.readwrite one.

I see three possible options to correct this inconsistency:

  1. Replace the "variable" default color with the gray one from the javascript syntax and remove the custom javascript variable syntax. This will result in a all C-like languages - C, Java, Javascript, Typescript - looking like the upper screenshot (nice!) but will also potentially cause weird changes in other syntaxes. I assume that "variable" is currently blue rather than gray to make variable stand out from regular foreground scripts in languages where variables aren't the most common type of word, and this would break that.

  2. Add custom code in the theme for java, C, C++, and Typescript syntaxes, just to make them look like the javascript one in this case. I assume there are other C-like languages - maybe PHP? - where we could do the same. This isn't ideal because it means new C-like languages will look like the bottom screenshot always.

  3. Come up with an entirely new set of colors for "storage" and "variable" that look good in all syntaxes without needing custom code for the javascript readwrite variables (which are really the default variables!)

DavidRGriswold commented 6 years ago

I found a simpler solution that fixes the reported problem but still leaves some flexibility without (likely) breaking other languages.

I changed the token customization for "variable.other.readwrite.js" to "variable.other.readwrite", along with all the other specialized .js tokenizations. The Typescript syntax engine uses the same initial scope, so it now grabs the right colors. I would imagine that if any other syntax engines are also using the same scope, they are doing it intentionally to match the js/ts ones, so now they get to take advantage of our colors too. Java still looks a little weird, but after playing with it it's actually not too bad - the storage and variable commands only are the same color during definitions, after that variables do change color.

PR coming soon. I can approve it myself, but will wait for your go-ahead @braver.

braver commented 6 years ago

@DavidRGriswold I’m not in range of a computer the coming days but what you’re saying makes a lot of sense to me. The trick is in finding something that works and looks nice without introducing too many language specific tweaks. I’m looking forward to seeing what you come up with 👍🏻