Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Theme Maint] Write up on changes for theme maintainers #3134

Closed
joshgoebel opened this issue Apr 13, 2021 · 13 comments
Closed

[Theme Maint] Write up on changes for theme maintainers #3134

joshgoebel opened this issue Apr 13, 2021 · 13 comments
Labels
autoclose Flag things to future autoclose. theme
Projects

Comments

@joshgoebel
Copy link
Member

joshgoebel commented Apr 13, 2021

I'm going to use this issue to track items that theme maintainers may want to keep in mind when updating their themes for v11 and beyond. Each of the below items should be expanded into a brief explanation.

Overall Intention

Please skim #2500 and #2521 if you have time. The idea is higher fidelity highlighting and more nuanced highlighted (when possible). Often there are things we could easily highlight but we have chosen not to in the past (operators, punctuation, properties, function dispatch, etc)... we'd like to start highlighting those things going forward when it makes sense to do so.

We're also open to feedback on these things as there is still wiggle room in the v11 window on how some of these things will land. (particularly exactly how the new tiered scopes shake out)

Newer scopes

We've recently added:

  • property
  • punctuation
  • operator
  • variable.language
  • title.function
  • title.class
  • title.class.inherited
  • char.escape
// > is an operator
// ( and ) are punctuation
// player, score, and enemy are properties
if (state.player.score > state.enemy.score) {
  // ...
}

You can see GitHub itself highlighting the operator and properties.

Please see our CSS Class reference. This is part of our higher fidelity goals. A theme that highlights these items should feel more "complete". If your theme purposely choses to opt-out you should provide an empty rule so our theme checker knows you purposely don't want to highlight these. Example:

.hljs-property {} /* do not highlight */

Do not worry about any wasted space - these things will be minified/stripped at build time.

New tiered scopes

