Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
For new module developers hooks are a bit confusing to figure out how they work. One helpful bit is when the hooks show _FORM_ID_ or THEME_ the identify the parts of the hook that need to be changed for your module or theme.
Proposed resolution
Id propose making all the hook_ to HOOK_ to promote this visual modifiable part of the identifier throughout core.
Before:
hook_entity_type_alter
After:
HOOK_entity_type_alter
Remaining tasks
Discuss any drawbacks.
Create a meta and add to coder review.
User interface changes
n/a
API changes
n/a
Comment | File | Size | Author |
---|---|---|---|
#4 | rename-HOOK-2240119-4.patch | 799.26 KB | Jon Pugh |
#1 | rename-HOOK-2240119-1.patch | 136.24 KB | Jon Pugh |
Comments
Comment #1
Jon Pugh+1 to that! Makes it much more consistent.
Attached is a quick search and replace.
Comment #2
Jon PughComment #3
joelpittetThat's awesome @Jon Pugh! Is there any implements hook_... to change as well?
Comment #4
Jon PughYep, a few ;)
Did a final replace of all instances of hook_, there are many @see and other mentions in comments.
Left any instances of `_hook_`, as those functions shouldn't be changed.
So for example:
Comment #5
Jon PughThis is a big patch so I made a Pull Request on my mirror on GitHub, in case anyone wants to review there:
https://github.com/jonpugh/drupal/pull/1/files
Comment #6
joelpittetOh wow, that is a lot, and is green too! Thanks a lot again @Jon Pugh.
Hope to get some people and especially @jhodgdon's thoughts on this change?
Comment #7
jhodgdonMy opinion is NO PLEASE NO.
Why?
a) We've been doing it the other way for as far as I know, the entire history of Drupal.
b) The Hooks topic on api.drupal.org already clearly explains how to implement a hook.
https://api.drupal.org/api/drupal/core!includes!module.inc/group/hooks/8
c) There are also a ***lot*** of drupal.org docs pages explaining this, and many pages that will have to be updated if all the hook function names change, not to mention many books on Drupal programming.
d) A lot of links to api.drupal.org will be wrong because the hook names changed.
e) Pages on api.drupal.org for the D7 and D6 hooks will now not show links to the corresponding D8 hooks, because the names changed.
So I think this should be closed as "works as designed".
Comment #8
joelpittetThanks for the details @jhodgdon on what would have to change for this to get in and the gut reaction.
In response to the whys?
a) Because it has done like this, doesn't mean it should remain like this... obviously. And there is inconsistencies with THEME, FORM_ID, MENUNAME, etc and other uppercased replaceable tokens within the hook system. The issue that triggered this issue was I noticed some fixes for THEME and MENUNAME in #1533104: Make bartik theme pass Coder Review and wondered why it wasn't consistent accross core.
b) No where in that document does it explain that you should replace HOOK and no reason given why FOO is replaced and not BAR. So that is more of a case and point rather than an example of good documentation of how hooks work.
c) Adding documentation to explain the documentation doesn't help the issue. Maybe there should be less docs and more concise and intuitive instead of having to over explain the "how and why" of HOOKs?
d) Thanks for bringing this up, I would have never though of that. I've seen slight name changes show up in the API docs before maybe this could be automated away?
e) Same response as D.
Comment #9
jhodgdonOne more, definitely serious objection:
f) If you want to talk about consistency, then for consistency with other places we use all caps, like _FORM_ID_ or THEME_, you would need to use MODULE_, not HOOK_, because it is being replaced by the module name, not a "hook" name.
In fact, we are already using _HOOK_ to mean "the theme hook" and _MODULE_ to mean "the module short name" in:
https://api.drupal.org/api/drupal/includes!theme.inc/function/theme/7
which is on a topic in D8:
https://api.drupal.org/api/drupal/core!modules!system!theme.api.php/grou...
So you would need to change, for instance, hook_help() to be called MODULE_help(), if you want to maintain this consistency and actually give module developers something obvious to know what to replace.
Which means the hooks wouldn't even be called hook_something at all.
Comment #10
alexpottMy initial gut on reading the issue title was "hmm, makes sense". But, as @jhodgdon outlines this change affects far more than just code and the consistent thing would be to replace hook_ with something else - MODULE_, THEME_, EXTENSION_ are possibilities. But given where we in the release cycle and where effort needs to be focussed I think we should discuss this again for d9.
Comment #11
catchSince this is documentation I think we could make it optional in a minor version and deprecate hook_ - api.drupal.org would need to support both until 9.x.
Whether we want to actually make the change is a different matter, but moving to 8.1.x since it's not impossible to do there.
Comment #12
jhodgdonJust as a note, api.d.o will need to support both hook_ and whatever is eventually decided on here, for the forseeable future. The reason is that api.d.o does not know about coding standards changes; its code must be able to display pages for Drupal 7, 6, 5, 8, 9, etc. with the same logic. That said, changing it to support MODULE_ or EXTENSION_ as well has hook_ will not be difficult. There's just a small bit of logic that says "If this function name starts with hook_ then try to find implementations by pattern matching on the rest of the name". That would just need a slight bit more to say "starts with hook_ or ...". Just a few lines of code would change.