It's confusing that when menu items are defined, it's called $items, but when they're altered they're called $callbacks.

Needs to be changed in the docs, and anywhere in core that hook_menu_alter() is implemented (unit tests if anywhere, probably).

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

webchick’s picture

fusedpro’s picture

Assigned: Unassigned » fusedpro
Status: Active » Needs review
FileSize
1.13 KB

Changed references in the only hook_menu_alter() reference in core.

Unit tests didn't appear to have any references.

sun’s picture

Issue tags: +Novice
Dave Reid’s picture

There are a few more replacements in menu.inc that we should do as well for consistency:

./includes/menu.inc:1810:      $callbacks = array();
./includes/menu.inc:1817:          $callbacks = array_merge($callbacks, $router_items);
./includes/menu.inc:1821:      drupal_alter('menu', $callbacks);
./includes/menu.inc:1822:      $menu = _menu_router_build($callbacks);
./includes/menu.inc:2410:function _menu_router_build($callbacks) {
./includes/menu.inc:2414:  foreach ($callbacks as $path => $item) {
./modules/menu/menu.api.php:91: * in by reference. Each element of the $callbacks array is one item returned
./modules/menu/menu.api.php:95: * @param $callbacks
./modules/menu/menu.api.php:100:function hook_menu_alter(&$callbacks) {
./modules/menu/menu.api.php:102:  $callbacks['node/add']['access callback'] = FALSE;
webchick’s picture

Status: Needs review » Needs work

Oh, shoot. :) In the meantime, I committed the patch from #2. Thanks, fusedpro!

Want to take it all the way? :)

webchick’s picture

Removing some tags. We had a nice discussion in #drupal on how to highlight these types of issues and "novice" was the winner.

fusedpro’s picture

Status: Needs work » Needs review
FileSize
1.7 KB

Fixed the other references as pointed out by Dave Reid.

sun’s picture

Status: Needs review » Needs work

$items is very ambigious and unclear. We should do the opposite - hook_menu() implementations should use $callbacks, not $items.

webchick’s picture

Version: 7.x-dev » 6.x-dev
Status: Needs work » Reviewed & tested by the community

After talking over this more in IRC, I agree with sun that changing the internal menu system names for these actually makes *that* code less clear. Let's just take this as far as the module-developer-facing APIs, as scoped in the original patch. So I consider this issue closed since #2 was committed to HEAD. Thanks, fusedpro!

Anyone up for a back-port to 6.x?

webchick’s picture

To be clear, someone with a CVS account needs to commit this patch to the DRUPAL-6 branch of contributions/docs/developer/hooks/core.php.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Patch (to be ported)

Does not apply directly to docs/hooks/core.php in 6.x. Needs backport.

Dave Reid’s picture

Status: Patch (to be ported) » Fixed

I went ahead and committed the change to docs/hooks/core.php since any CVS user has commit access to that project.
http://drupal.org/cvs?commit=172153

This is now fixed! Thanks everyone!

Status: Fixed » Closed (fixed)
Issue tags: -Quick fix, -Novice

Automatically closed -- issue fixed for 2 weeks with no activity.