Using hook_theme() to allow override of display.

Can you give an example how you would use this?

I was looking for overriding a menu block so between the list item and hyperlink, it has a div surrounding it (so i can use sifr to target it easily as sifr seems to have trouble with targetting lists when it has sublists). Something to target between them will allow me to target the sifr css rules better i think

subscribing.

+1, I think this is the way to go.
However, in addition to the $tree parameter, I would also let the theme function know about the menu name and the trail (for recursive calls).

I solved this problem for myself with a custom module that creates a duplicate/mirror for each menu blocks. These new blocks use all the settings of the respective menu block, but are easier to theme or alter. The downside is that my block list gets cluttered up.

If this is to be solved by menu_block itself (which it should), I think the functions that generate the tree should be further split up into reusable pieces:

<?php
function _menu_block_view($delta) {
  list($tree, $settings) = _menu_block_load($delta);
  return theme('menu_block_tree_output', $tree, $settings, array());
}

function theme_menu_block_tree_output($tree, $settings, $trail) {
  $items = menu_block_build_items($tree);
  foreach (array_keys($items) as $key) {
    $subtrail = $trail;
    $subtrail[] = array($key, $items[$key]);
    $below = !empty($tree[$key]['below']) ? theme('menu_block_tree_output', $tree[$key]['below'], $settings, $subtrail) : '';
    $items[$key]['link']['localized_options']['attributes']['class'] = menu_block_build_link_class($item);
    $extra_class = menu_block_build_item_class($item);
    $link = theme('menu_item_link', $items[$key]['link']);
    $output .= theme('menu_item', $link, $items[$key]['link']['has_children'], $items[$key]['below'], $items[$key]['link']['in_active_trail'], $extra_class);
    ++$i;
  }
  return $output ? theme('menu_tree', $output) : '';
}
?>

Something like this.
The point is that a theme override shouldn't need to copy all the details.

That function is the heart of the menu tree building code. If you want to override that, just build a different module that uses some of menu_block's supporting APIs.

Fundamentally, this proposal is not the proper way to "override" the building of the tree. Theming is for changing the way the data is displayed, not changing the way the data is built.

Sorry, guys! :-(

Hey, wait!

Fundamentally, this proposal is not the proper way to "override" the building of the tree. Theming is for changing the way the data is displayed, not changing the way the data is built.

We don't need to do the tree building in a theme function. Instead, extending on the code from #4, we can do the following:
- load the tree.
- send the $tree and $settings through a drupal_alter() hook to allow modifications.
- then call theme('menu_block_tree_output', $tree, $settings, $trail). The tree is already build at this point, but the theme hook would allow to render it in a different way - with more flexibility than the usual theme('menu_item_link') etc.

This would be fundamentally the correct way of doing theme overrides.

Another interesting idea would be callbacks ("handlers") for tree loading and tree rendering. A custom load handler could skip the localize() stuff, or do access checking in a different way, or even merge menu items from two different menus. We would need another alter() hook before the loading, to set the load handler..

For those worried about performance: Doing things for the entire tree is cheap. Doing things for single menu items is expensive. A mytheme_menu_block_tree_output() custom override could be much faster than the usual menu_block_tree_output(), because it doesn't need to call theme('menu_item') and theme('menu_item_link').

That function is the heart of the menu tree building code. If you want to override that, just build a different module that uses some of menu_block's supporting APIs.

I would like to have two modules doing almost the same thing..

For those interested, I can attach the module that I currently use. No guarantees. Works as explained in #4, but with an added alter() hook.

EDIT:
Please ignore the "// Lazy render the tree" comment. This was a funny idea that I discarded for this module.