To add more nuance (#2521) we have added tiered scopes. For things that are pure renamings the core team can handle those... such as say if we decided to rename meta-keyword to meta.keyword. But sometimes the changes are a bit broader. This allows for more nuance in parsing (and possibly also more nuance in coloring).

Instead of just a string in the future we might easily have:

  • string.unquoted
  • string.quoted
  • string.quoted.triple

If you've worked with TextMate grammars this should be immediately familiar.

A theme can provide coloring for just string at the top-level or it could get as nuanced as it wants. Note: It will of course take time for grammars to support this additional nuance and there will be some discovery on how much nuance is good to have. This is why I say themes may require some degree of "maintenance" as time goes on.

Deprecation of class and function for indicating definitions

According to our docs these are support to be used fo class and function definitions... but they have been used (and abused) rather inconsistently so they are being deprecated in core.

  • Sometimes they would cover the definition only (the idea)
  • Sometimes they wrap a whole function (we generally want to avoid this, it results in complex grammars)
  • Sometimes they are used to signify function name

This can result in some themes & grammar combos looking funny and large swaths of unintended highlighting. Going forward it's recommended that class and function simply not be highlighted at the top-level. And the recommendation for grammars is that they not emit these top-level scopes at all.

.hljs-class {} /* do not highlight */
.hljs-function {} /* do not highlight */

This should be fine for v10 support as well. Grammars using function to indicate a function name are considered broken and should instead by updated.

Deprecation of class > title and function > title for titles

We're trying to simplify grammars where possible to be flatter... where-as before you may have had a nested scope... now you would instead have a flatter tree but with parts of the match have tiered scopes. Often then can significantly reduce the complexity of a grammar, but with exactly or almost the same fidelity of highlighting.

Before (note the "class" class wrapping the whole start of the class):

<span class="hljs-class">
    <span class="hljs-keyword">class</span>
    <span class="hljs-title">Person</span>
</span> {
  // ...
}

Which you might have targeted with the selector .hljs-class .hljs-title.

In the future (no wrapper):

<span class="hljs-keyword">class</span>
<span class="hljs-title class_">Person</span> {

Which you would target with a .hljs-title.class_ selector.

Supporting v10 and v11 simultaneously.

Generally that simply means supporting old and new scope selectors. This is desirable for the immediate future, perhaps until v12.

/* Title of a function */
.hljs-function .hljs-title { } /* Highlight.js v10 */
.hljs-title.function_ { } /* Highlight.js v11 */
.hljs-title.function_.call__ { } /* Highlight.js v11 */

Note the single underscore _ in function_ and the double in call__. Please see our CSS Class reference for documentation on how tiered scopes work.

Adding metadata.

Theme meta-data should lead the CSS and look something like:

/*!
   Theme: Name of theme
   Description: Optional description of the theme
   Author: Name <email@server.com>
   Maintainer: @githubhandle
   Website: [canonical URL of theme]
   License: [if anything other than BSD-3-clause]
   Updated: [when the theme was last reviewed/updated]

  Other general unstructured comments may follow here.
*/

At a minimum the following should be provided:

  • Theme:
  • Author: if known
  • Maintainer:
  • Website: (if there is a canonical source of the theme elsewhere)
  • Updated:

Sample:

/*!
   Theme: An Old Hope – Star Wars Syntax
   Author: (c) Gustavo Costa <gusbemacbe@gmail.com>
   Maintainer: @gusbemacbe
   Updated: 2021-03-22

   Original theme - Ocean Dark Theme – by https://github.com/gavsiu
   Based on Jesse Leite's Atom syntax theme 'An Old Hope'
   https://github.com/JesseLeite/an-old-hope-syntax-atom
 */

The new ./tools/checkTheme utility.

Our new theme checked can tell you what your theme might be missing. You simply pass the filename of a theme to it. This tool is brand new and likely to improve with time.

Example:

# ./tools/checkTheme.js src/styles/dark.css
Missing selector .hljs-tag
<snip>
Theme does not fully support Templates/HTML/XML, etc..

Missing selector .hljs-number
<snip>
Theme does not fully support program code.
@joshgoebel joshgoebel added language bug help welcome Could use help from community theme and removed bug help welcome Could use help from community language labels Apr 13, 2021
@taufik-nurrohman
Copy link
Member

Would like to update my theme once the default theme is updated. I want to look around.

@joshgoebel
Copy link
Member Author

joshgoebel commented Apr 13, 2021

Our default theme is pretty boring and doesn't really change much. I did just add operator and punctuation to it and some purposeful "opt-outs" to make the new theme checker happy.

@joshgoebel joshgoebel changed the title [Theme Maint] Issue for theme maintainers [Theme Maint] Write up on changes for theme maintainers Apr 13, 2021
@joshgoebel joshgoebel added this to In Progress in Version 11 Apr 14, 2021
@joshgoebel
Copy link
Member Author

I'll probably ping everyone after #3159 is merged and our first v11 beta is released.

@Hirse
Copy link
Contributor

Hirse commented Apr 24, 2021

I really like the formatted metadata. There are some suggestions for it:

  1. Should we include whether a theme is considered "light", "dark" (or maybe "high contrast")?
    1. Relatedly, should we include what accessibility standard (i.e. AA or AAA) a theme is targeting, so future contributors don't regress it?
    2. Is there a way to match "light" and "dark" variants of a theme in the meta-data (apart from string matching on the name)?
  2. Personally, I think it would look nicer to have block comments prefix each line with *:
    /*!
     * Theme: Name of theme
     * Description: Optional description of the theme
     * Author: Name <email@server.com>
     * Maintainer: @githubhandle
     * Website: [canonical URL of theme]
     * License: [if anything other than BSD-3-clause]
     * Updated: [when the theme was last reviewed/updated]
     *
     * Other general unstructured comments may follow here.
     */
  3. Having the "Updated" information is tricky as lengthy PRs require updating the date over and over again.
    I would prefer adding the version number on build time and relying on git for exact date information.

@joshgoebel
Copy link
Member Author

Having the "Updated" information is tricky as lengthy PRs require updating the date over and over again.

I'm not sure what you mean. When the theme maintainer makes changes, they bump it. Apart from that it requires no updates.

relying on git for exact date information.

Git only tells me when the FILE was last changed, not when the theme itself was last changed. Many of themes have seen tons of commits, none related to THEME changes itself but rather just CSS maintenance, etc. I'm open to a better name for this.

I think it would look nicer

Looks like clutter to me - perhaps I'm just used to the existing standard from all our language files which use undecorated comments.

Should we include whether a theme is considered "light", "dark" (or maybe "high contrast")?

I'm not opposed. I think for me maintainer and the name are the most important things here.

@joshgoebel
Copy link
Member Author

CC @highlightjs/theme-maintainers

Pinging this now as v11 is getting close to release. If any of you would like to improve your themes with support for all the new things, now is your chance. :)

I should have already taken care of the more boring "busywork" stuff for you... ie, supporting both .function .title and title.function... so mostly you'd want to focus on adding meta-data, taking advantage of any of the newer classes, using the new theme check utility, etc.

If you have any questions feel free to ask!

@gusbemacbe
Copy link
Contributor

Are .function .title and title.function new elements in the CSS for highlighting the code?

@joshgoebel
Copy link
Member Author

See Deprecation of class > title and function > title for titles section for details. The only thing that is new is the scope title.function itself...

@gusbemacbe
Copy link
Contributor

So, only is title.function the new element in the CSS file?

@joshgoebel
Copy link
Member Author

joshgoebel commented May 14, 2021

I'm not sure I understand what you are asking? title.function is a scope... scopes are encoded to CSS as shown above (and as documented in css-classes-reference.rst.

/* BEFORE: Highlight.js v10 */
.hljs-function .hljs-title { } 

/* AFTER: Highlight.js v11 */
.hljs-title.function_ { } 
.hljs-title.function_.call__ { } 

But as already mentioned I believe I've taken care of this boing "maintenance" stuff already myself in many cases.

@Hirse
Copy link
Contributor

Hirse commented May 16, 2021

@joshgoebel Your (very helpful) tools/checkTheme-utility is complaining about missing hljs-meta-keyword selectors, but as I understand those have been intentionally removed in #3167. Is the utility not up to date?

@joshgoebel
Copy link
Member Author

Is the utility not up to date?

Correct. :)

@joshgoebel
Copy link
Member Author

@Hirse See 9f52d99

@joshgoebel joshgoebel added the autoclose Flag things to future autoclose. label Jun 22, 2021
Version 11 automation moved this from In Progress to Done Jul 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autoclose Flag things to future autoclose. theme
Projects
Development

No branches or pull requests

4 participants