Posted by JohnAlbin on November 5, 2009 at 8:57am
4 followers
| Project: | Zen |
| Version: | 6.x-2.x-dev |
| Component: | PHP code |
| Category: | task |
| Priority: | normal |
| Assigned: | JohnAlbin |
| Status: | needs review |
Issue Summary
drupal_get_filename() is broken; see #341140: drupal_get_filename() when database is down, does not deal with phptemplate themes.
When that is fixed in Drupal 6, replace _zen_path() with drupal_get_path('theme', 'zen').
Comments
#1
#2
#3
drupal_system_listing() is expensive.
I don't understand the exact problem with
drupal_get_path('theme', 'zen'), but following the title of #341140: drupal_get_filename() when database is down, does not deal with phptemplate themes, it sounds like this only applies if the database is down? Also, what exactly happens if you call drupal_get_path() instead of _zen_path() ?My proposal:
We check if the database is there, or if whatever condition applies that allows us to use drupal_get_path().
We could also check if the returned path contains the zen.info file.
Only if this fails, we call drupal_system_listing().
I will create a patch, if someone can tell me what the exact problem is we need to work around.
--------------
For reference, here the current code of _zen_path(): (in 6.x-2.1)
<?php/**
* Returns the path to the Zen theme.
*
* drupal_get_filename() is broken; see #341140. When that is fixed in Drupal 6,
* replace _zen_path() with drupal_get_path('theme', 'zen').
*/
function _zen_path() {
static $path = FALSE;
if (!$path) {
$matches = drupal_system_listing('zen\.info$', 'themes', 'name', 0);
if (!empty($matches['zen']->filename)) {
$path = dirname($matches['zen']->filename);
}
}
return $path;
}
?>
#4
Here is my proposed replacement of _zen_path().
<?phpfunction _zen_path() {
static $_path = FALSE;
if (!$_path) {
if (db_is_active()) {
$_path = drupal_get_path('theme', 'zen');
}
else {
$matches = drupal_system_listing('zen\.info$', 'themes', 'name', 0);
if (!empty($matches['zen']->filename)) {
$_path = dirname($matches['zen']->filename);
}
}
}
return $_path;
}
?>
I could make this a patch, but for now I just want some feedback..
#5
looks good. but if you want actual feedback, from people testing the code and benchmarking, you got to provide a path. but i believe it makes lots of sense.
#6
Here we go.