8000 [Profiler] Add HttpClient copy as curl feature by noniagriconomie · Pull Request #37691 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Profiler] Add HttpClient copy as curl feature #37691

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[Profiler] Add HttpClient copy as curl feature #37691

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/http_client.html.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
{% extends '@WebProfiler/Profiler/layout.html.twig' %}

{% import _self as helper %}

{% block toolbar %}
{% if collector.requestCount %}
{% set icon %}
Expand Down Expand Up @@ -88,6 +90,7 @@
{% if trace.options is not empty %}
{{ profiler_dump(trace.options, maxDepth=1) }}
{% endif %}
<button class="btn btn-sm" data-clipboard-text="{{ helper.render_curl(trace) }}">Copy-as-Curl</button>
</th>
{% if profiler_token and profiler_link %}
<th>
Expand Down Expand Up @@ -121,11 +124,28 @@
</tr>
</tbody>
</table>
{# tmp: introspect the "trace" #}
{{ dump(trace) }}
{% endfor %}
{% endif %}
</div>
</div>
{% endfor %}
{% endif %}
</div>

<script type="text/javascript">//<![CDATA[

document.querySelectorAll('[data-clipboard-text]').forEach(function(button) {
button.addEventListener('click', function() {
navigator.clipboard.writeText(button.getAttribute('data-clipboard-text'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of writing support for Copy to clipboard as an inline script in this panel, I suggest adding such a feature in the shared JS of the profiler pages, so that it can be reused by any other panel too (as we do for tabs and toggles).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll refactor indeed

})
});

//]]></script>

{% endblock %}

{% macro render_curl(trace) %}
curl "{{ trace.url }}" --compressed -H "Connection: keep-alive" -v
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we can provide a reusable service (request to curl) that be neat :) i believe there are some CurlBuilders in the wild to take inspiration from.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, I would say that it must at least include the method and the headers. For the body, that's harder.

Also, be careful about the fact that you need to escape " in the command arguments (or even better apply proper shell escaping, which might imply moving this logic to a helper method in the collector class rather than doing it in a twig macro)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moving this logic to a helper method in the collector class

would there be any reason to not make it a first class feature, if we want this feature for the profiler?

a usecase i have in mind is adding the curl command in some log entries its context. Or perhaps display in CLI verbose mode.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, if you want to make it a first-class feature, the argument of this API cannot be the internal data structure of this collector

Copy link
Contributor Author

Maybe it will be easier in the datacollector, agreed i'll move this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a ref, the curl formatter used by httplug: https://github.com/php-http/message/blob/master/src/Formatter/CurlCommandFormatter.php

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'll check this thx

{% endmacro %}
0