inconsistent calls #47
Conversation
| @@ -42,7 +42,7 @@ function menu_overview_form(&$form_state, $menu) { | |||
| menu_tree_check_access($tree, $node_links); | |||
| $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.)
| @@ -22,7 +22,7 @@ | |||
| <?php print phptemplate_get_ie_styles(); ?> | |||
| <![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?
| @@ -122,7 +122,7 @@ function drupal_test_pgsql($url, &$success) { | |||
| // Test DROP. | |||
| $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.
| @@ -143,7 +143,7 @@ function _batch_progress_page_nojs() { | |||
| print $fallback; | |||
|
|
|||
| // 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.
fix some inconsistent calls (excess/missing arguments)