yiisoft / yii2-debug

Debug Extension for Yii 2
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
201 stars 149 forks source link

Fix inability to change LogTarget #469

Closed laxity7 closed 2 years ago

laxity7 commented 2 years ago

Added the ability to change logTarget, now it's hard code class.

$config['modules']['debug'] = [
    'class' => 'yii\debug\Module',
    'allowedIPs' => ['*'],
    'logTarget' => \common\components\DebugLogTarget::class // nothing affects
];
Q A
Is bugfix? ✔️
New feature?
Breaks BC?
Fixed issues none
yii-bot commented 2 years ago

Thank you for putting effort in the improvement of the Yii framework. We have reviewed your pull request.

Unfortunately a use case is missing. It is required to get a better understanding of the pull request and helps us to determine the necessity and applicability of the suggested change to the framework.

Could you supply us with a use case please? Please be as detailed as possible and show some code!

Thanks!

This is an automated comment, triggered by adding the label pr:missing usecase.

laxity7 commented 2 years ago

@samdark, look here please

samdark commented 2 years ago

@laxity7 I still don't get the use-case. That's debug module and it uses debug log target.

laxity7 commented 2 years ago

@samdark there is an error in the code, the attribute (logTarget) responsible for the class does not set this class. Because now it's hard code class:

$this->logTarget = $app->getLog()->targets['debug'] = new LogTarget($this);

I need to change LogTarget to my class, because I wanted to process information a little differently

samdark commented 2 years ago

The only consumer of this information is the debugger itself, chaning the format basically won't work so it must be something about processing. I wonder what differences are these? If these are bug-fixes or enhancements these should be done in the debugger itself rather than giving an ability to break debugger.

laxity7 commented 2 years ago

If someone wants to knowingly break the debugger, then so be it.

I want the attribute that is responsible for setting the class to do its job and that the classes are not hard-coded

bizley commented 2 years ago

If you want to make that change in your app you can always extend debug module and do it. If you want this change to be added for everyone we should have a valid use case and so far it doesn't look valid.

laxity7 commented 2 years ago

For others, nothing will change, everything will work as before, and for those who want to change something there will be such an opportunity

laxity7 commented 2 years ago

In my case, we use prefixes for logging. But the same prefixes are used by Debug, which breaks the display of data in panels, since there is a strict search by category (see log/Target.php:263)

image image

Prefixes in debug image

laxity7 commented 2 years ago

Fellows, it's just a hardcode bug. My PR does class configuration like in other parts of yii

bizley commented 2 years ago

Well, I'm ok with it, not sure about @samdark But please change it so it can benefit from providing the array config as well or an object (Yii style thing).

laxity7 commented 2 years ago

@bizley rightly, done

samdark commented 2 years ago

OK, that's valid. Thanks for the case.

laxity7 commented 2 years ago

that's right?

laxity7 commented 2 years ago

Changed version from 2.1.20 to 2.1.19 in ChangeLog

samdark commented 2 years ago

Thanks!