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

Change the signature of Prism.highlight #3416

Closed
RunDevelopment opened this issue Mar 31, 2022 · 4 comments
Closed

Change the signature of Prism.highlight #3416

RunDevelopment opened this issue Mar 31, 2022 · 4 comments

Comments

@RunDevelopment
Copy link
Member

RunDevelopment commented Mar 31, 2022

Motivation
Right now, Prism.highlight takes the parameters text, grammar, and language is that exact order. This is suboptimal because grammar and language are redundant. Is can plainly be seen in the example given in the docs:

Prism.highlight('var foo = true;', Prism.languages.javascript, 'javascript');

Why do I have to give it Prism.languages.javascript? I already told the function the language.

The redundancy also makes it possible to use the function incorrectly. I.e. if grammar doesn't have the value Prism.languages[language], then what does the language parameter even mean?

Description
Change the signature from:

function highlight(text: string, grammar: Grammar, language: string) { ... }

to:

interface HighlightOptions {
  grammar?: Grammar;
}

function highlight(text: string, language: string, options?: HighlightOptions) { ... }

Edit: As suggested by @LeaVerou, the grammar parameter gets moved into a dictionary of optional options. The grammar sued for highlighting will be evaluated as options.grammar ?? Prism.languages[language]. No functionality will be lost, but the minimal example will be a lot simpler:

Prism.highlight('var foo = true;', 'javascript');

What do you think? This will make Prism a little easier to use, but it's also a considerable breaking change (obviously v2), maybe even too much of a breaking change?

@LeaVerou @mAAdhaTTah @Golmote @JaKXz

@LeaVerou
Copy link
Member

If we're going to be making breaking changes like that, we should probably move any optional options to a dictionary.
Also, we need to make sure one can highlight text with an "anonymous" grammar, i.e. by just providing a grammar object that is not on Prism.languages (or would that not work today?).

@RunDevelopment
Copy link
Member Author

move any optional options to a dictionary

It's probably overkill right now, but it'll make it easy to add new stuff in the future. I like it. I'll update the issue.

just providing a grammar object that is not on Prism.languages (or would that not work today?).

Well, the language parameter is still required because it is passed to plugins via the hook environment. However, our plugins just need some language id/name. From my understanding, they don't assume/require that env.grammar == Prism.languages[env.language], so everything should work fine.

Of course, what plugins are allowed to assume about env.grammar and env.language is also an interesting question.

@mAAdhaTTah
Copy link
Member

I'm 👍 on the changes proposed here.

@RunDevelopment
Copy link
Member Author

Implemented in v2.

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

No branches or pull requests

3 participants