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

Export hljs object #169

Merged
merged 7 commits into from
Feb 8, 2022
Merged

Export hljs object #169

merged 7 commits into from
Feb 8, 2022

Conversation

imjamesb
Copy link
Contributor

@imjamesb imjamesb commented Feb 6, 2022

Ability to register languages is missing without it. Workaround is to import the dependency manually, but urgh..

Ability to register languages is missing without it. Workaround is to import the dependency manually, but urgh..
@oscarotero
Copy link
Member

Hi. Thanks for your contribution.
By default, all languages are loaded, but I guess you want to register additional languages, right?

I'm not sure if import and customize hljs from the _config file is a good idea. What do you thing about adding this option to the plugin? Something like this:

site.use(codeHighlight({
    languages: { javascript, css, html }
}));

Internally, the plugin could do this:

for (const [name, lang] of Object.entries(options.languages)) {
    hljs.registerLanguage(name, lang);
}

Doing so, the plugin doesn't need to export anything to be configured. This is how all other plugins work.

@imjamesb
Copy link
Contributor Author

imjamesb commented Feb 6, 2022

That definetely works :)

@oscarotero
Copy link
Member

Nice! Do you like to implement it in this pull request? I'll be happy to merge it

Copy link
Contributor Author

@imjamesb imjamesb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added languages property on highlightjs plugin options to register languages.

@@ -1,6 +1,8 @@
import hljs, { HighlightOptions } from "../deps/highlight.ts";
import { merge } from "../core/utils.ts";

export { hljs };
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not export, rather expose a languages property.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It did not push changes from github editor...

Copy link
Contributor Author

@imjamesb imjamesb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added languages property on highlightjs plugin options to register languages.

@@ -1,6 +1,8 @@
import hljs, { HighlightOptions } from "../deps/highlight.ts";
import { merge } from "../core/utils.ts";

export { hljs };
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It did not push changes from github editor...

@imjamesb
Copy link
Contributor Author

imjamesb commented Feb 6, 2022

Had a bit of troubles with the github.dev editor, it's probably smart to squash the commits for better commit message :D

@oscarotero
Copy link
Member

There are some problems regarding to the typescript types. I think it's better to make changes in local and test it before commit.
Do you want to work on it? If not I can fix it, but I let you do it if you want.

@imjamesb
Copy link
Contributor Author

imjamesb commented Feb 6, 2022

Yeah, I am currently not on my PC with an editor, so I am using github.dev :)

@oscarotero
Copy link
Member

Ok, nice!
Let me know when you want to review and merge. No rush.

@imjamesb
Copy link
Contributor Author

imjamesb commented Feb 6, 2022

Should be good to go

@imjamesb
Copy link
Contributor Author

imjamesb commented Feb 6, 2022

Seems like the esm mirror doesn't export types :( I'll look through highlight.js source code and find the type manually.

@imjamesb
Copy link
Contributor Author

imjamesb commented Feb 6, 2022

Please re-run :)

plugins/code_highlight.ts Outdated Show resolved Hide resolved
Co-authored-by: Oscar Otero <oom@oscarotero.com>
@@ -30,6 +33,13 @@ export default function (userOptions?: Partial<Options>) {
// @ts-ignore: Property 'configure' does not exist on type '{}'
hljs.configure(options.options);

if (options.languages) {
for (const [name, fn] of Object.entries(userOptions.languages)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missed change it here

@oscarotero oscarotero merged commit b13f9e2 into lumeland:master Feb 8, 2022
@oscarotero
Copy link
Member

thanks!

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