Skip to content

Commit

Permalink
Run before-highlight and after-highlight hooks even when no grammar i…
Browse files Browse the repository at this point in the history 
…s found. Fix #1134
  • Loading branch information
@Golmote
Golmote committed May 8, 2017
1 parent 867ea42 commit 70cb472
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 1 deletion.
2 changes: 2 additions & 0 deletions components/prism-core.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,9 @@ var _ = _self.Prism = {

if (!env.code || !env.grammar) {
if (env.code) {
_.hooks.run('before-highlight', env);
env.element.textContent = env.code;
_.hooks.run('after-highlight', env);
}
_.hooks.run('complete', env);
return;
Expand Down
2 changes: 1 addition & 1 deletion components/prism-core.min.js
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions prism.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,9 @@ var _ = _self.Prism = {

if (!env.code || !env.grammar) {
if (env.code) {
_.hooks.run('before-highlight', env);
env.element.textContent = env.code;
_.hooks.run('after-highlight', env);
}
_.hooks.run('complete', env);
return;
Expand Down

3 comments on commit 70cb472

@mAAdhaTTah
Copy link
Member

Choose a reason for hiding this comment

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

@Golmote This causes a bug in the show-invisibles plugin when used with the autoloader. The grammar is undefined, so attempting to add tokens to it fails. I can add a check in show-invisibles to make sure the grammar exists, but wondering if that's part of the "contract" of the hook. Should we able to rely on the existence of env.grammar in plugins?

@Golmote
Copy link
Contributor Author

@Golmote Golmote commented on 70cb472 Sep 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@mAAdhaTTah Currently, element is the only key in env that should always be properly set... code and language might be empty strings (though still strings !), and grammar can indeed be undefined.
What are our options here? We could set grammar to the empty object instead, and use another special key to indicate the grammar does not exist in Prism... Or we could officially decide grammar is not safe to use in hooks without checking it's defined first...

EDIT: Maybe we don't need another key... The Autoloader plugin could probably directly check Prism.languages[language] instead of relying on grammar.

@mAAdhaTTah
Copy link
Member

Choose a reason for hiding this comment

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

Or we could officially decide grammar is not safe to use in hooks without checking it's defined first...

My PR (#1195) assumes that's the case for the show-invisibles plugin. However, there are a couple other plugins that assume env.grammar. If we're going to set that as the "contract" for hooks, then there are a few others to update.

Please sign in to comment.