Skip to content

Add copy actions to network requests (Copy as URL, cURL) - #4113

Merged
kenzieschmoll merged 4 commits into
flutter:masterfrom
jankuss:copy-network-tab
May 19, 2022
Merged

Add copy actions to network requests (Copy as URL, cURL)#4113
kenzieschmoll merged 4 commits into
flutter:masterfrom
jankuss:copy-network-tab

Conversation

@jankuss

@jankuss jankuss commented May 17, 2022

Copy link
Copy Markdown
Contributor

This PR adds the ability to copy HTTP requests in the Network screen.
Right now it is possible to Copy the URL, and to Copy as a cURL command.

Screen.Recording.2022-05-18.at.00.25.24.mov

This closes #3042

As per #3042 (comment), we use the Icons.more_vert for the button which opens the menu.
The menu button is only shown when the request is selected.

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read the [Flutter Style Guide] recently, and have followed its advice.
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@jankuss jankuss changed the title Add actions popup to network requests (Copy as URL, cURL) Add copy actions to network requests (Copy as URL, cURL) May 17, 2022
@jankuss
jankuss force-pushed the copy-network-tab branch from 7d18fa5 to 5560463 Compare May 17, 2022 22:45

@kenzieschmoll kenzieschmoll left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

some initial comments. Thanks for your work on this!

Comment thread packages/devtools_app/lib/src/screens/network/network_screen.dart Outdated
Comment thread packages/devtools_app/lib/src/screens/network/network_screen.dart Outdated
Comment thread packages/devtools_app/lib/src/shared/curl_command.dart Outdated
Comment thread packages/devtools_app/lib/src/shared/table_data.dart Outdated
Comment thread packages/devtools_app/test/shared/curl_command_test.dart
Comment thread packages/devtools_app/lib/src/shared/curl_command.dart Outdated
Comment thread packages/devtools_app/lib/src/shared/curl_command.dart Outdated
Comment thread packages/devtools_app/lib/src/shared/curl_command.dart Outdated
Comment thread packages/devtools_app/lib/src/shared/curl_command.dart Outdated
Comment thread packages/devtools_app/lib/src/shared/curl_command.dart Outdated
@jankuss

jankuss commented May 19, 2022

Copy link
Copy Markdown
Contributor Author

Thanks for the review 👍

I have made adjustments based on your suggestions. I think the CurlCommand class is now easier to grasp.

The comments #4113 (comment) and #4113 (comment) clashed a little, since the _headers() and _body() couldn't be called in the factory because they were instance members. I solved this by marking them static, and passing the required information as parameters instead.

In this case when we reduce this even further, the properties DartIOHttpRequestData data, bool followRedirects and bool multiline are not needed anymore, as they are not accessed in any way. The only thing that remains is the List<String> commandParts.

@jankuss
jankuss requested a review from kenzieschmoll May 19, 2022 16:18
Comment thread packages/devtools_app/lib/src/screens/network/network_screen.dart Outdated
Comment thread packages/devtools_app/test/http/curl_command_test.dart Outdated
Comment thread packages/devtools_app/test/http/curl_command_test.dart Outdated
Comment thread packages/devtools_app/test/http/curl_command_test.dart Outdated
Comment thread packages/devtools_app/lib/src/http/curl_command.dart Outdated

@kenzieschmoll kenzieschmoll left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Thanks for your contribution of this feature!

@kenzieschmoll
kenzieschmoll merged commit 1be2571 into flutter:master May 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feature request: copy http request "as curl"

2 participants