Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inconsistent calls #47

Open
wants to merge 1 commit into
base: 6.x
Choose a base branch
from
Open

inconsistent calls #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion includes/batch.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if ($percentage == 100) {
$new_op = 'finished';
}
Expand Down
6 changes: 3 additions & 3 deletions includes/install.pgsql.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ function drupal_test_pgsql($url, &$success) {
// Test UNLOCK, which is done automatically upon transaction end in PostgreSQL
$query = 'COMMIT';
$result = pg_query($connection, $query);
if ($error = pg_result_error()) {
if ($error = pg_result_error($result)) {
drupal_set_message(st('Failed to unlock a test table on your PostgreSQL database server. We tried unlocking a table with the command %query and PostgreSQL reported the following error: %error.', array('%query' => $query, '%error' => $error)), 'error');
$err = TRUE;
}
Expand All @@ -111,7 +111,7 @@ function drupal_test_pgsql($url, &$success) {
// Test DELETE.
$query = 'DELETE FROM drupal_install_test';
$result = pg_query($connection, $query);
if ($error = pg_result_error()) {
if ($error = pg_result_error($result)) {
drupal_set_message(st('Failed to delete a value from a test table on your PostgreSQL database server. We tried deleting a value with the command %query and PostgreSQL reported the following error: %error.', array('%query' => $query, '%error' => $error)), 'error');
$err = TRUE;
}
Expand All @@ -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)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I'll be using that.

drupal_set_message(st('Failed to drop a test table from your PostgreSQL database server. We tried dropping a table with the command %query and PostgreSQL reported the following error %error.', array('%query' => $query, '%error' => $error)), 'error');
$err = TRUE;
}
Expand Down
6 changes: 3 additions & 3 deletions modules/menu/menu.admin.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

@rmuit rmuit Jan 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

$form['#menu'] = $menu;
if (element_children($form)) {
$form['submit'] = array(
Expand All @@ -59,7 +59,7 @@ function menu_overview_form(&$form_state, $menu) {
/**
* Recursive helper function for menu_overview_form().
*/
function _menu_overview_tree_form($tree) {
function _menu_overview_tree_form($tree, $form_state) {
static $form = array('#tree' => TRUE);
foreach ($tree as $data) {
$title = '';
Expand Down Expand Up @@ -111,7 +111,7 @@ function _menu_overview_tree_form($tree) {
}

if ($data['below']) {
_menu_overview_tree_form($data['below']);
_menu_overview_tree_form($data['below'], $form_state);
}
}
return $form;
Expand Down
6 changes: 3 additions & 3 deletions modules/openid/openid.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,14 +245,14 @@ function _openid_signature($association, $message_array, $keys_to_sign) {

function _openid_hmac($key, $text) {
if (strlen($key) > OPENID_SHA1_BLOCKSIZE) {
$key = _openid_sha1($key, true);
$key = _openid_sha1($key);
}

$key = str_pad($key, OPENID_SHA1_BLOCKSIZE, chr(0x00));
$ipad = str_repeat(chr(0x36), OPENID_SHA1_BLOCKSIZE);
$opad = str_repeat(chr(0x5c), OPENID_SHA1_BLOCKSIZE);
$hash1 = _openid_sha1(($key ^ $ipad) . $text, true);
$hmac = _openid_sha1(($key ^ $opad) . $hash1, true);
$hash1 = _openid_sha1(($key ^ $ipad) . $text);
$hmac = _openid_sha1(($key ^ $opad) . $hash1);

return $hmac;
}
Expand Down
2 changes: 1 addition & 1 deletion themes/garland/maintenance-page.tpl.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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); ?>>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?


<!-- Layout -->
<div id="header-region" class="clear-block"><?php print $header; ?></div>
Expand Down
2 changes: 1 addition & 1 deletion themes/garland/page.tpl.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<?php print phptemplate_get_ie_styles(); ?>
<![endif]-->
</head>
<body<?php print phptemplate_body_class($left, $right); ?>>
<body<?php phptemplate_body_class($left, $right); ?>>

<!-- Layout -->
<div id="header-region" class="clear-block"><?php print $header; ?></div>
Expand Down