Provide a global way to disable auto highlighting

[markhalliwell]

Similar to #366, this is very cumbersome when working with something like a CMS that aggregates is assets and doesn't allow extra attributes (like data-manual) to be set on the script tags.

The only recourse I had was to manually "hack" the HTML template:
https://github.com/unicorn-fail/drupal-bootstrap.org/blob/831f047cfb7125778364cbccd2db34256b643222/docroot/sites/all/themes/bootstrap_subtheme/templates/system/html.tpl.php#L77

I'm ok with this solution, temporarily, but it doesn't bode well in the long term for performance reasons. Adding the file in this fashion prevents the file from becoming aggregated and causes an extra HTTP request.

I really don't care either way if it's called window.PRISM_JS_MANUAL or something else. The fact is that there needs to be a better way for Prism to detect when it's not supposed to run automatically than checking for a data attribute.

[Golmote]

I can think of multiple ways to solve this:

• Release a plugin that would basically contain:

  document.removeEventListener('DOMContentLoaded', Prism.highlightAll);

  This looks a bit ridiculous, though.

• Look for a class or an attribute on the <pre> tags (or their ancestors) to determine which code blocks are candidates to auto highlighing and which are not. This would allow some granularity, but I don't think anybody would actually need it.

• Allow for a global configuration variable to be defined before Prism. Something like:

  <script type="text/javascript">
  var Prism_config = {
     manual: true
  };
  </script>
  <script type="text/javascript" src="prism.js"></script>

  This is probably, despite the global variable, my preferred solution. We could think about adding more options to this configuration object, like hooks definitions, async highlighting by default, maybe some plugin-related stuff...

@LeaVerou: what's your opinion on this?

@MarkCarver: Note that the first solution would be an easy way to solve your issue for now.

[markhalliwell]

@Golmote that's brilliant! Thank you! I'm not even sure why I didn't try that to begin with, makes total sense now that I'm looking at it.

FWIW, the third suggestion is my preference too. This is a very common approach in other JS libraries, especially ones like jQuery, e.g. $.fn.somePlugin.defaults. Granted, they have the jQuery ($) global already defined, but the concept is the same regardless.

[dellagustin]

