In #241570: Theme preprocess functions do not get retained when using patterns the 'original hook' index was added to the hook_theme() array so that preprocess functions would carry over to any pattern-based variations of themes. But there's no documentation of this new feature in hook_theme(), which means it's not used in places where it should be.
Comment | File | Size | Author |
---|---|---|---|
#10 | 909954-document-base-hook.patch | 852 bytes | barraponto |
Comments
Comment #1
voxpelli CreditAttribution: voxpelli commentedI agree on this - subscribing.
Comment #2
jhodgdonThis was only I think added to Drupal 7?
Comment #3
voxpelli CreditAttribution: voxpelli commentedIt's very much in Drupal 6 - but not documentated
Comment #4
jhodgdonThen it needs to be documented in Drupal 7 first if it is in Drupal 7, and then the doc fix can be backported to Drupal 6.
Comment #5
jhodgdonI went and read through that referenced issue and the patches that were committed in it.
It looks to me as though the 'original hook' component is something used internally by the theme system, not something you are supposed to use yourself in hook_theme(). Also, in D7 it is apparently called 'base hook' and not 'original hook'.
So I'm not sure whether we really need to document this?
Comment #6
voxpelli CreditAttribution: voxpelli commentedIt's needed for other modules to be able to theme a theme suggestion from another module without losing the original preprocess functions.
That's why Views is suggesting people to use it: http://views-help.doc.logrus.com/help/views/api-default-views
That's why bangpound suggested me to use it in: #890490: Formatter themes don't see preprocessor functions and don't see provider-specific themes.
At least this is true for Drupal 6 - seems like there has been some extra changes made to Drupal 7. Still needed to document this in D7 prior to D6?
Comment #7
voxpelli CreditAttribution: voxpelli commented#34 in #241570: Theme preprocess functions do not get retained when using patterns had the comment for the D7 patch: "It leaves open the ability for a hook_theme_registry_alter() to unset the 'original hook' key"
Sounds like to be able to do that you need to know about the existence of such a property?
In addition merlinofchaos who wrote the original patch flagged the change of 'original hook' to 'base hook' in D7 to be an API-change - something it only was if other modules is supposed to know about the hook.
I can't find anything in the original issue that either explicitly says that 'original hook' shouldn't be documented or implies that it shouldn't.
Comment #8
jhodgdonOK.
Yes, we need to document D7 before we port changes to D6. We're generally trying to avoid having things fixed in D6 that haven't yet been fixed in D7, and that also applies to documentation.
Comment #9
Anonymous (not verified) CreditAttribution: Anonymous commentedFunny, I actually used 'base hook' just last week after discovering it in code, and was wondering why I'd never heard about it before.
At this point, it will be a docs change to D8, then D7, then D6.
Comment #10
barraponto CreditAttribution: barraponto commentedI could really use this documentation since 2009. I've been asking for directions on irc/stackoverflow since I always forgot how to do this. Let me scratch my own itch.
Comment #11
jhodgdonThanks for the patch! It seems to me to be well-written and clear documentation.
But I can't vouch for its accuracy -- if someone who knows more about this than I do could review it for accuracy, that would be good.
Comment #12
JohnAlbinThe Fences module may use this as well. I don't specifically recall.
I can confirm that the "base hook" in hook_theme is indeed what Barraponto documents. So the docs in the patch are correct, though I haven't applied the patch. We should let the testbot have a go at that, but its RTBC from me.
Comment #13
jhodgdonTestbot says go. JohnAlbin says go. I'll get it committed next time I'm doing commits. Thanks!
Comment #14
jhodgdonPatch committed to 8.x and 7.x. Needs backport to 6.x (with change: component is called 'original hook' in D6 apparently).
Comment #15
jhodgdonComment #16
Cameron Tod CreditAttribution: Cameron Tod commentedIf I'm not mistaken, the hook documentation for Drupal 6 is in the documentation project. I've created an issue over there and posted a patch.
#1817140: Backport: document 'original hook' in hook_theme() in Drupal 6
Comment #17
jhodgdonCorrect, thanks! So let's leave this at 7.x, fixed. I'll take care of the other issue.
Comment #19
jay.dansand CreditAttribution: jay.dansand commentedThe current documentation could be improved. On the API page: https://api.drupal.org/api/drupal/core%21modules%21system%21system.api.p... the documentation says only:
I was attempting to implement exactly that - a hook suggestion for
theme_image()
- and the single sentence above left many questions unanswered; for instance, did I need to fully copy the base hook's'variables' => array(...)
in my hook's info? The answer in this case is "yes." But the documentation is entirely unclear.As it turns out, when theme($hook, $variables) (in D7; the steps are basically the same for D8) is called
base hook
can have either no effect or greatly change the outcome oftheme()
. For the following, assumebase hook = 'image'
andhook = 'image__example'
.'image'
in our example) has any$info['includes']
files, they are included.'image'
) does have preprocess or process functions, then:theme_hook_suggestion
is set to'image__example'
(the original$hook
parameter passed totheme('image__example', $variables)
in our example), and$hook
is changed to'image'
(the base hook).'image'
preprocess and process functions are run. Those (pre)process functions have the option to changetheme_hook_suggestion
(where'image__example'
is stored), preventing the original hook from ever firing.'image__example'
remains astheme_hook_suggestion
after the previous step, and if it has$info
defined (which it is guaranteed to have, otherwise$info['base hook']
wouldn't have existed to start this whole process), then$hook
is changed back from'image'
to'image__example'
.I think the documentation should instead say something like:
The proposed documentation would be correct for D8 and D7 (although obviously in D7 both preprocess and process functions are checked on the base hook), and D6 with the caveat that "base hook" was called "original hook".
Comment #20
jhodgdonAdding to this documentation seems like a good idea. Please make a patch. I think maybe more than you suggested should be included -- you seem to have discovered quite a bit about how this works, and much of this would be useful to others to know.
Comment #21
jhodgdonAlthough it would be much better actually to open a new issue instead of reopening this issue -- it is quite old and the original intent was satisfied. Can you do that please?
Comment #22
jay.dansand CreditAttribution: jay.dansand commentedAbsolutely! I just created issue #2106635: Improve documentation of "base hook" in hook_theme(). and attached a patch with more verbose documentation.