backdrop-contrib / devel

Provides helper functions for Backdrop developers.
https://backdropcms.org/project/devel
GNU General Public License v2.0
10 stars 13 forks source link

Follow-up to updating the Krumo library #135

Open jenlampton opened 2 years ago

jenlampton commented 2 years ago

This is a follow-up to https://github.com/backdrop-contrib/devel/issues/102

There's obviously more work to do here, like deciding what removed customisations are still needed (and working out how to add them back in, preferably without hacking Krumo again).

From that issue (thank you @BWPanda)

It seems Devel has hacked Krumo and changed a bunch of stuff in Krumo's files - including adding the addCssJs() function.

Here's a diff I made of Krumo v0.2.1a downloaded from their GitHub repo, and Devel's copy (diff -uprwiNEB --ignore-file-name-case krumo-0.2.1a/ krumo/): krumo.diff.txt

ghost commented 2 years ago

One of the main differences I've noticed between this new, clean version and the old, hacked version is the calling function listed at the bottom of the Krumo output.

My limited testing shows it always seems to say Devel is the caller. One of the hacks we had before was to filter the backtrace by skipping functions with 'devel' in the name, obviously for this reason.

Not sure how to address this now. Maybe a feature request to Krumo that adds the ability to specify additional backtrace skips...? (it already skips 'krumo' functions).

jenlampton commented 2 years ago

Not sure how to address this now. Maybe a feature request to Krumo that adds the ability to specify additional backtrace skips...? (it already skips 'krumo' functions).

That sounds like a great idea. Maybe someone else has opened a similar issue in their queue before?

docwilmot commented 2 years ago

I tried the latest dev and did a dpm() on a views row $fields variable and keep getting Error: Maximum function nesting level of '256' reached, aborting! in is_callable() (line 1109 of modules\devel\lib\krumo\class.krumo.php), which doesnt happen if I revert to previous version

argiepiano commented 2 years ago

I can confirm this problem dumping a view structure. To reproduce:

$v = views_get_view('node_admin_content');
$v->set_display('page');
$v->execute();
dpm($v);

You get a 500 error in the browser's console, and the nesting error posted above by @docwilmot.

docwilmot commented 2 years ago

Got the same nesting error yesterday on a FormAPI $variables array. I wonder if this is a more general issue than just Views.

It's much much faster though.

argiepiano commented 2 years ago

Agreed. Perhaps some of the krumo customizations that the upgrade removed were necessary. I'm wondering if it may be worth looking at the current D7 version, which is fast and works well with views and other complex structures.

jenlampton commented 1 year ago

I would love some help with this. Last I looked at the Drupal 7 version it was still using the older customized version of the Krumo library. I'm going to roll back this change until we can resolve some of these issues, and make a new release with the other fixes first.

jenlampton commented 1 year ago

Okay, here is a branch with the krumo update and the linux line-endings fix @quicksketch added: https://github.com/backdrop-contrib/devel/pull/153

We should keep working on this here and can merge once enough people feel it's ready.