Currently it goes like this:
<?php
foreach ($trace as $key => $value) {
$rich_trace[$value['function']] = $value;
}
?>Better would be:
<?php
foreach ($trace as $key => $value) {
$rich_trace[$key . ': ' . $value['function']] = $value;
}
?>This way we get unique array keys, even for repeated functions.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | devel.ddebug_backtrace.d6.patch | 797 bytes | donquixote |
| #8 | devel.ddebug_backtrace.d7.patch | 785 bytes | donquixote |
| #3 | ddebug_backtrace.0.png | 9.75 KB | salvis |
Comments
Comment #1
salvisI don't have the context here...
What's the key of $rich_trace used for? What is in $key?
Will this have any effect on what is displayed? Screenshot?
Comment #2
donquixote commentedAh, sorry. I should have explained further.
http://drupalcontrib.org/api/function/ddebug_backtrace
The problem is that $value['function'] is not unique and thus not suitable as an array key.
Say you have a stack trace of
index.php -> foo_a(1) -> foo_b(2) -> foo_c(3) -> foo_b(4) -> foo_c(5) -> foo_d(6)If you use the function name as a key, this will collapse to
index.php -> foo_a(1) -> foo_b(4) -> foo_c(5) -> foo_d(6)because foo_b(4) overwrites foo_b(2).
$rich_trace is the array that will be displayed by ddebug_backtrace().
$key is a simple number, afaik, with the benefit of being unique.
Comment #3
salvisOk, I agree that this needs to be fixed. I provide the screenshot and you provide the patch file (and set the status to CNR when it's attached).
Can we avoid showing the numbers? They provide no information and add a bit of confusion because they go the wrong way.
Comment #4
donquixote commentedCould we simply reverse the numbers?
I think they do provide some information: It is sometimes interesting to know how many stack levels we have.
patch: You will have to wait a bit for that. But it would be not that different from the code I posted in the topic starter.
Comment #5
salvisYes, reversing the numbers would work for me.
Comment #6
moshe weitzman commentedI am OK with the proposed fix. Either ascending or descending is fine with me.
Comment #7
salvisWe're still waiting for donquixote's patch...
An alternative to numbering would be to add an (invisible) <span> tag with a class attribute containing an incrementing number. But I agree that visible numbers might be useful on some occasions, but the counting should start at menu_execute_active_handler() and not the other way around.
Comment #8
donquixote commentedYou are obsessed about patches..
Comment #9
donquixote commentedSome variables renamed:
"$call" instead of "$value". The items in a stack trace are function calls.
"$i" instead of "$key". The keys in the original $trace array are numeric.
Comment #10
salvisThe D7 coding standards prescribe spaces on both sides of the concatenation (dot) operator.
If I was obsessed by patches, I'd ask you for an updated one... :-)
On the receiving end, patches make my life easier, especially with Dreditor and an IDE like Eclipse. Try them...
Committed to D7 and D6, thanks! On to #919260: ddebug_backtrace should have a $return option like print_r...
Comment #11
donquixote commentedhehe, i saw it coming .. :)
i like to have the dot directly on the quote.
(off-topic)
My problem with patching is that I usually do the bugfixes on whatever project i am currently working on, with ssh+gedit, on a server somewhere. But the cvs checkout that i need to make the patch is on my local machine. Thus, i need to checkout the latest -dev in cvs on my local machine, then manually apply the changes there, and create and upload the patch. Or, drush dl the latest dev on a test install somewhere, apply the changes, test it, then manually apply it to the cvs checkout on my local machine. With the number of bugs I report on d.o., this is a serious extra effort, compared to just explaining what is wrong in the code.
On the other hand, on my own projects, I usually don't care about patches, since most often I have my own ideas how something should be implemented. Better if someone explains what I need to change, and why.
(/off-topic)
Comment #12
salvis[Oh, I won't deny that preparing a patch is more work than just dropping a few lines of code, but the key to getting fixes committed is to make it as easy as possible for the committers to do their work. Getting a patch has lots of advantages, even if you don't commit it verbatim.]
Comment #13
donquixote commentedCan you make a new official D6 release with the patch?
Comment #14
salvisNo, only Moshe can...