Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
While searching for the hook_menu invocation, I came across something pretty weird: Rather than module_invoke, menu.inc pastes together $module . '_menu'
and calls call_user_func.
I was almost completely sure that is the wrong way to do it, and I wanted to find out why on Earth we do it.
Turns out that this goes all the way back to May 2007 and #140218: Make Drupal smaller: move page callbacks to their own include files.
- $callbacks = module_invoke_all('menu');
+ // We need to manually call each module so that we can know which module a given item came from.
+ $callbacks = array();
+ foreach (module_implements('menu') as $module) {
+ $items = call_user_func($module . '_menu');
+ if (isset($items) && is_array($items)) {
+ foreach (array_keys($items) as $path) {
+ $items[$path]['module'] = $module;
+ }
+ $callbacks = array_merge($callbacks, $items);
+ }
+ }
So I want to ask, is there any reason we don't use module_invoke($module, 'menu') here?
Comment | File | Size | Author |
---|---|---|---|
#8 | 626134-8.patch | 1.01 KB | pfrenssen |
#5 | 626134.patch | 1.51 KB | RobLoach |
#1 | drupal-menu-inc-module_invoke-626134-1.patch | 698 bytes | cburschka |
Comments
Comment #1
cburschkaHere is a patch to change this.
Comment #2
moshe weitzman CreditAttribution: moshe weitzman commentedWhats the advantage of module_invoke() here? All it does is make sure that the module implements the specified hook but we already know it does - thats the point of module_implements(). So, I consider module_invoke() slightly less direct and thus less desireable. IMO, this is By Design
Comment #3
RobLoach#1: drupal-menu-inc-module_invoke-626134-1.patch queued for re-testing.
...... I'd opt to use the API Drupal provides rather then relying on call_user_func(). Or maybe switch to a $function() call?
Comment #4
Damien Tournoud CreditAttribution: Damien Tournoud commentedTraditionally, we do that:
So this implementation is slightly inconsistent. We could fix this, but for sure there is no point in using module_invoke() here.
Comment #5
RobLoachSo let's move to the $function() call. This patch also does two other things:
Comment #6
Damien Tournoud CreditAttribution: Damien Tournoud commentedLet's keep the type checking here. There are enough ways to brick a site during development, we don't need to add some more.
Comment #7
catchNeeds work per #6.
Comment #8
pfrenssenUpdated the patch with the suggestion in #6.
Comment #9
Niklas Fiekas CreditAttribution: Niklas Fiekas commentedWhat is the argument here? Doesn't this change what is actually returned?