Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
More compact display of vendor code in exception pages#26671
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Simperfit commentedMar 26, 2018
This is really nice! |
lyrixx commentedMar 26, 2018
I have often some issues in vendor so for me it will be more complicated to get to the point. Thanks |
nicolas-grekas commentedMar 26, 2018
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. |
nicolas-grekas commentedMar 26, 2018
Or instead of hidding the full vendor frame, can't we just hide the arguments? |
javiereguiluz commentedMar 26, 2018
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
|
ogizanagi commentedMar 26, 2018
That's really better this way. I love it 👍 |
sroze left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Very nice idea 👍
| <divclass="trace-line"> | ||
| {{include('@Twig/Exception/trace.html.twig', {prefix:index,i:i,trace:trace,_display_code_snippet:_display_code_snippet },with_context =false) }} | ||
| <divclass="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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Shouldn't it bestyle: _is_vendor_trace ? 'compact' : 'expanded' instead? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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) andnormal which displays your code but doesn't expand the code snippet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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!
fabpot commentedMar 27, 2018
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
| {%set_is_first_user_code=true %} | ||
| {%fori,traceinexception.trace %} | ||
| {%set_display_code_snippet=_is_first_user_codeand ('/vendor/'not intrace.file)and ('/var/cache/'not intrace.file)and (trace.fileis notempty) %} | ||
| {%set_is_vendor_trace=trace.fileis notemptyand ('/vendor/'intrace.fileor'/var/cache/'intrace.file) %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
does this work on Windows ?
stof commentedMar 28, 2018
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 |
Uh oh!
There was an error while loading.Please reload this page.
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 tohide 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