Nice DB error screens

I was about to do something similar on a less ambitious scale (mainly the connect function), because what we get now is ugly and looks unprofessional.

+1 from me for this. Great job!

+1, This looks great. Get this in and then we can do something with the upgrade.php system and it will look just great.

I indeed like the patch! you have some perfectly helptexts which asure that people *must* be foulish when they do not know how to setup drupal the correct way. On that, a very, very fine patch..

Though, code-wise I have one nit-pick, which is propably not even a real issue. Only, I think it looks better:

+    print theme('maintenance_page', '<p>We were able to connect to the MySQL database server (which means your username and password is okay) but not able to select the database.</p>
+<p>The MySQL error was: <em>'. mysql_error() .'</em>.</p>
+<p>Currently, the database is <code>'. substr($url['path'], 1) .'

. The username is '. $url['user'] .' and the database server is '. $url['host'] .'.

+

+
• Are you sure you have the correct database name?

+

• Are you sure the database exists?

+

• Are you sure the username has permission to access the database?

+

+

For more help, see the Installation and upgrading handbook. If you are unsure what these terms mean you should probably contact your hosting provider.

');

Wouldn't it be nicer (code-wise) todo:

    $message[] =  'We were able to connect to the MySQL database server (which means your username and password is okay) but not able to select the database.';
  $message[] = strtr('The MySQL error was: %error.', array('%error' => theme('placeholder', mysql_error())));
  $message[] = etc, etc;
  print theme('maintenance_page', $message);

Also would I encourage you to make a function theme_message($type, $message), like:

function theme_message($type, $message) {
  switch($type) {
     case 'code':
       $output .= '<code>'. $message .'<code>';
       break;
    case 'placeholder':
      $output .= '<em>'. $message .'<em>';
       break;
     case 'important':
      $output .= '<strong>'. $message .'<strong>';
       break;
    default:
      $output .= '<p>'. $message .'<p>';
       break;
  }
}

Which makes theme_placeholder() redundant and could be removed. Codesnippets could be handled in a more consistent way then we currently do (having a theme_placeholder() only is rediculous imo afterall).

Also this function could be used to remove the hardcoded HTML from the helptexts..

The code is untested and needs some love here-and-there. But, when it's done, I think we will create (codewise) more consistency than we currently have!

I look forward to see your idea's and hear your opinion about this..

Okay, I think I have to agree with Neill in here.. If this patch hits the trunk, I'll make a new issue and rolls a patch for introducing new functions which should improve code-wise consistency..

Let me introduce you to:

function theme_placeholder($message) {
  return "<em>$message</em>";
}

function theme_important($message) {
  return "<strong>$message</strong>";
}

function theme_code($message) {
  return "<code>$message

";
}

function theme_text($message) {
return "

$message

";
}

Patches are soon to come in another issue, after this has hit the trunk..

Commited to HEAD.

Basically works nicely, but the function theme_maintenance_page doesn't set the BASE tag, which (at least in my setup with the site not at the root directory) fails to find the CSS files. The patch adds the same BASE tag as is present on normal pages (i.e., $base_url).

That was too quick, I have some more info. The current CVS code does not work when not accessing the start page, e.g. http://drupal/, but something at a deeper level (with search engine friendly URLs turned on) like http://drupal/user/. This case can easily happen with deep links from another site. With the patch attached to my previous post the properly formatted error page appears in this case, too.

Committed drumm's patch.