Posted by bangpound on September 13, 2010 at 1:21pm
9 followers
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | documentation |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | needs backport to D6, needs backport to D7 |
Issue Summary
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.
Comments
#1
I agree on this - subscribing.
#2
This was only I think added to Drupal 7?
#3
It's very much in Drupal 6 - but not documentated
#4
Then 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.
#5
I 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?
#6
It'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?
#7
#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.
#8
OK.
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.
#9
Funny, 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.
#10
I 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.
#11
Thanks 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.
#12
The 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.
#13
Testbot says go. JohnAlbin says go. I'll get it committed next time I'm doing commits. Thanks!
#14
Patch committed to 8.x and 7.x. Needs backport to 6.x (with change: component is called 'original hook' in D6 apparently).
#15
#16
If 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
#17
Correct, thanks! So let's leave this at 7.x, fixed. I'll take care of the other issue.
#18
Automatically closed -- issue fixed for 2 weeks with no activity.