[Profiler] Use tabular design for HTTP request/headers #355
Conversation
|
awesome, thanks! and completely agree to keep syntax highlighting or more complicated layouting tweak to separate pull requests to make it easier to review. can you please rebase? there is a conflict (i guess because #354 adding a link to the debug token) |
|
I've slightly changed the design of #354, because its label style didn't fit in my feature, hopefully that's fine.
|
|
i like it! @dbrumann as you contributed the profile link, i ask you if you are happy with this too or want a change? |
|
These changes look great. 👍 I like it. I will have a look at the code changes later today, but consider the end result approved |
|
It just accured to me that maybe we could just highlight any link in header table instead? Because if you notice, there is a x-debug-token-link header there already. That would be more universal |
|
as this is a very specific thing to symfony, i like the link at the top. additionally detecting urls in headervalues and link them sounds like a good idea (for a separate pull request) |
|
Allright. We can also reduce complexity further by getting token link from header to display in twig, then Profiler no longer needs to store it separately. |
|
indeed. i see you removed the token from the display and from the profiler data (i think thats good, the token code does not help and is annoying to copy-paste if its part of a link anyways, better copy-paste from the headers). want to do that in this PR or separately? |
|
I can do it here |
|
done |
| {% set hasReachedBody = false %} | ||
| {% set content = '' %} | ||
| {% set data = data|split("\n")|slice(1) %} | ||
| {% set xdebugTokenLink = data|filter(v => 'x-debug-token-link:' in v|lower)|first|split(':')|slice(1)|join(':')|trim %} |
There was a problem hiding this comment.
could you instead split with a limit of 1, to not break up the url and then have to re-join it? would be more elegant imho. i think: data|filter(v => 'x-debug-token-link:' in v|lower)|first|split(':', 2)|slice(1)|trim (but i have not actually tested it)
There was a problem hiding this comment.
I came up with even better simplification. WDYT?
|
nice! |
What's in this PR and why?
Improved profiler design. Check Before and After. Also check linked issue.
Checklist
Out of scope
<t(d|h)>text trimming. I just want to make this simple first time. This can be added in future, at this time horizontal scrolling works perfect here.