@mbabker

When debug parameter is ON, a lot get displayed, besides the recently added backtrace,
the DEBUG console shows a lot
and not no website will really run with it DEBUG on, it makes sense to add this

to me this sound a little confusing ...

IMHO this would be a better to have it on the backtrace ... maybe the numbering also inverted?

I actually prefer the debug plugin Log call back

Yes i had similar thoughts ...

but before anything, we could agree that it is ok if this get displayed when debug parameter is ON:

When debug parameter is ON, a lot get displayed, besides the recently added backtrace,
the DEBUG console shows a lot
and not no website will really run with it DEBUG on, it makes sense to add this

but before anything, we could agree that it is ok if this get displayed when debug parameter is ON:

i see no problem with that.

When debug parameter is ON, a lot get displayed, besides the recently added backtrace,
the DEBUG console shows a lot
and not no website will really run with it DEBUG on, it makes sense to add this

IMO no production website should ever be with debug set to ON in global config or/and the system debug plugin enabled.

The backtrace that's rendered here comes from the Throwable object, which
doesn't include the line it was created at. The debug plugin uses
debug_backtrace() for its traces, different API with different behavior.

On Friday, August 19, 2016, andrepereiradasilva notifications@github.com
wrote:

to me this sound a little confusing ...

[image: image]
https://cloud.githubusercontent.com/assets/9630530/17826330/8eb14678-6667-11e6-90f2-0882c990750d.png

IMHO this would be a better to have it on the backtrace ... maybe the
numbering also inverted?

I actually prefer the debug plugin Log call back

[image: image]
https://cloud.githubusercontent.com/assets/9630530/17826396/42ee7c3c-6668-11e6-8447-138bd44cc4f4.png

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#11675 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoXJnGVIj44xXBxl9g8qm3vHjD7fIks5qhjRPgaJpZM4Jo1QN
.

@mbabker

The backtrace that's rendered here comes from the Throwable object, which doesn't include the line it was created at.

ok, so this, you say this should not be modified to include it :?
https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/document/error.php#L197

What about (1) adding it the way this PR suggests:

IMO no production website should ever be with debug set to ON in global config or/and the system debug plugin enabled.

and (2) what about changing method signature (so that we can render trace in reverse):

public function renderBacktrace()

to

public function renderBacktrace($reverse = false)

I wouldn't add it to the render method right now. Personally I'd go for
refactoring to inject a Throwable into it versus relying on the internal
pointer (it's why the stacked Exception PR added so many lines to the
template) and finish the PR that moves rendering to a layout and let the
order be a design decision. But the former's a 4.0 thing, or creating a
more generic and reusable helper function for rendering traces in general
with the object injected correctly.

On Friday, August 19, 2016, Georgios Papadakis notifications@github.com
wrote:

@mbabker https://github.com/mbabker

The backtrace that's rendered here comes from the Throwable object, which
doesn't include the line it was created at.

ok, so this, you say this should not be modified to include it :?
https://github.com/joomla/joomla-cms/blob/staging/
libraries/joomla/document/error.php#L197

What about (1) adding it the way this PR suggest:

IMO no production website should ever be with debug set to ON in global
config or/and the system debug plugin enabled.

and (2) what about changing method signature of (so that we can render
trace in reverse):

public function renderBacktrace()

to

public function renderBacktrace($reverse = false)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#11675 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfobQliMq6Y4uqFH3DKXkt7Xw_h2TWks5qhjk4gaJpZM4Jo1QN
.

@ggppdk Where are we at with this - is it on hold?

Well noone objected for adding this file and line of the error when DEBUG is ON,

there was a concern for doing it in more proper place,
but this is not at renderBacktrace() as @mbabker said,

so i think we are good to test it
(unless someone has a better suggestion)

So, I ran into this today and even opened an issue as you can see. Can you update the branch to fix the merge conflict? Then we should test this. :-)

To clarify: I'd rather implement this the way it is proposed here, than to have the broken display that we have right now. If there is a better solution, we can still implement that in 4.0.

The changes to the error.php files do work, but unfortunately another change crept into this PR. Can you fix that and also again merge in the staging branch?

PR updated

Can you add this code to the other error.php files in the frontend, too?

PR updated to add these changes to the error.php of the frontend templates

I have tested this item ✅ successfully on 4d54887

Operating System

• Joomla! 3.8.0-beta3-dev
• PHP 5.6.2
• MySQLi 5.5.38
• Apache/2.2.29 (Unix)

Steps

• Enable Debug mode
• Cause component error by calling a non-existent component (e.g. index.php?option=com_xxx)

Test before patch

• Error message and stack trace shown (see image attached below)
  

Test after patch

• Error message naming faulty statement and stack trace shown (see image attached below)
• Disabling the Debug Mode hides the message
  

Tested @icampus

@lavipr please mark your Test at Issue Tracker.

If this is not possible, i can alter Test.

I have tested this item ✅ successfully on 4d54887

I have tested this patch succesfully. Installed is latest joomla version from github with sample data.

Steps:

enable debug mode
caused a type error of "this" in backend view contacts
Test before Patch : The file and line, don't show up in the error message
Test after Patch: The file and line, are displayed in the error message

Tested @icampus

RTC after two successful tests.

I have tested this item ✅ successfully on 4d54887

I followed the Steps, explained by DrDreave and lavipr I got the same result.

Tested @icampus

@Crometor thanks for Test this Pull Request. As it has 2 successfully Tests, it wasn't necessary for this PR for 3rd Test :-)