In case you don't know, Edward Z. Yang (ezyang) is squashing a lot of bugs in coder_format in the scope of a GHOP task currently.
In case you don't know coder_format, it's a script I wrote some time ago which re-formats one PHP file, or recursively all files in a directory according to Drupal's Coding Standards. Doug Green has been so kind to add it to the Coder module, so if you are a Drupal developer, you probably have (an out-dated version of) it already. Unfortunately, it did not receive that much attention yet.
I do not yet want to promote coder_format with this post. Today, I just want to show you the results of an experiment, which in turn will show you, what you can expect from coder_format.
Guess, what this ugly code snippet is:
<?php
function drupal_add_feed($url = NULL, $title = '') { static $stored_feed_links = array(); if (!is_null($url) && !isset($stored_feed_links[$url])) { $stored_feed_links[$url] = theme('feed_icon', $url, $title); drupal_add_link(array('rel' => 'alternate', 'type' => 'application/rss+xml', 'title' => $title, 'href' => $url)); } return $stored_feed_links; } function drupal_get_feeds($delimiter = "\n") { $feeds = drupal_add_feed(); return implode($feeds, $delimiter); } function drupal_query_string_encode($query, $exclude = array(), $parent = '') { $params = array(); foreach ($query as $key => $value) { $key = drupal_urlencode($key); if ($parent) { $key = $parent .'['. $key .']'; } if (in_array($key, $exclude)) { continue; } if (is_array($value)) { $params[] = drupal_query_string_encode($value, $exclude, $key); } else { $params[] = $key .'='. drupal_urlencode($value); } } return implode('&', $params); } function drupal_get_destination() { if (isset($_REQUEST['destination'])) { return 'destination='. urlencode($_REQUEST['destination']); } else { $path = isset($_GET['q']) ? $_GET['q'] : ''; $query = drupal_query_string_encode($_GET, array('q')); if ($query != '') { $path .= '?'. $query; } return 'destination='. urlencode($path); } } function drupal_goto($path = '', $query = NULL, $fragment = NULL, $http_response_code = 302) { if (isset($_REQUEST['destination'])) { extract(parse_url(urldecode($_REQUEST['destination']))); } else if (isset($_REQUEST['edit']['destination'])) { extract(parse_url(urldecode($_REQUEST['edit']['destination']))); } $url = url($path, array('query' => $query, 'fragment' => $fragment, 'absolute' => TRUE)); $url = str_replace(array("\n", "\r"), '', $url); if (!defined('MAINTENANCE_MODE') || MAINTENANCE_MODE != 'update') { module_invoke_all('exit', $url); } session_write_close(); header('Location: '. $url, TRUE, $http_response_code); exit(); } function drupal_site_offline() { drupal_maintenance_theme(); drupal_set_header('HTTP/1.1 503 Service unavailable'); drupal_set_title(t('Site off-line')); print theme('maintenance_page', filter_xss_admin(variable_get('site_offline_message', t('@site is currently under maintenance. We should be back shortly. Thank you for your patience.', array('@site' => variable_get('site_name', 'Drupal')))))); } function drupal_not_found() { drupal_set_header('HTTP/1.1 404 Not Found'); watchdog('page not found', check_plain($_GET['q']), NULL, WATCHDOG_WARNING); if (!isset($_REQUEST['destination'])) { $_REQUEST['destination'] = $_GET['q']; } $path = drupal_get_normal_path(variable_get('site_404', '')); if ($path && $path != $_GET['q']) { menu_set_active_item($path); $return = menu_execute_active_handler($path); } if (empty($return) || $return == MENU_NOT_FOUND || $return == MENU_ACCESS_DENIED) { drupal_set_title(t('Page not found')); $return = t('The requested page could not be found.'); } print theme('page', $return, FALSE); }
?>No, it is not some nasty code that I found somewhere on the net. It's a stripped version of common.inc from Drupal 6 generated with
php -w includes/common.inc > common.stripped.incAnd, it is an example for some PHP code that does not follow Drupal's Coding Standards at all.
Guess again, how this snippet will look after processing it with coder_format. Ready? Go!
<?php
function drupal_add_feed($url = NULL, $title = '') {
static $stored_feed_links = array();
if (!is_null($url) && !isset($stored_feed_links[$url])) {
$stored_feed_links[$url] = theme('feed_icon', $url, $title);
drupal_add_link(array('rel' => 'alternate', 'type' => 'application/rss+xml', 'title' => $title, 'href' => $url));
}
return $stored_feed_links;
}
function drupal_get_feeds($delimiter = "\n") {
$feeds = drupal_add_feed();
return implode($feeds, $delimiter);
}
function drupal_query_string_encode($query, $exclude = array(), $parent = '') {
$params = array();
foreach ($query as $key => $value) {
$key = drupal_urlencode($key);
if ($parent) {
$key = $parent .'['. $key .']';
}
if (in_array($key, $exclude)) {
continue;
}
if (is_array($value)) {
$params[] = drupal_query_string_encode($value, $exclude, $key);
}
else {
$params[] = $key .'='. drupal_urlencode($value);
}
}
return implode('&', $params);
}
function drupal_get_destination() {
if (isset($_REQUEST['destination'])) {
return 'destination='. urlencode($_REQUEST['destination']);
}
else {
$path = isset($_GET['q']) ? $_GET['q'] : '';
$query = drupal_query_string_encode($_GET, array('q'));
if ($query != '') {
$path .= '?'. $query;
}
return 'destination='. urlencode($path);
}
}
function drupal_goto($path = '', $query = NULL, $fragment = NULL, $http_response_code = 302) {
if (isset($_REQUEST['destination'])) {
extract(parse_url(urldecode($_REQUEST['destination'])));
}
else if (isset($_REQUEST['edit']['destination'])) {
extract(parse_url(urldecode($_REQUEST['edit']['destination'])));
}
$url = url($path, array('query' => $query, 'fragment' => $fragment, 'absolute' => TRUE));
$url = str_replace(array("\n", "\r"), '', $url);
if (!defined('MAINTENANCE_MODE') || MAINTENANCE_MODE != 'update') {
module_invoke_all('exit', $url);
}
session_write_close();
header('Location: '. $url, TRUE, $http_response_code);
exit();
}
function drupal_site_offline() {
drupal_maintenance_theme();
drupal_set_header('HTTP/1.1 503 Service unavailable');
drupal_set_title(t('Site off-line'));
print theme('maintenance_page', filter_xss_admin(variable_get('site_offline_message', t('@site is currently under maintenance. We should be back shortly. Thank you for your patience.', array('@site' => variable_get('site_name', 'Drupal'))))));
}
function drupal_not_found() {
drupal_set_header('HTTP/1.1 404 Not Found');
watchdog('page not found', check_plain($_GET['q']), NULL, WATCHDOG_WARNING);
if (!isset($_REQUEST['destination'])) {
$_REQUEST['destination'] = $_GET['q'];
}
$path = drupal_get_normal_path(variable_get('site_404', ''));
if ($path && $path != $_GET['q']) {
menu_set_active_item($path);
$return = menu_execute_active_handler($path);
}
if (empty($return) || $return == MENU_NOT_FOUND || $return == MENU_ACCESS_DENIED) {
drupal_set_title(t('Page not found'));
$return = t('The requested page could not be found.');
}
print theme('page', $return, FALSE);
}
?>Please bear in mind, that php -w strips all comments and white-space from a PHP file.
Now, guess again, what a diff between the original common.inc from Drupal 6 and the code above (which has not been altered) will show you. I won't tell. You might look at the attached patch :)
If you want to support this effort, please help in finding wrong applied formattings by coder_format in the GHOP #136: Improve coder_format to re-format Drupal 6 core without wrong formattings issue, respectively in the corresponding wiki page.
Daniel F. Kudwien
unleashed mind
| Attachment | Size |
|---|---|
| common.inc_.patch_.txt | 7.61 KB |
Comments
Looks good
Does it issue any rewritten lines? or just reformats? Good idea though, more consistent the better, which is defiantly needed in core.
I might have read this wrong for the coder module but I thought it had mentioned concatenation would be without spaces, I find it looks cleaner when you are only concatenating one or two variables but when you get a few going in a single line it looks pretty ugly.
My main beef in core would probably be some of the formats for the menu arrays, yikesss
vision media
350designs
Tj Holowaychuk
Vision Media - Victoria BC Web Design
Victoria British Columbia Web Design School
It re-formats
coder_format rewrites the file it is applied on. If it is applied on a directory using the
--batch-replaceargument, then it additionally creates backup files of all rewritten files. All changes can then be reverted by invoking coder_format again with the--undoargument.See inline documentation of
coder_format.phpfor further information.Regarding string concatenations: Should always be written this way:
<?php// Concatenation of strings:
$var = $foo .'bar'. $baz;
// Concatenation of variables:
$var = $foo . $bar;
// Concatenation of strings and variables:
$var = $foo .'bar'. $baz . $boom;
?>
Daniel F. Kudwien
unleashed mind
Daniel F. Kudwien
netzstrategen