Summary: | Web Inspector: Show requests in `curl` syntax in DevTools → Network → Headers | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sergey Ryazanov <serya> | ||||||||||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Sergey Ryazanov <serya> | ||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||
Severity: | Normal | CC: | apavlov, buildbot, keishi, loislo, mathias, paulirish, pfeldman, pmuellr, rniwa, vsevik, web-inspector-bugs, webkit.review.bot, yurys | ||||||||||||||||
Priority: | P2 | ||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||
Hardware: | All | ||||||||||||||||||
OS: | All | ||||||||||||||||||
URL: | http://crbug.com/166617 | ||||||||||||||||||
Attachments: |
|
Description
Sergey Ryazanov
2013-01-18 06:08:42 PST
Created attachment 183447 [details]
Patch
Created attachment 183448 [details]
Screenshot of the new button.
Comment on attachment 183447 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183447&action=review Mostly looks good. Few nits inline. > Source/WebCore/English.lproj/localizedStrings.js:89 > +localizedStrings["Copy `curl` command"] = "Copy `curl` command"; Consider renaming to "Copy as curl"? > Source/WebCore/inspector/front-end/RequestHeadersView.js:341 > + var copyCurlButton = this._createToggleButton(WebInspector.UIString("Copy `curl` command")); I think this should be exposed as a context menu item on the network panel items. > Source/WebCore/inspector/front-end/RequestHeadersView.js:408 > + function escape(str) { keep { on the next line. > Source/WebCore/inspector/front-end/RequestHeadersView.js:417 > + var supposedMethod = "GET"; inferredMethod? > Source/WebCore/inspector/front-end/RequestHeadersView.js:419 > + if (this._request.requestContentType() == "application/x-www-form-urlencoded" && this._request.requestFormData) { === > Source/WebCore/inspector/front-end/RequestHeadersView.js:423 > + } if (this._request.requestFormData) { else if ? > Source/WebCore/inspector/front-end/RequestHeadersView.js:429 > + if (this._request.requestMethod != supposedMethod) !== > Source/WebCore/inspector/front-end/RequestHeadersView.js:438 > + command += data; A good pattern is to make command an array, push tokens into it and then return command.join(""); You don't need "data" as well. Created attachment 183736 [details]
Patch
Comment on attachment 183736 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183736&action=review > Source/WebCore/inspector/front-end/NetworkPanel.js:1380 > + return command.join(""); maybe use join(" ") and get rid of the spaces in "curl ", " --data ", and other strings? Created attachment 183760 [details]
Patch
Comment on attachment 183760 [details]
Patch
The change looks good. It is missing a test. You can call into new private method from within the test and dump the resulting curl. Note that you might need a mock request for that since otherwise it is going to be platform specific. Clearing r? while waiting for the test.
Created attachment 183821 [details]
Patch
Comment on attachment 183821 [details] Patch Attachment 183821 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/16038217 New failing tests: svg/as-image/img-relative-height.html Comment on attachment 183821 [details] Patch Clearing flags on attachment: 183821 Committed r140391: <http://trac.webkit.org/changeset/140391> All reviewed patches have been landed. Closing bug. Reopening to attach new patch. Created attachment 190206 [details]
Patch
Please follow the bug-per-patch policy. Created attachment 190246 [details]
I occasionally uploaded patch from another bug. Returning the right patch.
Sergey, if you can do a driveby it'd be nice to rename the command to match the correct capitalization: http://en.wikipedia.org/wiki/cURL |