You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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:
Both now add special attributes to the pre element depending on the current status. The attributes are data-{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 all pre 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.
Deprecated Prism.fileHighlight in favor of Prism.plugins.fileHighlight.highlight analog to Prism.plugins.jsonphighlight.highlight.
Added a timeout config property for JSONP Highlight.
Updated error messages of both plugins to be more consistent.
FH preloads Both preload languages using the Autoloader if available.
Resolves the plugin incompatibility between FH and Copy to clipboard.
Details
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 pres 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 allow highlightElement 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 the callback).
The environment of the hook I imagine like this:
Hook callbacks will set the delegate property to true to say yes.
The hooks will be run before any DOM mutation, so here and the code to do so could look like this:
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.
It's pretty much ready as-is aside from one detail:
The :empty pseudo-class added to the selector is a breaking change. I understand why it makes sense to enforce this but is it worth a breaking change?
I removed the :emtpy check.
@mAAdhaTTah
I showed this PR some love again and would like to merge this soon (because both #1813 and #1969 are blocked by this one).
As a note:
The before-sanity-check hook of both plugins is quite similar because I wanted them to have a somewhat similar API. Despite that, I don't think that there is much to gain from trying to abstract and moving the common elements into a separate plugin.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
preelement 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:emptyfor allpreelements being processed by the plugins.Why? It doesn't make any sense for the
preto 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
codeelement without doing anything.Edit: Removed for backward compatibility.
Prism.fileHighlightin favor ofPrism.plugins.fileHighlight.highlightanalog toPrism.plugins.jsonphighlight.highlight.timeoutconfig property for JSONP Highlight.FH preloadsBoth preload languages using the Autoloader if available.Details
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
pres 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 allowhighlightElementto 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
delegateproperty totrueto 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
asyncparameter and highlight code synchronously.Note: The proposed solution for 2) fixes this issue as well.
This resolves #1965.