Empty language cannot gain hljs plaintext classes

[unphased]

Hi again.

import { Marked } from 'marked';
import { markedHighlight } from "marked-highlight";
import hljs from 'highlight.js';

const marked = new Marked(
  markedHighlight({
    langPrefix: 'hljs language-',
    highlight(code, lang, info) {
      const language = hljs.getLanguage(lang) ? lang : 'plaintext';
      return hljs.highlight(code, { language }).value;
    }
  })
)

console.log(marked.parse(`
\`\`\`
plaintext
<
&
>
\`\`\`

\`\`\`blah
blah lang
<
&
>
\`\`\``));

<pre><code>plaintext
&lt;
&amp;
&gt;
</code></pre><pre><code class="hljs language-blah">blah lang
&lt;
&amp;
&gt;
</code></pre>

• Note that if lang is empty, even though it does pass through hljs.highlight() with plaintext lang, the rendered code has no hljs language-plaintext class as expected.

Further notes:

    highlight(code, lang, info) {
      const language = hljs.getLanguage(lang) ? lang : 'blahfakelang';
      const out = hljs.highlight(code, { language });
      console.log('out:', out);
      return out.value;
    }

leads to uncaught throw:

Could not find the language 'blahfakelang', did you forget to load/include a language module?
/Users/slu/stevenlu.net/node_modules/highlight.js/lib/core.js:2107
      throw new Error('Unknown language: "' + languageName + '"');
            ^

Error: Unknown language: "blahfakelang"

essentially, when lang comes in empty, even though it's converted here to plaintext, it won't take, but when a nonempty lang is specified in the payload, then we override it to be plaintext here and it shows up PROPERLY as plaintext, but we cannot configure an empty lang into plaintext as expected via marked-highlight.

[unphased]

Culprit is https://github.com/markedjs/marked-highlight/blob/main/src/index.js#L45

@UziTech can you go over what you think would break if we remove this ternary and apply the class every time? User could at least override it if they need a plain code block for some reason.

[UziTech]

I don't know if it would break anything. It is set like that to be closer to the CommonMark spec where no lang mean there shouldn't be a class set.

We could add an option to specify the empty class. Something like:

marked.use(markedHighlight({
  emptyLangClass: "hljs",
  langPrefix: "hljs language-",
  highlight() {
    ...
  }
}));

[unphased]

Since I already made a fork and can deploy my app for now, there is no urgency on this, but I would love to help improve the behavior here to have a higher chance of meeting what is expected behavior and then I don't have to maintain my own fork.

For context, I'm designing a custom blog platform for my site, so I'm just approaching the code blocks from a minimalistic perspective. Since I haven't yet found a problem (trust that I will! sooner or later! I suspect I will want to SSR all code with tree-sitter at some point...) with highlight.js, that means I am currently all-in on highlight.js.

Because of the way hljs CSS is set up to behave, (and keep in mind I've only tested the default hljs theme so far lol I will build a themer for users to twiddle with that's hooked into a library of css themes) the background styling of code blocks doesn't appear if we don't include hljs class on a pre code tag.

Of course highlightjs doesn't care about markdown, so we provide a highlight() cb to implement it and we (following Marked extension convention) take over the unit of markdown code block rendering so the <pre><code> HTML manipulation is marked-highlight's unambiguous responsibility.

Here's the crux of it then, when a language isn't even specified, it is unclear whether to assign classes:

• lang-plaintext
• no lang class
• hljs class (this one confers code block bgcolor styling)

I want to say that use of marked-highlight implies "going all in on highlight.js" as is the obvious choice in the context of my project.

This would require at minimum hljs class assigned to code tags.

I just cannot imagine right now a use case where somebody would want to exclude highlight.js behavior when lang is not specified. But I do not want to rule that out. Perhaps your suggestion of a new config satisfies both. I just can't help but feel like the default existing behavior is wrong in the happy path case as it was for me.

Perhaps another reasonable approach could be to expose the <pre><code> tag assembly as well to the highlight callback. This would have prevented the issue I was having from only being resolvable by editing code in this repo. I suspect that this idea is a non-starter due to async tokenization concerns.

[unphased]

by the way the change i made in my fork (simply removing the ternary) leads to empty blocks having the class hljs lang- which looks questionable but certainly renders as I expect. It may more correctly be hljs without any lang, if lang is ''. It might also be plaintext. But I can't decide yet if it should be filtered down into to a lang that highlight.js can render or if it should keep (as now) verbatim to the specified lang...

[UziTech]

The issue with your argument is that marked-highlight does not imply using highlight.js. marked-highlight is generic so the user can use whatever highlighting library they want. Other highlighting libraries highlight all <pre><code> tags instead of requiring a class.

If you want to fork marked-highlight specifically for highlight.js I think that would be a good idea since that seems to be the most common one. So users wouldn't even have to specify a highlight function.

We do want to keep marked-highlight generic

[chky-nmnsoftware]

I don't know if it would break anything. It is set like that to be closer to the CommonMark spec where no lang mean there shouldn't be a class set.

We could add an option to specify the empty class. Something like:

marked.use(markedHighlight({
  emptyLangClass: "hljs",
  langPrefix: "hljs language-",
  highlight() {
    ...
  }
}));

I would love to see this as well. I am having the same issue, so this would be very helpful. Another way would be to have an option called "defaultToPlaintext" that automatically adds hljs language-plaintext to the code. Both approaches work.

[unphased]

It sure seems like this exact suggestion

I don't know if it would break anything. It is set like that to be closer to the CommonMark spec where no lang mean there shouldn't be a class set.

We could add an option to specify the empty class. Something like:

marked.use(markedHighlight({
  emptyLangClass: "hljs",
  langPrefix: "hljs language-",
  highlight() {
    ...
  }
}));

will provide what is needed for hljs to play nice. Let's plan to do that, then.

[github-actions[bot]]

:tada: This issue has been resolved in version 2.2.0 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket: