Classes `hljs-attribute` vs `hljs-attr` mismatch

highlightjs / highlight.js

JavaScript syntax highlighter with language auto-detection and zero dependencies.

https://highlightjs.org/

BSD 3-Clause "New" or "Revised" License

23.63k stars 3.58k forks source link

Classes `hljs-attribute` vs `hljs-attr` mismatch #1472

Closed feimosi closed 5 years ago

feimosi commented 7 years ago

highlight.js used to append hljs-attribute class in the past, but now it's hljs-attr. Unfortunately styles are not updated to match that. I've spotted it after migrating from 8.1 to 9.10. See https://github.com/isagalaev/highlight.js/blob/master/src/languages/xml.js#L14 and https://github.com/isagalaev/highlight.js/blob/master/src/styles/tomorrow-night.css#L37

isagalaev commented 7 years ago

There was a huge overhaul of the style rules for 9.0, and the change you're describing is an intentional decision. We now have both attr and attribute with slightly different semantics, and attribute names within XML tags get the "less important" attr which is not highlighted in many styles by choice.

Having said that, if you want to advocate Tomorrow in particular should have a different color for "attr", we should pick a color that doesn't break other usages of this class name in [crmsh fix gcode haml htmlbars ini javascript json nix puppet xml yaml].

feimosi commented 7 years ago

Well, I didn't know that, but still I reported the issue because styling after the update is worse for my HTML snippets.

For comparison, here's how it used to look like: Here's how the new Tomorrow-Night looks like:

If we just change the CSS rule to use hljs-attr instead of hljs-attribute, we can achieve: Note: on this example I also 'fixed' < >

joshgoebel commented 5 years ago

Closing this:

It was an intentional change.
It has received very little discussion (so most must be ok with the change or not care)
Adjusting the CSS is always a fairly easy possibility.

Trying to clear out old issues.