simonhaenisch / md-to-pdf

Hackable CLI tool for converting Markdown files to PDF using Node.js and headless Chrome.
https://www.npmjs.com/md-to-pdf
MIT License
1.16k stars 110 forks source link

Use marked `highlight` option instead of overriding `code` #114

Closed dyllan-to-you closed 2 years ago

dyllan-to-you commented 2 years ago

https://github.com/simonhaenisch/md-to-pdf/blob/e70ea15d29af0add7bcca4f8c3cc8b78b08ac2ff/src/lib/get-marked-with-highlighter.ts#L8-L15

Hi! I was wondering why code highlighting was implemented as an override to the code function of the marked renderer, versus utilizing the highlighted option in marked?

I believe setting the options such that

highlight: function(code, lang) {
    const hljs = require('highlight.js');
    const language = hljs.getLanguage(lang) ? lang : 'plaintext';
    return hljs.highlight(code, { language }).value;
  },
langPrefix: 'hljs '

would have the same result.

My current goal is to override the code renderer, and I do so by creating a new class that extends marked.Renderer and defining a new code method within there. The current implementation of the marked integration, however, ends up overriding my custom class. The alternative method

const renderer = new marked.Rendere0;
renderer.code = (code, infostring, escaped) => { /* do stuff */ }

does not allow me to easily fall back to the default handler.

If this change is OK I can submit a PR.

simonhaenisch commented 2 years ago

Hey, thanks for bringing this up... I was not aware of the highlight option, maybe that wasn't around yet when I implemented this? I started out with marked@0.3 😅

This should also allow to simplify the code base, but yeah I'm also planning to change the whole implementation around marked because there's now the use method to allow extensions, which I want to expose properly (will have to be a new major version though I think)... currently don't really have much time to work on this though. So thanks for the PR already, will have a look tomorrow.

dyllan-to-you commented 2 years ago

Heh that's fair. It does look like highlight is pretty new, and yeah utilizing that use method sounds like it would provide a nicer API!

Glad I could help!

simonhaenisch commented 2 years ago

I am mistaken, it was actually available in 0.3.0 already... must have just missed it (: