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

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

salvis’s picture

Yes, I've made the same experience. Please provide a patch for D7 first.

jonathan1055’s picture

Version: 6.x-1.24 » 7.x-1.x-dev
FileSize
1.83 KB

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

salvis’s picture

Status: Active » Needs review

Please don't name it _d7, because that keeps the testbot from picking it up...

jonathan1055’s picture

Sorry. I thought that _d7 was ok and it was -d7 which avoided the test bot. My mistake.

salvis’s picture

Status: Needs review » Needs work
+++ 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).

jonathan1055’s picture

Status: Needs work » Needs review
FileSize
2.7 KB

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.

salvis’s picture

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!).

jonathan1055’s picture

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

$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.

salvis’s picture

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').

jonathan1055’s picture

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.

Status: Needs review » Needs work

The last submitted patch, _1163356_10.devel_.error_handler_option.patch, failed testing.

salvis’s picture

Status: Needs work » Fixed

Pushed to D8 and D7, thanks!

Please reopen if you want to post a patch for D6, too.

salvis’s picture

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...

jonathan1055’s picture

Version: 7.x-1.x-dev » 6.x-1.x-dev
Status: Fixed » Patch (to be ported)

From the error:

This may be a -p0 (old style) patch, which is no longer supported by the testbots.

Yes I made the patch with diff -up and I tested it by using patch -b -p0

There 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.

salvis’s picture

I'm not up to speed with the subtleties of patching, I just use git diff and never had a problem.

I didn't have any problem applying your patch with git apply either, for that matter.

jonathan1055’s picture

Status: Patch (to be ported) » Needs review
FileSize
3.39 KB

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

Status: Needs review » Needs work

The last submitted patch, _1163356_16.devel_.error_handler_option.patch, failed testing.

jonathan1055’s picture

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

jonathan1055’s picture

Status: Needs work » Needs review
FileSize
3.37 KB

Manually editted patch file, just to see if this gets applied ok.

salvis’s picture

Status: Needs review » Fixed

Pushed to the -dev version, thanks!

jonathan1055’s picture

Thank you.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.