Issue status update for http://drupal.org/node/27231 Post a follow up: http://drupal.org/project/comments/add/27231 Project: Drupal Version: cvs Component: base system Category: feature requests Priority: normal Assigned to: drumm Reported by: drumm Updated by: drumm Status: patch Attachment: http://drupal.org/files/issues/errorhandling_1.patch (8.75 KB) Updated patch using theme_placeholder() where appropriate. drumm Previous comments: ------------------------------------------------------------------------ Thu, 21 Jul 2005 22:43:16 +0000 : drumm Attachment: http://drupal.org/files/issues/screenshot2.png (96.1 KB) Screenshot. ------------------------------------------------------------------------ Thu, 21 Jul 2005 23:21:31 +0000 : drumm Attachment: http://drupal.org/files/issues/screenshot_10.png (88.44 KB) ------------------------------------------------------------------------ Thu, 21 Jul 2005 23:25:57 +0000 : drumm Attachment: http://drupal.org/files/issues/druplicon.png (3.98 KB) New Druplicon which will need to replace misc/druplicon.png. The current file in that location is a giant flat druplicon which isn't used anywhere. ------------------------------------------------------------------------ Thu, 21 Jul 2005 23:26:26 +0000 : drumm Attachment: http://drupal.org/files/issues/maintenance.css (329 bytes) Some CSS ------------------------------------------------------------------------ Thu, 21 Jul 2005 23:30:34 +0000 : drumm Attachment: http://drupal.org/files/issues/errorhandling.patch (8.47 KB) This patch makes the database errors which occur when the database connection information is wrong or the database isn't up much nicer and more explanatory. To do this it adds a themeable function, theme_maintenance_page(), and a way to get Drupal running without a theme but the ability to execute themeable functions in the usual way. This means we can use the same code to give upgrade.php a nicer look in the future. ------------------------------------------------------------------------ Fri, 22 Jul 2005 06:33:57 +0000 : kbahey 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! ------------------------------------------------------------------------ Fri, 22 Jul 2005 06:52:30 +0000 : gordon +1, This looks great. Get this in and then we can do something with the upgrade.php system and it will look just great. ------------------------------------------------------------------------ Fri, 22 Jul 2005 19:36:39 +0000 : drumm Attachment: http://drupal.org/files/issues/errorhandling_0.patch (8.44 KB) New patch with a couple suggestions from chx. * $foo === NULL is faster than is_null($foo) * Removed 'bootstrap' from the function name since it is not really part of the bootstrap system. When testing/applying updates #2 and #3 of this issue need to be handled too. ------------------------------------------------------------------------ Fri, 22 Jul 2005 20:47:18 +0000 : stefan nagtegaal 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 [1]. 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.. [1] http://drupal.org/node/258 ------------------------------------------------------------------------ Fri, 22 Jul 2005 23:27:53 +0000 : drumm The theme_message() which stefan nagtegaal looks like it should be four separate themeable functions, one of which already exists. I think any such modifications are outside the scope of this patch. I am nearly finished preparing a patch to update.php which uses this patch, but I can not post it until this patch is resolved. ------------------------------------------------------------------------ Sat, 23 Jul 2005 09:35:03 +0000 : stefan nagtegaal 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..