More compact display of vendor code in exception pages #26671
Conversation
|
This is really nice! |
|
I have often some issues in vendor so for me it will be more complicated to get to the point. Thanks |
|
Would be great if the toggle could be inplace of the hidden vendors. Putting it on the top right corner makes it hard to spot. |
|
Or instead of hidding the full vendor frame, can't we just hide the arguments? |
|
After reading your comments, I propose a complete redesign of this feature. Now we don't hide anything, we just make info more compact for vendors and keep the existing design for your code. Simple exceptionsBefore
After
Complex exceptionsBefore
After
|
|
That's really better this way. I love it 👍 |
| <div class="trace-line"> | ||
| {{ include('@Twig/Exception/trace.html.twig', { prefix: index, i: i, trace: trace, _display_code_snippet: _display_code_snippet }, with_context = false) }} | ||
| <div class="trace-line {{ _is_vendor_trace ? 'trace-from-vendor' }}"> | ||
| {{ include('@Twig/Exception/trace.html.twig', { prefix: index, i: i, trace: trace, style: _is_vendor_trace ? 'compact' : _display_code_snippet ? 'expanded' }, with_context = false) }} |
There was a problem hiding this comment.
Shouldn't it be style: _is_vendor_trace ? 'compact' : 'expanded' instead? 🤔
There was a problem hiding this comment.
This is a bit trickier because we have 3 states: compact for vendors, expanded for the only trace that displays the code snippet expanded (there should be just one) and normal which displays your code but doesn't expand the code snippet.
There was a problem hiding this comment.
Maybe I'm missing something, but doesn't this check will interpret any application path containing "/vendor/" as part of vendor code? I know my point is really border, but could we check if the path is really in the project's /vendor directory?
There was a problem hiding this comment.
Although you are right, I'd prefer to ignore that edge case: having /vendor/ in the file path and not being a real vendor. Moreover, in the last iteration of this feature we no longer hide anything, so the user won't miss anything important even in that edge case. Cheers!
|
Thank you @javiereguiluz. |
… (javiereguiluz) This PR was squashed before being merged into the 4.1-dev branch (closes #26671). Discussion ---------- More compact display of vendor code in exception pages | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #23144 | License | MIT | Doc PR | - I like the general idea proposed in #23152 ... but I don't like the implementation ... so this PR is an alternative implementation. **UPDATE**: the proposed feature has been completely redesigned. See #26671 (comment) ~~The idea is to **hide by default all information that comes from "vendors"** (`vendor/` or `var/cache/` dir) because that code is out of your reach and you can't change it to fix your error.~~ ~~In practice, each exception trace now would display a `Hide vendors` option enabled by default:~~ <details> <summary>Click here to view the outdated images</summary>  It works like a toggle ... and it's compatible with the overall toggle of each trace header:  When exceptions are complex, the amount of noise removed is massive: ## Before  ## After  </details> Commits ------- 510b05f More compact display of vendor code in exception pages
| @@ -22,10 +22,11 @@ | |||
| <div id="trace-html-{{ index }}" class="sf-toggle-content"> | |||
| {% set _is_first_user_code = true %} | |||
| {% for i, trace in exception.trace %} | |||
| {% set _display_code_snippet = _is_first_user_code and ('/vendor/' not in trace.file) and ('/var/cache/' not in trace.file) and (trace.file is not empty) %} | |||
| {% set _is_vendor_trace = trace.file is not empty and ('/vendor/' in trace.file or '/var/cache/' in trace.file) %} | |||
|
This is changing only the template of the TwigBundle exception controller, not the output of the Debug component, so this does not solve the same case than #23152 |
I like the general idea proposed in #23152 ... but I don't like the implementation ... so this PR is an alternative implementation.
UPDATE: the proposed feature has been completely redesigned. See #26671 (comment)
The idea is to hide by default all information that comes from "vendors" (vendor/orvar/cache/dir) because that code is out of your reach and you can't change it to fix your error.In practice, each exception trace now would display aHide vendorsoption enabled by default:Click here to view the outdated images
It works like a toggle ... and it's compatible with the overall toggle of each trace header:
When exceptions are complex, the amount of noise removed is massive:
Before
After