Posted by jonathan1055 on May 20, 2011 at 6:16am
3 followers
| Project: | Devel |
| Version: | 6.x-1.x-dev |
| Component: | devel |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
Hello,
The current devel error handling options let you choose between the nice backtrace and the standard drupal message area, but I have had situations where I could really have done with both. Some themes do not display the backtrace the same in every situation and depending on what the page is doing it has been very useful to get both. Attached are a couple of screen shots.
I will add the patch too, and you can consider whether you like it. Obviously I will make a D7 patch too, but I was working on a D6 site when I needed this.
Jonathan
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| new error option - admin page.jpg | 89.3 KB | Ignored: Check issue status. | None | None |
| new error option - example.jpg | 505.73 KB | Ignored: Check issue status. | None | None |
Comments
#1
Yes, I've made the same experience. Please provide a patch for D7 first.
#2
Here is the patch for D7 as requested, against the dev of 8th April. There are 3 chunks in .module and 1 in .admin.inc
Jonathan
#3
Please don't name it _d7, because that keeps the testbot from picking it up...
#4
Sorry. I thought that _d7 was ok and it was -d7 which avoided the test bot. My mistake.
#5
+++ devel.module 2011-05-21 10:33:41.000000000 +0100@@ -634,6 +636,11 @@ function backtrace_error_handler($error_
+ drupal_set_message(t('%error: %message in %function (line %line of %file).', $variables), $type[1]);
$type[1] is the error level (E_WARNING for example, which is 4), not the drupal_set_message() type ('warning' in this case).
+++ devel.admin.inc 2011-05-21 09:25:48.000000000 +0100@@ -113,6 +113,7 @@ function devel_admin_settings() {
+ $form['devel_error_handler']['#options'][DEVEL_ERROR_HANDLER_BACKTRACE_AND_MESSAGE] = t('Backtrace plus standard drupal message');
Please spell "Drupal" with a capital "D" (and correct the other option, too).
#6
Yes, sure. Both messages now have capital D, and $type[1] replaced with 'error' as it is a real error condition not just a warning. I should have checked that.
#7
Depending on the error_level() there could be warnings or notices, too. I'm sure someone will complain if we display these in bright red...
($type[1] <= WATCHDOG_ERROR ? 'error' : 'warning')
should do the trick (untested!).
#8
I realised that what we actually need is a conversion from the error_level to the drupal_set_message status value, so that the correct status and styling are used. I've added that into this patch, using a simple array
<?php$message_status = array(WATCHDOG_ERROR => 'error',
WATCHDOG_WARNING => 'warning',
WATCHDOG_NOTICE => 'warning',
WATCHDOG_DEBUG => 'status');
?>
The actual values can be changed, this was just my initial mapping.
#9
Yes, that would be the ultimate perfect solution, but it's too involved for this little feature, IMO. I think we'll be fine with WATCHDOG_DEBUG formatted as 'warning' (just not as 'error').
#10
Sorry, I seem to have forgotten about this. Yes, take your point about being too involved. Here is a patch with your simpler translation. Patched against dev of 28th Sept.
#11
The last submitted patch, _1163356_10.devel_.error_handler_option.patch, failed testing.
#12
Pushed to D8 and D7, thanks!
Please reopen if you want to post a patch for D6, too.
#13
Hmm, that's funny! Until a few minutes ago, #10 was yellow (waiting), and now that I've pushed the patch (with a few minor tweaks), it just turned red.
Obviously, the patch cannot be applied a second time, but the timing is surprising...
#14
From the error:
Yes I made the patch with
diff -upand I tested it by usingpatch -b -p0There is obviously a better way I should be using, so that it is consistent with the new testbots. I'll have a look round to see if I can find it, unless you can tell me here ;-)
Yes I will do a D6 patch too.
#15
I'm not up to speed with the subtleties of patching, I just use
git diffand never had a problem.I didn't have any problem applying your patch with
git applyeither, for that matter.#16
Here is the patch ported for D6, created against the 1.26 dev dated 13th Sept 2011.
There are many developers still working on existing D6 sites, so I hope you can commit this. Thanks
Jonathan
#17
The last submitted patch, _1163356_16.devel_.error_handler_option.patch, failed testing.
#18
My patch failed to apply because the testbots do not accept p0 style patches any more, they only accept p1 style, as I found out via http://groups.drupal.org/node/140204
I do not have git installed, so does anyone know how I can make the required patches using diff?
Jonathan
#19
Manually editted patch file, just to see if this gets applied ok.
#20
Pushed to the -dev version, thanks!
#21
Thank you.
#22
Automatically closed -- issue fixed for 2 weeks with no activity.