| Project: | Nice Menus |
| Version: | 6.x-2.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
In every other module I've ever worked with, the theme functions are namespaced according to the module's name. theme_imagecache. theme_aggregator_whatever. etc.
This module is violating that rule by name-spacing the theme function as "nice_menu" rather than the module name, "nice_menus." While I guess I can see the argument for this (you're not theming *menus* you're theming *a* menu) this resulted in a 15+ minute bout of smashing my face against the desk thinking I was going completely insane when instead of menu output I was getting *nothing*.
I imagine it will also cause problems if another module ever decides to take the "nice_menu" namespace.
Anyway, this is probably won't fix, but even a wrapper function that calls theme('nice_menu') internally would be extremely helpful for saving the next poor chump these kinds of headaches.
Comments
#1
Oops.
#2
Yeah, this was legacy naming and I didn't want to to screw people up who are already calling it with the existing name. I guess I could use a wrapper function but that seems like extra junk for one letter.I agree that it is annoying. :-/
#3
I was thinking something like this (untested, but to get the idea).
#4
Added to HEAD. Didn't use the patch but the basic idea was used to make the old functions just be a wrapper to the new ones. Also updated all internal theme calls and hook_theme().
#5
Automatically closed -- issue fixed for 2 weeks with no activity.
#6
Please remove the default values in the function calls in the wrapper and replace them with the given variables. This doesn't let you set your own values if you use the deprecated functions.
By the way: The documentation is not up to date as of this issue: http://drupal.org/node/236418
In my opinion the wrapper functions should be removed completely in 2.0. Just add a note to the upgrade docs.
#7
Please review and commit. This one is easy and critical.
#8
Not critical since 2.x is only a dev and not fully baked, so upgrade issues are not critical. People upgrading to 2.x do so at their own risk. That said, I'll try to give this a look soon and get into dev.
#9
Hm, yeah, the more I think about it, I agree that we should probably just remove this and make it an upgrade task. Since I don't intend to maintain it in 7, and that is fairly close at this point, we may as well have people just get it done now. I'll need to write up the upgrade docs for folks before I remove them. We'll see how painful or not it is for testers in the alpha phase and hopefully refine the docs so that by release it is clear what to do.
#10
I have just removed the legacy wrappers from HEAD. The rename info is already in the UPGRADE.txt file, so we just need to make sure that the handbook gets updated and I will add a bold note to the project page when alpha 2 goes out.
#11
Automatically closed -- issue fixed for 2 weeks with no activity.