-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Initial implementation of manual highlighting #1087
Initial implementation of manual highlighting #1087
Conversation
Borrow the `manual` property of whatever already existing Prism instance there is.
Thanks! See comment. |
Cleans up the implementation w/ less code.
Updated! |
Just a thought: this does require some checks in the code that's supposed to disable Prism, if as in the scenario from the original issue, the "disabling" script can't know whether it's been loaded before or after Prism, but I don't think there's anything we can do about that. |
Thanks, merged! Yeah it does, but it could be as simple as: (window.Prism = window.Prism || {}).manual = true; |
I dont think that'll work because the callback will have already been attached. So more like: if (window.Prism) {
document.removeEventListener('DOMContentLoaded', Prism.highlightAll);
else {
window.Prism = { manual: true };
} |
Oh, right. The check for |
We can; we would break BC for people using the old |
I think that's ok, since this is much more robust :) What do you think? |
Fine by me; if we do that, we should document that somewhere though. Is the CHANGELOG enough or should we include a "how to disable auto-highlighting" somewhere? |
Ideally it should be in the docs, alongside the docs for |
My use case, which I think is fairly general when wanting to use manual, is to import and use the This is what I'm currently using, with the data-manual on my html/script tag. I'm hoping with this PR I can take that html tag out.
|
@mAAdhaTTah I am using PrimsJS yet again after many years and hit the same issue that had before, (#366), so thanks for fixing this. This could use some documentation though. It is not obvious how one would set this option. |
Borrow the
manual
property of whatever already existing Prism instancethere is.
Fixes #765.
In this setup, the
manual
property would have to be set before the script is included. Open to suggestions for improvement.