Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
During the DBTNG conversion (and this was not done by Berdir, alas) we got some superb ugly code in menu.inc. Begone.
Comment | File | Size | Author |
---|---|---|---|
#13 | menu_is_array.patch | 2.11 KB | chx |
#11 | menu_is_array.patch | 1.48 KB | chx |
#4 | menu_is_array.patch | 871 bytes | chx |
#2 | menu_is_array.patch | 1.61 KB | chx |
menu_is_array.patch | 1.6 KB | chx | |
Comments
Comment #2
chx CreditAttribution: chx commentedComment #4
chx CreditAttribution: chx commentedA more elegant patch thx DamZ.
Comment #6
chx CreditAttribution: chx commentedComment #7
Damien Tournoud CreditAttribution: Damien Tournoud commentedYay for code elegance.
Comment #9
Dries CreditAttribution: Dries commentedI'm undecided. First I thought #4 was uglier than #2. Then I decided it was just ugly to pass in a database result object as a parameter. ;-)
Other opinions?
Comment #10
Crell CreditAttribution: Crell commentedWell #2 just flat out doesn't work anyway because the API doesn't work like that. :-) #4 is an improvement over the current HEAD, but I agree with Dries that passing a database result object around is ugly in and of itself.
Also, the preferred way to specify the fetch mode is in the calling function (db_select() in this case), not on the result set after it's been retrieved. setFetchMode() is there because it's part of PDO, and it supports some more esoteric fetch options that as of yet I haven't found a use for but PDO allows. :-)
Comment #11
chx CreditAttribution: chx commentedRefactoring the whole shebang is not this issue. This is just removing some ugly code.
Comment #13
chx CreditAttribution: chx commentedmenu.admin.inc is already converted(!)
Comment #14
Crell CreditAttribution: Crell commentedI still agree with Dries that passing around a result set is ugly. But the patch in #13 is an improvement over what's in HEAD right now, so RTBC.
Comment #15
quicksketchI fix the passing around a database object in #483614: Better breadcrumbs for callbacks, dynamic paths, tabs. However, we can get this committed and then we can fix passing around the result set there, since I needed to rewrite menu_tree_data() there for other reasons.
Comment #16
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.