gotcha: menu wildcards, foreach() and MENU_SUGGESTED_ITEM
Hello, I just noticed a bug in directory.module, and I see the exact same bug in tagadelic.module. This bug involves the new menu system. In D5, we used to be able to dynamically create a series of menu items with a foreach loop within hook_menu(). When upgrading to D6, the tendency has been to remove the foreach loop, and replace the arg(n) with a wildcard loader, but it is the wrong solution. In most cases, you don't want to remove the foreach() loop you had in D5. Within D6, it is perfectly ok to have a loop, recursively declaring several well defined menu items, e.g. one item for each vocabulary. When you have such a loop, do not use the wildcard loader. For a detailed example see this issue: http://drupal.org/node/283198 I have updated the documentation in the handbook. Blessings, Augustin.
Hi again, On the issue, crell write: " A foreach loop inside a D6 menu handler is 99% of the time the WRONG way to do it." http://drupal.org/node/283198 Have I stumbled upon the 1% of cases where it's the right thing to do? If you agree with crell and if I am wrong, I certainly would like to know it, and I'll revert the documentation changes I made. What is the proper way to have MENU_SUGGESTED_ITEM with wildcards and without a loop? See the issue for code examples. Thanks, Augustin.
On Wed, 16 Jul 2008 14:15:05 +0800, "augustin (beginner)" <drupal.beginner@wechange.org> wrote:
Hi again,
On the issue, crell write:
" A foreach loop inside a D6 menu handler is 99% of the time the WRONG way to do it." http://drupal.org/node/283198
Have I stumbled upon the 1% of cases where it's the right thing to do? If you agree with crell and if I am wrong, I certainly would like to know it, and I'll revert the documentation changes I made.
What is the proper way to have MENU_SUGGESTED_ITEM with wildcards and without a loop? See the issue for code examples.
Thanks,
Augustin.
I will defer to chx and pwolanin on this front. I am by and large quoting chx here, who said something very similar to me back in February. I won't say you have definitely not found that 1%, but the whole point of all the fancy math in the menu system is to not have to loop but to simply declare matchable patterns. Looping should be very strongly discouraged. --Larry Garfield
In most cases, you DO want to remove the foreach() loop you had in D5.
there, I fixed for you :)
Within D6, it is ALMOST NEVER NEEDED TO HAVE a loop, recursively declaring several well defined menu items, e.g. one item for each vocabulary. When you have such a loop, DO use the wildcard loader.
there, I fixed for you :) You forgot about the distinction of router items and menu links. You will want to use a single router item and save several links with menu_link_save as you see fit. Regards Karoly Negyesi
On Thursday 17 July 2008 12:02:08 Karoly Negyesi wrote:
You forgot about the distinction of router items and menu links. You will want to use a single router item and save several links with menu_link_save as you see fit.
Ah, yes! Not knowing too much about the internals, I was looking at it from the perspective of a module developer and what was possible using hook_menu() only. Thank you very much for your feedback. I have updated the issue with a copy of your comment: http://drupal.org/node/283198 And since this problem is not particularly obvious, I have updated my handbook edits to correspond to the sanctioned solution: http://drupal.org/node/103114 http://drupal.org/node/109153 Thanks again to Larry and Karoly for their comments. Blessings, Augustin. P.S. to Larry: I guess I *still* need my beginner handle for a while longer, after all... ;)
Menu module is one example in core where several links are created and added to the navigation menu. For some things like node types, we probably could have done the same (rather than using a foreach) but we had also hit the "don't fix it if it works" phase of the core cycle. -Peter On Thu, Jul 17, 2008 at 12:28 AM, augustin (beginner) <drupal.beginner@wechange.org> wrote:
On Thursday 17 July 2008 12:02:08 Karoly Negyesi wrote:
You forgot about the distinction of router items and menu links. You will want to use a single router item and save several links with menu_link_save as you see fit.
Ah, yes! Not knowing too much about the internals, I was looking at it from the perspective of a module developer and what was possible using hook_menu() only.
Thank you very much for your feedback.
I have updated the issue with a copy of your comment: http://drupal.org/node/283198
And since this problem is not particularly obvious, I have updated my handbook edits to correspond to the sanctioned solution: http://drupal.org/node/103114 http://drupal.org/node/109153
Thanks again to Larry and Karoly for their comments.
Blessings,
Augustin.
P.S. to Larry: I guess I *still* need my beginner handle for a while longer, after all... ;)
participants (4)
-
augustin (beginner) -
Karoly Negyesi -
Larry Garfield -
Peter Wolanin