PrismJS / prism-themes

A wider selection of Prism themes
MIT License
1.4k stars 504 forks source link

Add plugin overrides for One Dark and One Light #134

Closed hoonweiting closed 3 years ago

hoonweiting commented 3 years ago

Updating One Dark and One Light with plugin overrides!

Currently a work in progress! I noticed an issue filed for a new release, hence the draft as a head's up. Hopefully I'll be done in the next few hours.

RunDevelopment commented 3 years ago

Thank you for making this @hoonweiting!

May I ask which plugins you intend to improve?

Hopefully I'll be done in the next few hours.

Don't feel pressured and take your time. The release can wait for another day (or week).

hoonweiting commented 3 years ago

I'm looking at Show Invisibles, Toolbar, Line Highlight, Line Numbers, Diff Highlight, Command Line, Match Braces, and Previewers. I might drop some of them though! I'll see how it goes.

Thank you for your support :)

RunDevelopment commented 3 years ago

Wow. That's pretty much all of them. Thank you!

If you want to prioritize certain plugins, let me tell you that Line Numbers, Line Highlight, and Toolbar are the most popular ones in that order.

hoonweiting commented 3 years ago

Ah that's great to know, thank you! :)

hoonweiting commented 3 years ago

I'm about done, but to be certain I want to double check again tomorrow before submitting the PR, it's almost 4am here oops 💤

hoonweiting commented 3 years ago

I think I'm done with the plugin overrides! :)

hoonweiting commented 3 years ago

I have increased the selector specificity of most of them by at least (0, 0, 1). (The only one that didn't get increased does not override, and instead just sets a new property.) Maybe repeating a class would have been easier for maintenance though, hmm, not sure if I should change it.

RunDevelopment commented 3 years ago

The only one that didn't get increased does not override, and instead just sets a new property.

Plugins may (need to) add additional properties in the future. So increasing the specificity for all plugin overrides is probably a good idea.

Maybe repeating a class would have been easier for maintenance though, hmm, not sure if I should change it.

I think repeating a class would be better.

I know that it looks silly but that is actually a good thing. It makes it clear that there is something special about that selector. With your current approach, it's not possible to see which parts of the selector are necessary for selecting the element and which are there to increase the specificity.

It also has the advantage that there are no new requirements.

(There's also the very minor advantage that repeating classes is easy for gzip to compress.)

hoonweiting commented 3 years ago

Right, gotcha! I'll work on it in a bit.

RunDevelopment commented 3 years ago

Thank you very much for contributing @hoonweiting!

I'll make a new release tomorrow or the day after.

hoonweiting commented 3 years ago

Alright! I'll look into the plugin overrides for the other existing themes as well, shouldn't take too long since I've already ironed out a sample of sorts.