Allow className to be configured via functions

[izaakschroeder]

Previously hljs had fixed rules for injecting classes into markup – no longer! You can now control exactly what classes get generated based on language, mode or keyword. This functionality is useful if you use CSS modules and do not wish to pollute the global CSS namespace. It also means that classPrefix isn't strictly necessary anymore – you can generate the names however you like.

[isagalaev]

Hello Izaak! While I can certainly see the appeal of this idea, this is something that we deliberately don't want to do. The reason for this is explained in #348, which was a large refactoring we did recently to enforce a limited set of class names. And this how highlight.js is going to live from now on.

[izaakschroeder]

@isagalaev I think this still fits in alright? My aim is mostly not to let language authors pick whatever names they want, but to allow library consumers to transform the local set of available names. (For use in CSS modules / avoiding collisions, etc.)

[izaakschroeder]

Happy to consider other strategies for making CSS modules work 😄 this seemed like the least intrusive.

[izaakschroeder]

I'd be open to having a transformClassName if that sounds safer, where it just maps an existing class name to some new class name; the current strategy just seemed more general/flexible.

[izaakschroeder]

onmessage = ({data: {code, language, styles}}) => {
  hljs.configure({
    tabReplace: '  ',
    className: {
      default: () => null,
      language: () => null,
      mode: (mode) => {
        console.log('GOT MODE', mode);
        return styles[mode.className];
      },
      keyword: (keyword) => {
        console.log('GOT KEYWORD', keyword);
        return null;
      },
    },
  });
  require('bundle!highlight.js/build/node/lib/languages/' + language)((data) => {
    if (!hljs.getLanguage(language)) {
      hljs.registerLanguage(language, data);
    }
    const result = hljs.highlight(language, code, true);
    postMessage(result);
  });
};

is what I am using now where styles is generated via CSS modules + webpack:

import styles from './theme.css';

As you can see the majority requirement just comes from being able to do a lookup. 😄

[isagalaev]

Ah, I've got your motivation this time :-) I'll have to think about it a bit more. It may be better to first refactor the library to separate tree building from rendering (something we're about to do in #1086) , and then let users to customize the HTML renderer in this way.

[izaakschroeder]

I am totally happy with having a totally customizable renderer which sounds great! 😄 I think this is a reasonable stop-gap solution in the interim though? It doesn't break anything and keeps all existing behaviour as-is?

[izaakschroeder]

I'm also super open and happy to making the API a little nicer.

[isagalaev]

As a stop-gap, it's simpler to just keep it in your fork in whatever way you most like it. I wouldn't want to first merge it and then have to immediately rewrite it heavily during that refactoring. Our core is hairy enough, no need to make it more complex :-)

[izaakschroeder]

Complexity reduced! It's actually pretty painful to keep externals in npm due to needing funny postinstall scripts and other such things to ensure source dependencies get built properly. 😭

The new code is now much closer to a simple class name map.

[izaakschroeder]

À la:

onmessage = ({data: {code, language, styles}}) => {
  hljs.configure({
    tabReplace: '  ',
    className: (item) => styles[item],
  });
  require('bundle!highlight.js/build/node/lib/languages/' + language)((data) => {
    if (!hljs.getLanguage(language)) {
      hljs.registerLanguage(language, data);
    }
    const result = hljs.highlight(language, code, true);
    postMessage(result);
  });
};

[izaakschroeder]

Any other thoughts? 😄

[isagalaev]

I still haven't got around this issue. I can only dedicate about one day a week to highlight.js these days, and some weeks don't even get that. (My suggestion to use a fork in the mean time is very practical, the project simply can't change very fast.)

[isagalaev]

Back to the issue! I've been thinking a lot about all the little things around this feature and I do find it useful, as there were a few requests about having different styling for different snippets on one page. Here are my current thoughts on the actual design:

• options.classFunc is a configurable callback returning a class name based on some context, which is by default just a base class name.
• By default options.classFunc should be function(className) {return options.classPrefix + className}. Having this default should allow us not to drop conditional logic in buildSpan and simply call the function.
• The distinction between language name classes and other classes is irrelevant by now and we can drop it (we may even drop generating language-specific <span>s in the future altogether).
• The distinction between keywords and modes should not concern style authors either.

@izaakschroeder do you think this would work with your use case with CSS modules (I think it should). If yes, do you think you could take it on and re-implement it this way?

[isagalaev]

Have to close it due to inactivity and the lack of bandwidth from my side to work on it.

[pa-nam]

@izaakschroeder Totally agree with you about customizing the className, especially with data like keyword and mode. Having this feature would be a lot of nicer. Will take a deeper look at your pull.