-
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
File Highlight & JSONP Highlight update #1974
File Highlight & JSONP Highlight update #1974
Conversation
I added this to the PR.
|
@mAAdhaTTah As a note: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If my understanding of that coordination is correct, then this looks good to me.
This implements the changes to File Highlight discussed here and also applies them to JSONP Highlight making both APIs very similar.
I took the chance to basically rewrite both plugins, so here's a list of changes:
pre
element depending on the current status. The attributes aredata-{src|jsonp}-status="{loading|loaded|failed}"
depending on the plugin and the current status. These also prevent multiple (or even concurrent) executions for the same element.Edit: changed attributes
It adds a new restriction:empty
for allpre
elements being processed by the plugins.Why? It doesn't make any sense for the
pre
to contain anything, so enforcing it seems like a good idea (and a breaking change).The previous behavior was that FH just delete the text and JH just appended the new
code
element without doing anything.Edit: Removed for backward compatibility.
Prism.fileHighlight
in favor ofPrism.plugins.fileHighlight.highlight
analog toPrism.plugins.jsonphighlight.highlight
.timeout
config property for JSONP Highlight.FH preloadsBoth preload languages using the Autoloader if available.A few things I would like to further discuss and potentially change:
1. Preloading language. Edit: Done.
Both plugins know which language the code is before the file is received. This can be used to preload the language definition using the Autoloader which has to fetch it anyway.
This requires exposing Autoloader's language loading API (already done in #1898). Alternatively, we could also create a fake element to highlight which will cause Autoloader to load the languages.
2. Fixing callbacks. Edit: The fix in #1973 is good enough
Right now, the callback function for the
pre
s won't be executed at all. #1973 fixes this but only calls it synchronously.To fix this I would like to add a new hook:
delegate
. The purpose of the hook is to allowhighlightElement
to hand off the responsibility of highlighting the given element to some other instance. All registered hook callbacks will then be asked whether they take the element and the first to say yes has the responsibility to handle highlighting the element (including calling thecallback
).The environment of the hook I imagine like this:
Hook callbacks will set the
delegate
property totrue
to say yes.The hooks will be run before any DOM mutation, so here and the code to do so could look like this:
Thoughts?
3. Asynchronous highlighting. Edit: Maybe later.
Right now, both FH & JH ignore the
async
parameter and highlight code synchronously.Note: The proposed solution for 2) fixes this issue as well.
This resolves #1965.