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

Improve code_highlight configuration options type to be Partial #679

Merged
merged 2 commits into from
Oct 23, 2024

Conversation

philj0st
Copy link
Contributor

@philj0st philj0st commented Oct 23, 2024

Description

Make it so users of the Highlight Code Plugin don't have to pass all options to the configure wrapper for highlightjs.
The object is only merged with the defaults and then passed to PublicApi::configure which has a type signature itself of:
configure: (options: Partial<HLJSOptions>) => void
https://github.com/highlightjs/highlight.js/blob/4e485f222e8c604b92937870414cf401e9dff31f/types/index.d.ts#L38

Related Issues

Example: I wanted to have the pre code.mermaid-language processed by another plugin so i wanted to add:

site.use(codeHighlight({
    options: {
        noHighlightRe: /^mermaid$/i
    }
}));

Deno-ts then sais:

Type '{ noHighlightRe: RegExp; }' is missing the following properties from type 'Omit<HLJSOptions, "__emitter">': languageDetectRe, classPrefix, cssSelector`

Check List

I assume it's not a big enough change to warrant tests or an entry in CHANGELOG.md.

  • Have you read the
    CODE OF CONDUCT
  • Have you read the document
    CONTRIBUTING
    • One pull request per feature. If you want to do more than one thing,
      send multiple pull request.
    • Write tests.
    • Run deno fmt to fix the code format before commit.
    • Document any change in the CHANGELOG.md.

@philj0st
Copy link
Contributor Author

philj0st commented Oct 23, 2024

Sorry, I just realised this brakes processCodeHighlight. I will fork it locally properly and add tho this PR.

Add non-null assertion for highlightjs cssSelector option
@philj0st
Copy link
Contributor Author

I added the non-null assertion for the cssSelector option, if you don't like it, i can change it to:

    function processCodeHighlight(pages: Page[]) {
      const cssSelector = options.options?.cssSelector
      if (cssSelector) {
        for (const page of pages) {
          page.document!.querySelectorAll<HTMLElement>(
            cssSelector
          )
            .forEach((element) => {
              try {
                hljs.highlightElement(element);
                element.removeAttribute("data-highlighted");
              } catch (error) {
                log.error(
                  `Error highlighting code block in ${page.sourcePath}: ${error}`,
                );
              }
            });
        }
      }
    }

@oscarotero oscarotero merged commit e3a68e7 into lumeland:main Oct 23, 2024
6 checks passed
@oscarotero
Copy link
Member

it's perfect! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants