jdinhify / vscode-theme-gruvbox

Gruvbox theme for vscode based on https://github.com/morhetz/gruvbox
https://marketplace.visualstudio.com/items?itemName=jdinhlife.gruvbox
MIT License
267 stars 63 forks source link

TS syntax color is very orange after update #85

Open cortlin opened 8 months ago

cortlin commented 8 months ago

It flickers with the correct highlighting when opening a new file, but then resorts to the screenshot below. My rust code looks similar but don't have a screenshot for that.

Screenshot 2023-10-26 at 10 35 03 AM
cortlin commented 8 months ago

Same as #84 , closing

jdinhify commented 8 months ago

Thanks @cortlin

The changes were actually intentional for this one. I notice that the flickering seems to happen because the editor trying to apply semanticHighlighting after the initial load. So if you prefer the previous look and no flickering, you can try disabling semanticHighlighting by putting this in the settings

"editor.semanticHighlighting.enabled": false

Regarding the newest changes, it's improvement that the highlighting of functions & methods are different from others such as class name/builtin types Although I do agree that the orange-ish looks isn't ideal so I just published a new version that reduces the usage of orange #86 (v1.13.0). Please have a try and let me know your feedback 🙏

pm0u commented 8 months ago

Is this actually not a regression? There's no color difference for properties in TS

v 1.13.0

image

v 1.16.0

image

v 1.16.0 - semantic highlighting disabled

image

I appreciate the semantic differentiation of the 2 consts that showed up in 1.13.0 and can deal with variables losing their coloring (variable names used to be blue back in 1.8.0) but all properties losing any highlighting is not great imo.

shouldn't these highlights agree?

image
jdinhify commented 8 months ago

thanks @pm0u

I see the issues. I think making the property colour blue makes senses, will do that shortly

A question about variables colours: Do you think making it blue like before would be better? In the screenshot that has semantic highlighting disabled, there's no distinction between variables & properties.

The reason for the variable changes was that I had a look at the original vim theme and saw that the main foreground colour (black in the light theme) being used a lot, where in this theme there's not much usage for it (almost no usage except for the tokens that aren't coloured), so I follow the original theme and make variables use foreground colours

pm0u commented 8 months ago

hmm - hadn't considered referencing the vim theme as the source of truth, that's a valid point.

I can see the case for using black/fg0 for variable names - it is otherwise used in very few locations.

going to dive into some personal preferences here, but I think the v1.13.0 coloring makes the most sense to me - in that, variables are always black (whether declared or referenced) and properties are always blue (whether being defined or referenced). That said, in my vim theme this does diverge from the original behavior. My gripe was that (at least in this section of code) there was a lot of black that made things hard to distinguish.

jdinhify commented 8 months ago

thanks @pm0u ,

I'm about to push the change to update the property colour to blue (going back to v1.13), that should fix the issue with variables & properties being on the same colour

dark

  "property": "#83a598"

light

  "property": "#076678"

I saw your screenshot in the other issue that you override functions to use yellow. The problem with using yellow is that it's clashing with others such as types, class name (useExtensionAccounts has the same colour with Object/JSX.Element/string/number/etc...)

pm0u commented 8 months ago

@jdinhify - think you may be referring to the other participant here (@alecdwm), but I agree with your points (although, I haven't used the dark mode - I stick to light, I think it would still apply there).

jdinhify commented 8 months ago

lol sorry 🤦 , thanks for pointing it out, I thought Alec replied super fast when I mentioned him in the other issue

the change to properties colour has been published please update when it's available to you & let me know your feedback 🙏

pm0u commented 8 months ago

no worries - initial reaction is that its great! thanks for the fast response. I'll let you know if anything comes up as I test it out more.