Conversation
| $menu_admin = FALSE; | ||
|
|
||
| $form = _menu_overview_tree_form($tree); | ||
| $form = _menu_overview_tree_form($tree, $form_state); |
There was a problem hiding this comment.
Interesting...
If your goal is for the code to be less misleading, I'd advocate for the reverse change instead: remove $form_state completely from _menu_overview_tree_form(). This is only called on first form build when $form_state is empty, and on form rebuild / early submit, when $form_state[MLID] is never filled, so it doesn't do anything. (Only $form_state['post'] is ever filled).
(Unless some custom code wants to call menu_overview_form($form_state, $menu) to build a menu. But it seems it's a little late in the D6 development cycle to start doing that.)
As a bonus, that also makes sure there's no behavior change.
(Not that I think it's necessary per se - I have no strong opinion on dead D6 code that doesn't do anything.)
| <![endif]--> | ||
| </head> | ||
| <body<?php print phptemplate_body_class($left, $right); ?>> | ||
| <body<?php phptemplate_body_class($left, $right); ?>> |
There was a problem hiding this comment.
Hmhmhm. This is unfortunate.
Indeed the print statement does not do anything, because phptemplate_body_class() as the only function in template.php has its own print statement. (I'm guessing that was a bug / oversight.)
So I can understand it messes with your static code analysis or IDE.
But wouldn't removing the print statement from here, cause more confusion for human code/template readers, who don't know that phptemplate_body_class() is quasi-buggy?
| $query = 'DROP TABLE drupal_install_test'; | ||
| $result = pg_query($connection, $query); | ||
| if ($error = pg_result_error()) { | ||
| if ($error = pg_result_error($result)) { |
There was a problem hiding this comment.
Thank you. I'll be using that.
|
|
||
| // Perform actual processing. | ||
| list($percentage, $message) = _batch_process($batch); | ||
| list($percentage, $message) = _batch_process(); |
There was a problem hiding this comment.
Blegh. Yes this change is good.
This wonky code is also still present in D7, and this line once cost me a few hours because I was assuming _batch_process() was actually using $batch. Which it isn't.
2 participants
fix some inconsistent calls (excess/missing arguments)