Exclude by default content-types !== 'text/html'#19
Conversation
|
@snapshotpl, here's a little P/R for JsonResponse. Actually I think I should be the default for all content-types !== 'text/html' Let me know what you think. This P/R is more for discussion. See also: #18 |
| * @param DebugBarRenderer $debugbarRenderer | ||
| * @param bool $excludeNonHtmlContent Whether to disable debugbar on content-types != 'test/html' | ||
| */ | ||
| public function __construct(DebugBarRenderer $debugbarRenderer, $excludeNonHtmlContent=true) |
There was a problem hiding this comment.
Default value change current behaviour. Can we stay as is by default?
|
|
||
| if ($this->isHtmlResponse($response)) { | ||
| return $this->attachDebugBarToResponse($response); | ||
| } elseif ($this->excludeNonHtmlContent) { |
| protected $excludeNonHtmlContent; | ||
|
|
||
| /** | ||
| * PhpDebugBarMiddleware constructor. |
There was a problem hiding this comment.
Useless information - to remove
|
|
||
| public function __construct(DebugBarRenderer $debugbarRenderer) | ||
| /** | ||
| * @var bool |
There was a problem hiding this comment.
Maybe will be better to have some policy for that? Sometimes you want to debug json, but not generated pdf. With bool it's not possible. We can provide some read implmentation and simple interface to provide own.
There was a problem hiding this comment.
Agree... not sure if I can get some time to think about it. We could add a special key '__debugbar' if it does not already exist . But I believe, going content-type-aware will need some more refactoring.
|
And as you see tests are broken ;( |
|
@snapshotpl I've fixed the tests by setting default I'm not sure that I can fix the current behaviour, would need probably some breaking changes, but for sure refactoring of the middleware. I let the P/R living here, but good if you can find a better fix |
see #18