There's a feature to override preprocess functions but it is not useful in its current state.
As the preprocess functions are built, the existence of the key "override preprocess functions' prevents further building of the "preprocess functions" array as it recurses through adding to the registry.
if (isset($cache[$hook]['preprocess functions']) &&
is_array($cache[$hook]['preprocess functions']) &&
empty($cache[$hook]['override preprocess functions'])) // <- Checks the cached version of this key.
{
$info['preprocess functions'] = array_merge($cache[$hook]['preprocess functions'], $info['preprocess functions']);
}
So, if a theme engine wanted to override a preprocess function, All it can do is override the functions that are set from themes as it is further down the chain. This is because it checks the version in $cache. For example, phptemplate_theme() can override all the preprocess functions set from garland_theme(), not the other way around. This seems backwards and not intentional. A theme cannot override the functions and template engines doing the overrides cripples the themers ability to add more preprocess functions.
What this patch does is allow *any* invocation of HOOK_theme() to override all preprocess functions that were set *before* it not after.
All that's needed is this. Overrides 'page' hook to use my custom preprocessors:
function garland_theme($cache, $type, $theme, $path) {
// Attache Cached information.
$result['page'] = $cache['page'];
// Supply override.
$result['page']['override preprocess functions'] = array(
'template_preprocess_page',
'garland_preprocess',
);
return $result;
}
What this will do is replace all cached preprocess functions with the override. Not many themers or engine authors may use this but this advanced feature makes it work with more flexibility. Besides, the current code seems broken.
Comment | File | Size | Author |
---|---|---|---|
#11 | theme_registry_override_4.patch | 1.88 KB | dvessel |
#3 | theme_registry_override_3.patch | 1.54 KB | dvessel |
#2 | theme_registry_override_2.patch | 2.3 KB | dvessel |
theme_registry_override_1.patch | 4.81 KB | dvessel | |
Comments
Comment #1
merlinofchaos CreditAttribution: merlinofchaos commentedWill this work?
Comment #2
dvessel CreditAttribution: dvessel commentedHere's another. The 'override preprocess functions' key works as a flag. Just like before but it doesn't check the cached version.
A few things were cleaned up too. The theme registry was littered with empty preprocess function arrays for functions that do not need them. (non templated functions). And I also added an array_unique() to safeguard it against duplicate preprocessors for any given hook. I think this is important since it can easily be overlooked placing more load.
I tested this in and out. It works like a charm.
Comment #3
dvessel CreditAttribution: dvessel commentedNot sure what happened. Did a fresh checkout and the previous patch wasn't doing it's job.
A bit more simple. doesn't try to clean the empty arrays.
Here's another. Seriously, tested and definitely works.
Comment #4
moshe weitzman CreditAttribution: moshe weitzman commentedthis is some confusing mojo.
Comment #5
dvessel CreditAttribution: dvessel commentedmosh, yeah I know. I'll make it clear inside the docs. Implementing isn't too complicated, just the underlying theme registry is but most theme dev's don't need to know all the details on that.
Comment #6
merlinofchaos CreditAttribution: merlinofchaos commentedMoshe: Yes, this is a little confusing. However:
1) This is an advanced power-users feature. This is only going to be used by really advanced users who want to be able to reduce the amount of code that's run on theme('page'), mostly. Otherwise, it will generally have no effect whatsoever, and 99.9% of the installations will never want nor need this feature.
It's analogous to creating your own theme_page() so that you didn't run everything that was in phptemplate_page() for whatever reason.
2) I apparently implemented it broken.
3) This fixes it.
Tested this out, and it does the job.
Comment #7
Dries CreditAttribution: Dries commentedI agree with Moshe that this is hairy. The PHPdoc doesn't really help me understand this -- it gives me a vague hint at best.
(Are we _sure_ we need this ability? What preprocess functions are so expensive that we might want to alter them? Is this driven by a concrete use-case or are we just over-engineering this? I'd be interested to learn more about dvessel's use case ...)
Comment #8
dvessel CreditAttribution: dvessel commentedDries, due to the additive nature of variable functions, we can no longer override them. In Drupal 5, it was a simple process.
Some uses for this:
Add more style sheets before drupal_get_css() is called from template_preprocess_page(). Doing it after creates tons of preprocessed style sheets. Doubles on each page load and each page can change its list of style sheets creating even more. On top of that, if color.module is implemented into the theme, we get even more. drupal_get_css() ideally should be called once since it does so much.
By overriding the *order* of the preprocessor functions, it can be overcome.
In 5 it was solved with this:
Similarly, altering $show_blocks can be intercepted in the same way. I've seen themers who needed it to be shown on all pages. Without this override in the registry, it will be out of our hands.
As merlinofchaos mentions, specific hooks like "page" can remove the default preprocessor. I can see this being useful to developers of theme engines. PHPTemplate makes a lot of assumptions on the output. Sidebars blocks and whatnot can be redone. Engines can bypass that completely and provide something very different.
This feature was already implemented but in a broken state. It doesn't affect most themers so why not?
Comment #9
merlinofchaos CreditAttribution: merlinofchaos commentedIt's worth noting that I made dvessel work pretty hard to convince me to put this feature in. =)
Comment #10
dvessel CreditAttribution: dvessel commentedI hope this can go in. I'll roll another after this patch goes in. http://drupal.org/node/165343
Comment #11
dvessel CreditAttribution: dvessel commentedDries, I can see plenty of uses for this. I hope you reconsider.
Another thing I had in mind is to line up a whole list of custom preprocessors instead of having functions calling other functions. With the new theme features patch that went in, I can supply a ui on the theme and have the registry build a custom list of preprocessors depending on what features are selected.
Comment #12
merlinofchaos CreditAttribution: merlinofchaos commentedIt's also important to note that this feature already exists...but went in broken. So this IS a bug fix of an existing feature.
Comment #13
dvessel CreditAttribution: dvessel commentedHeck, it's such a trivial patch so marking RTBC.
Functionality is already built in. This just fixes it. :)
Comment #14
Gábor HojtsyOK, you added the docs which Dries requested and this is really an existing faeture, although possibly only used by a small minority. Committed.
Comment #15
dvessel CreditAttribution: dvessel commentedThank you. :)
Comment #16
(not verified) CreditAttribution: commentedComment #17
moshe weitzman CreditAttribution: moshe weitzman commentedGiven that we now have a drupal_alter() operation on the theme registry, can this "feature" now be dropped? I would think so. I ask here becuase the ones talking here are surely the only ones who know the answer.
Comment #18
merlinofchaos CreditAttribution: merlinofchaos commentedsince themes can't use drupal_alter(), I would say no.
Comment #19
moshe weitzman CreditAttribution: moshe weitzman commentedbut since this is such a exotic need, i think we could recommend that sites use a module for this. you characterized it as a high performance tweak earlier, so we aren't talking about vanilla drupal sites.
Comment #20
merlinofchaos CreditAttribution: merlinofchaos commentedLet's see what dvessel says there; it was his feature request.
Comment #21
dvessel CreditAttribution: dvessel commentedYeah, it's not necessary any longer. drupal_alter can do everything this feature did. But...
I would prefer to have this functionality directly inside themes since it is after all a theming feature and publishing a theme with this enabled would mean pointing to another module. Theming engines can use this too and it opens a lot of possibilities there also.