I have a suggestion for this module which you may want to consider.

Firstly remove all that stuff from hook_init() and move it to another function, say syntaxhighlighter_headers();
Create a theme function theme_syntaxhighlighter($code) which calls syntaxhighlighter_headers() and prints out the PRE tags with appropriate class around the $code.
The correct way to output a syntax highlighted string now should be like so: theme("syntaxhighlighter", $code);

The reason for this is that it is really annoying to load the css/js all over the whole website, and telling people that the syntax to use your module is by typing in some html and setting classes is a bit weird. 'theme' functions are more of a drupal way.

Comments

mattyoung’s picture

mattyoung commented

The function of this module is for easy content formatting, just like any css styling.

>theme("syntaxhighlighter", $code);
make a php function call to format content is not the intended usage of this module. This module for syntax highlighting code listing in any Drupal content entry (e.g. blog entry, story, page). It not for programming usage.

styling paragraph:
<p class="some-css-style">

<p>

syntax highlight code:
<pre class="brush: php">

<pre>

>it is really annoying to load the css/js all over the whole website
If you install and enable this module, that mean you want to display syntax hightlighted code in your site's content. The browser only request js/css files once. Drupal aggregate js and css if you turn it on. So the action inside hook_init() is very cheap performance-wise and bandwidth wise is a no-op after the first page visit to your site.

danielb’s picture

danielb commented

Sorry but your reasoning doesn't make sense. Imagine I have 50,000 nodes on my website. One of these nodes, say node with nid 38,034, contains a code block, where I need the css and js of syntax highlighter. You are saying that it is more efficient to load the css and js for every visitor who may browse any one of 49,999 nodes instead of just the one node I need it for??
I don't think so mate.

By putting things in hook_init of every single page you are forcing the functionality of your module to appear for everybody everywhere. Especially when the drupal core provides a means to avoid doing exactly this, which would be perfect for your module.

danielb’s picture

danielb commented

Plus theme() should be the primary way of adding css/js and creating output, no question about it. It allows people to overwrite parts of your functionality, and you can simply rewrite your automated html replacement feature to use theme(). Then suddenly your module becomes more useful from a drupal developer's perspective in addition to what is currently on offer.

Everything in drupal that I've worked with (core and contrib), that adds css and js to create some sort of fancy feature, works exactly as I have explained it.

danielb’s picture

danielb commented
Status: Active » Closed (works as designed)

Actually I've found a few examples of modules that counter my idealistic philosophies. Maybe your way is fine too. Oh well consider this a thought exercise.

If I really cared I would try to reimplement things my way, but obviously it is a bit too much effort, lol..

jahjah92’s picture

jahjah92 commented

I join to this feature request.

It would be nice to load js/css files only when a node use it in its text filters.

thx br

jurriaanroelofs’s picture

jurriaanroelofs commented

Was a bit disappointed to see css and js being loaded on every page. I only use it on a relatively tiny part of my website and it adds 7+19.2+3.4+1.7=17kb with CSS optimization disabled and default theme. Not much but it adds up.

Thanks for a great module though! I hated the output of GeSHi

mattyoung’s picture

mattyoung commented

Browsers cache css/js so they are only loaded on first visit. So there is really no bandwidth overhead.

drupal_add_js/css() are called on every page. But they are not very expensive.

Keep in mind that syntax highlighting can be in comments and forum postings, not just in nodes. In fact, you can output syntax highlighting anywhere on a page. This make it difficult to detect if there is any syntax highlighting on a given page, then conditionally call drupal_add_js/css(). I don't even know how to detect this and it's probably far more expensive than adding the js/css on every page and let browsers caching them so they are not sent from Drupal to browsers every time but only the first time.

jurriaanroelofs’s picture

jurriaanroelofs commented

In my case I only use the syntax highlighter on some very deep pages that only get seen by less than 1% of my website visitors so it would be a nice feature if I could exclude the overhead from my other pages :)
Im not so much concerned about bandwidth but more about page load time. When Im out and about with my HSDPA modem I get 128kbit a lot of the time Im reminded that not everyone is on broadband.

mattyoung’s picture

mattyoung commented

>more about page load time

Can you do some benchmark to see what the page load time with syntax highlighter on vs. off? Be sure to use the latest version, run several times for both cases to prime the browser cache.

There is a "Turn off Syntax Highlighter on these pages" setting, can you use that?

momper’s picture

momper commented

subscribe