I remember I had a similar issue when reusing code from a Chrome extension in a Greasemonkey script. I had to adapt my code accordingly. Solution 1 would have probably worked, but I am a WebDev newbie.
(the script: https://github.com/dellagustin/SAP_Note_Enhancer/blob/firefox_gm_user_script/greasemonkey/sap_note_enhancer.user.js)

The third option looks interesting, maybe it would work with Greasemonkey by having a @require to a .js file containing the configuration before the @require to prismJS, but this would force the hosting of one additional file.

Maybe, you should provide a method in Prism that encapsulates

document.removeEventListener('DOMContentLoaded', Prism.highlightAll); 

like Prism.preventAutoHighlighting or Prism.manualHighlighting. This way you would be able to fix users implementations in case you make changes to the way the listener is currently used.

[markhalliwell]

Actually, considering that it doesn't highlight until the DOMContentLoaded event, this means that one can modify global Prism object after the script has been added. This would avoid having some weird standalone global object (Prism_config).

---

Hypothetically, all that's really needed is something like the following.

Add a config object to _self.Prism:

var _ = _self.Prism = {
  config: {
    autoHighlight: true
  },
  // ...
};

Then, refactor the script tag/autoHighlighting a bit:

// Get current script and check for "data-manual" attribute.
// @todo deprecate this?
var script = document.getElementsByTagName('script');
script = script[script.length - 1];
if (script) {
  _.filename = script.src;
  _.config.autoHighlight = !script.hasAttribute('data-manual');
}

// Auto Highlight.
if (document.addEventListener) {
  document.addEventListener('DOMContentLoaded', function () {
    if (_.config.autoHighlight) {
      _.highlightAll()
    }
  });
}

---

Then all one would have to do is:

<script src="prism.js"></script>
<script src="script.js"></script> <!-- script added after Prism in the CMS. -->

script.js:

// Disable Prism auto highlighting.
Prism.config.autoHighlight = false

// Script continues to do some stuff, like extending Prism.
// ...

// Then the script can manually highlight on DOMContentLoaded.
$(document).ready(function () {
  Prism.highlightAll();
});

[k-funk]

+1 for any solution that doesn't require me editing my html files.

Alternative to the other suggestions, if Prism was split into separate files, I could just import { highlight, languages } from 'prism/lib';

[LeaVerou]

We already have a setting for this, though via HTML (<script data-manual>). We should have a JS property for this, and just set that if data-manual is found on the script element, to have both a JS and an HTML API for this. It's probably a good first pull request, if there are any takers! :)

[markhalliwell]

We already have a setting for this, though via HTML

No there's not. This is a hard-coded DOM check; one that isn't able to be added in certain CMS/CMF instances very easily. This isn't a "setting" (in the normal sense)... it's a DOM hack. In fact, there are no true JS settings for that matter, nor is there any standard way of initialization of this plugin, regardless of when the script is loaded vs. DOM ready.

It's probably a good first pull request, if there are any takers! :)

Considering that this issue nearly a year and a half old, I doubt that's going to happen... especially since this issue implies some much needed refactoring around how and when this "auto highlight" is actually invoked based on possible "settings".

[LeaVerou]

This isn't a "setting" (in the normal sense)... it's a DOM hack.

It's funny how anything done via JS is considered "the normal way" by some. I consider an HTML API just as important (if not more important) than a JS API. Many people who use Prism cannot write JS. Allowing configuration via HTML is not a hack. Try being a little more inclusive in your thinking.

In any case, I wasn't even arguing that this is sufficient, I said myself that we should add a JS property for this as well, so not sure exactly what your problem is.

[LeaVerou]

@Golmote Just saw your message. I'm not a huge fan of adding a separate global object, I'd prefer Prism.manual. If Prism has not been created at the time of setting, we can use a similar solution to what we do in Bliss where we copy any properties from a pre-existing Bliss object onto the newly created Bliss object.

[markhalliwell]

It's funny how anything done via JS is considered "the normal way" by some.

I meant that there is no (JS) configurable "options" or "settings" anywhere in Prism's JS code. Almost ever single JS plugin/app out there has this... that's what is "normal". This isn't a radical idea............

I consider an HTML API just as important (if not more important) than a JS API.

Except that Prism (itself) isn't HTML, it's written and executed in JS.

When a website adds prism, it's executing JS, not HTML.

When highlighting code on node.js, it's executing JS, not HTML.

If a site needs to load additional plugins/events/whatever BEFORE Prism "autoloading" occurs, it should be able to set some configuration (in JS somewhere) that disables this "autoloading".

Many people who use Prism cannot write JS.

Seriously?! What kind of BS reason is this? It's certainly not reason enough to chastise those who do know how to "write JS" and need this pretty major bug fixed for very involved setups.

Allowing configuration via HTML is not a hack.

On normal DOM element, perhaps, however in this case... it most certainly is. There are too many assumptions with this approach, the least of which that there is some "dedicated <script> tag" for including Prism. As mentioned above, in CMS/CMF applications... this is rarely the case when these scripts are aggregated/minified.

Furthermore, and expanding on the "normal" comment above, the fact that this type of configuration has no native JS equivalent is massive anti-pattern in almost ever single piece of JS software out there. So yes, this type of configuration is a "hack", for the simple fact that it's using HTML to configure JS.

Try being a little more inclusive in your thinking.

Try not insinuating that I'm "close minded". It's clear from my comment above that I was relatively "happy" with the temporarily solution. That solution, however, has since stopped working now that the site has undergone some performance tuning and now aggregates/minifies absolutely everything.

In any case, I wasn't even arguing that this is sufficient, I said myself that we should add a JS property for this as well, so not sure exactly what your problem is.

My problem is that you essentially pawned this pretty major bug on other (non-project-specific) people:

"It's probably a good first pull request, if there are any takers! :)"

1. This issue isn't something that can be "easily fixed", if only because:
2. There will need to be a concise plan around how to move forward on this topic (as evidence from the divisiveness of even the collaborators in the project itself above).
3. Continues to ignore the problem by shifting responsibility from the ones actually responsible for the project. Granted, I get how open source works, but considering that it's already been almost a year and a half and there have been no "takers"..... that should have been a sign in and of itself....

My "problem" is that I don't particularly enjoy comments that just add more fluff to a conversation without actually adding anything to the conversation or even attempt to fix it.

My "problem" is that I need a more stable and standardized solution for this issue and it can only be provided upstream given how this code is constructed.

My "problem" is that it's been nearly a year and half with practically no progress from the initial report.

---

I'd prefer Prism.manual

I'd prefer that this project stop using anti-patterns (arbitrary naming) and instead use a dedicated configuration object (options/settings/config/whatever...) and property:

Prism.options.autoHighlight

1. It properly identifies what this configuration is/does.
2. It places this configuration is an appropriate namespace.
3. By virtue of the above, also allows for any future configuration growth.

This is what is "normal" looks like in JS....

[mAAdhaTTah]

Seriously?! What kind of BS reason is this?

It's pretty much the fundamental motivation behind the development of the project in the first place and has been a driving factor in a number of decisions related to the plugins. All of the plugins have HTML APIs.

My "problem" is that I don't particularly enjoy comments that just add more fluff to a conversation without actually adding anything to the conversation or even attempt to fix it.

In the amount of time it took you to write this giant comment, you could have opened a pull request and proposed a solution, instead of throwing a tantrum. If you don't like Prism.manual, then open the PR with Prism.options.autoHighlight and hash it out there.

[LeaVerou]

@MarkCarver Wow. You do realize that this is an open source project and all of us are working on it on our free time, right? We're not your employees, so I would advise you to watch your tone. And to think I didn't even reject your suggestion, lol. I wonder what kind of reaction that would have ensued!

and need this pretty major bug fixed for very involved setups.

I'd be careful labelling this as a "major bug". Thousands of people and big websites use Prism without having any issues with this. Just because it's causing trouble in your specific setup does not make it a major bug. It's not even a bug, you're requesting a feature.

Except that Prism (itself) isn't HTML, it's written and executed in JS.
When a website adds prism, it's executing JS, not HTML.
When highlighting code on node.js, it's executing JS, not HTML.

Yes, Prism is JS, that doesn't mean everyone who uses it needs to know JS. That's the whole point of abstractions. You use software coded in C++ or even assembly daily, that doesn't mean you have to know these languages to use this software. Also, given that I did not disagree with you about it being a good idea to have a JS property, it's still unclear to me why you're complaining. Just the existence of the HTML setting bothers you?!

as evidence from the divisiveness of even the collaborators in the project itself above

What divisiveness? @Golmote suggested three solutions, indicated that he prefers the 3rd, and I agreed with his suggestion and proposed a few tweaks. That's not divisiveness, that's how projects work.

[markhalliwell]

It's pretty much the fundamental motivation behind the development of the project in the first place

It's not reason enough to justify not putting in some basic JS configuration, nor should it be used as an excuse.

In the amount of time it took you to write this giant comment, you could have opened a pull request and proposed a solution, instead of throwing a tantrum.

First, it wasn't a "tantrum"; I was stating facts for an issue that has been overlooked time and time again. Second, the "giant comment" is exactly what I have been "hashing out" here in this issue (the issue I already created almost a year and a half ago).

You do realize that this is an open source project and all of us are working on it on our free time, right? We're not your employees, so I would advise you to watch your tone.

Of course I know this is open source. I'm not stupid, unlike the consistent insults y'all continue to insinuate my way regarding as such.

We aren't your "employees" either. A "solution" to an issue shouldn't be "open up a PR". There are reasons there are both ISSUES and PRs.

I don't have the time to setup a local dev/test environment for this project... jump through whatever insane local "PR/contributing rules" y'all might have and then go through the minutia of trivial bickering that will clearly happen as is quite evident in this issue and several other PRs/issues I have witnessed in this project.

I informed this project of a serious bug and I'm being treated as if it's trivial and some wayward soul who doesn't know what he's talking about.

I am very well versed in open source and maintain one of the most popular Drupal (CMS) Bootstrap base theme integrations.

I'd be careful labelling this as a "major bug".

The plugin auto highlights and there there is absolutely no way to turn it off at the JS level..... that's a major bug.

Thousands of people and big websites use Prism without having any issues with this.

I'm not discounting that. However, you are all assuming these are static implementations. When dealing at a CMS/CDN level... that is so not the case. This data-manual attribute cannot be added to the <script> tag because there is no dedicated/standalone <script> tag for Prism in CMSes like Drupal (and I'm not the "only one who uses" Drupal)....

