APi fixes for menu_tree_data() including depth param

We shoudl pull in some of the straigh menu APi fixes from #483614: Better breadcrumbs for callbacks, dynamic paths, tabs to make that issue simpler to deal with.

first pass here - passing an array into function menu_tree_data() since that makes the functions more re-usable

first pass here - passing an array into function menu_tree_data() since that makes the functions more re-usable

+  $result = array_reverse($result);
[...]
+  while ($item = array_pop($result)) {
+  }

I suggest walking the array in the normal order and using unset().

@Damien - but pop is such a nice function name and does just what we want :)

I'm really not such how do do what you suggest in as simple a way. To me this seems ugly and much harder to understand by comparison:

while ($data = each($result)) {
unset($result[$data[['key']);
$item = $data['value'];
}

I chose a reverse and pop because array_unshift() is a rather more expensive operation than pop.

Why not:

foreach ($result as $key => $item) {
  unset($result[$key]);
  // ...
}

@Damien - still rather ugly imho - in either case we need to pass the array by reference when we do the recursion. What happens when we come out of the recursive call and we have unset the the array element that the current foreach loop thinks is the current element? I hope PHP handles that gracefully?

A better pass - allows for radicial simplification of _menu_tree_data()

oops - forgot to use --no-prefix

fixed book and menu module.

I think the problem was that book_menu_subtree_data() had a bug- failed to rename a variable.

remove unneeded $result->setFetchMode() calls, fix comment

If those menu tree data functions would additionally allow to specify a starting menu link instead of a menu name, that would heavily trivialize admin_menu as well as http://drupal.org/project/menu_block (which contains some raw menu_*() forks that should be in core)

more feedback from chx, leading to improved and expanded code comments to clarify the algorithm.

slight additional clarification

Very nice code cleanup! I am surprised this could be simplified so much. I got a headache when coded the original, however, it's not an easy thing, this tree traversal because of the way it is. Some time and a "new" coder helped it :)

Oh, testing bot...

+  $links = array();
+  foreach ($result as $item) {
+    $links[] = $item;
+  }
+  $tree = menu_tree_data($links);

What am I missing here? It seems like we're just making a copy of $result?

@Dries - $reslt is a query result object that implements the Iterator interface so we can call foreach() one it (but not other array functions). By doing the foreach first, we allow the menu_tree_data() function to take a more generic array as a parameter.

fixed a very minor 1-line merge conflict in book.test (where I am fixing a typo in the test that was failing above).

minor doxygen changes per Dries and added type hinting to the arguments of menu_tree_data()

I think this PHPDoc is sufficient. While I too found this confusing, it's not the job of menu API code to document DBTNG, and the docs that talk about a query result object are enough of a breadcrumb for me to go off searching if I need to for further clarification.

This looks like a nice clean-up to make the menu API more comprehensible to mere mortals. :)

Committed to HEAD. Needs updating in the documentation.

menu_tree_data is the only function which needs a conversion between Drupal 6 and Drupal 7 because the depth param is optional.
Documented in http://drupal.org/update/modules/6/7#menu_tree_data

#d7csmtl