I really, don't understand how many times I have to say that.

It's not even a bug, you're requesting a feature.

No I'm not. The "feature" (auto-highlighting/toggling it) is already in, it's just not configurable from JS.

Just the existence of the HTML setting bothers you?!

No. Just the fact that there is no JS configuration ability of any kind. The only "configuration" is done via HTML data attributes, which.... again.... is quite assumptive.

Data attributes came after... everything. I wouldn't have imagined that is the only way to configure a JS plugin... but alas, that is the case.

---

That all being said, I can see now that this issue is a moot point. Clearly all any of you really care about is patting yourselves on the back and insulting people rather than actually fixing the problem in the first place (evidence by all the thumbs up/downs).

If any of you truly were concerned about the project you'd see that my frustration stems from the lack of progress and continued finger pointing. That's my "problem".

[Golmote]

Well that escalated quickly!

@MarkCarver This issue has, indeed, been open for a long time. You'll notice, though, that as of the first day you submitted it, we came out with an immediate (temporary) solution for you.
You bumped up the issue last year, without explicitely saying that the solution had stopped working for you (was it actually the case?). There did not seem to be any kind of urgency to fix this not critical lack of configuration through JS.

There has been no progress after that because we had not reached any consensus about the way of solving this issue. Now that we kinda have, someone might start working on it. Closing this issue won't help moving that way, though.

That solution, however, has since stopped working now that the site has undergone some performance tuning and now aggregates/minifies absolutely everything.

I'm not sure I understand this specific statement. If the removeListener is registered after the Prism code, it should still work, being minified or not, shouldn't it?

[mAAdhaTTah]

@MarkCarver I opened a pull request for this (#1087), because that's how open source works. It does not work by whining in GitHub issues. I spent 9 months shepherding a pull request into this repository, and not once did it rise to the level of acrimony you introduced in the past day or so.

I highly recommend you read this from James Kyle and reconsider how you treat maintainers of open-source projects in the future.

[markhalliwell]

You bumped up the issue last year, without explicitely saying that the solution had stopped working for you (was it actually the case?).

At that time, no it wasn't as apparent that it had stopped working.

If the removeListener is registered after the Prism code, it should still work, being minified or not, shouldn't it?

It's actually not that it doesn't work, but rather doesn't work consistently. This "solution" depends on when/where the removeListener event is actually registered.

If Prism and the custom removeListener event are located in separate files (which can happen in complex CMS aggregation), then the DOM ready event from Prism can still actually fire depending on any latency that may creep in (at least that has seemed to be the case from what I have observed).

Suffice it to say, this workaround is just that... a work around, it's not an stable enough solution.

---

Well that escalated quickly!

@LeaVerou made a comment without, what seemed, to have actually read the issue before hand.

I rebutted with my comment. It contained facts and no negative connotations towards anyone in particular.

This should have been the end of it, or at least the start of actually moving something forward.

However that was not the case and it escalated because @LeaVerou then started insinuating, insulting and belittling me.

It further escalated because I defended myself and this issue.

It blew way out of proportion because @mAAdhaTTah continued the insinuations, insults and belittling and I again, defended myself.

---

I'm very well aware of how open source works @mAAdhaTTah: the squeaky wheel gets the oil, no matter whether it's for positive or negative reasons... that's human nature. Case in point: the attention this issue got and the PR you have since submitted.

However, I find it extremely hypocritical and egotistical to reference a blog piece about tolerance and level-headedness when all you or @LeaVerou has does is nothing short of the opposite.

If you want people to "treat maintainers" with respect and in the spirit of collaboration, perhaps neither of you should belittle and insult them while also trying to make your points.

Also, all of you "thumbs down-ing" my comments and "thumbs up-ing" yours. I was under the impression that those tools were meant to convey importance of ideas, not be used as tools for ostracizing, criticizing and bullying people and what they have to say.

Can any of you see how that just furthers the escalation? You talk about being open-minded, but then (visually) negatively dismiss everything I said. Yeah, real mature.

Open source is about communication and being open to what the other person has to say. Just because I chose to talk this out instead of "opening a PR" doesn't mean that you have license to treat me or others like crap.

I'm really not an asshole, but can see how people may misconstrue my passion and defense of being insulted and belittled... as such. That isn't my problem.

Closing this issue won't help moving that way, though.

The summation of escalation was at a pinnacle when I realized that all @LeaVerou or @mAAdhaTTah were doing was continuing an ego and power trip instead of actually talking about the issue at hand.

I felt that, since I opened the issue, closing it would stop the incessant bickering, regardless if my OP has been "fixed" or not.

I'm sorry that you feel I was responsible for "acrimony" and that it got to this point.

I have no desire to cause discord or strife in any community.

I will not, however, allow myself to be treated like y'all have treated me.

Peace.

[LeaVerou]

Guys, it's obvious that @MarkCarver has a lot more time in his hands than we do, so I'd recommend we stop this fruitless conversation and focus on more productive and/or enjoyable things. Don't try to convince him of anything, it's very clear to him that he's right and we're all wrong and he's not even considering what we're saying, just being on the defensive. Don't let people like that suck the energy out of you.

@mAAdhaTTah thanks for the pull request! Yup, that's the way open source projects work. I'm glad the community has people like you.

[markhalliwell]

@LeaVerou, thanks for continuing to insult me, you just proved my point.

[mAAdhaTTah]

@MarkCarver Go away. Seriously, we've all had enough of your childish behavior.

[markhalliwell]

So defending myself against consistent insults and blatant attacks is considered "childish"?

Wow.... just wow...

The "contributors" of this project have some serious issues when it comes to people disagreeing with them, the least of which are down right cruel and rude remarks.

I will gladly unsubscribe myself now. I don't even care if this issue gets fixed anymore. I don't need this kind of abuse from power tripping egomaniacs.

[mAAdhaTTah]

I shepherded a PR into this project over 9 months, and it got nowhere near as acrimonious as this, despite a significant amount of debate on it. You're the problem here.