Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closes #959 Fix "Copy Curl" to respect "encode url" option #979

Merged
merged 3 commits into from Jun 9, 2018

Conversation

@besolov
Copy link
Contributor

besolov commented Jun 8, 2018

Closes #959 [Bug] 'Copy as curl' feature doesn't respect "Automatically encode special characters in URL" option

@besolov besolov changed the title Closes 959 Fix "Copy Curl" to respect "encode url" option Closes #959 Fix "Copy Curl" to respect "encode url" option Jun 8, 2018
Copy link
Contributor

gschier left a comment

Nice work @besolov. Just a few minor comments.

@@ -360,7 +360,8 @@ export async function exportHarWithRenderedRequest (
queryString: getRequestQueryString(renderedRequest),
postData: getRequestPostData(renderedRequest),
headersSize: -1,
bodySize: -1
bodySize: -1,
encode: renderedRequest.settingEncodeUrl

This comment has been minimized.

Copy link
@gschier

gschier Jun 8, 2018

Contributor

Since the encode property isn't officially part of the HAR spec, it might make more sense to prefix this like we do with request settings.

I vote to keep it consistent and use the same name settingEncodeUrl.

This comment has been minimized.

Copy link
@gschier

This comment has been minimized.

Copy link
@besolov

besolov Jun 8, 2018

Author Contributor

Done

@@ -174,7 +174,9 @@ HTTPSnippet.prototype.prepare = function (request) {

// update the uri object
request.uriObj.query = request.queryObj
request.uriObj.search = qs.stringify(request.queryObj)
request.uriObj.search = qs.stringify(request.queryObj, '&', '=', {

This comment has been minimized.

Copy link
@gschier

gschier Jun 8, 2018

Contributor

It took me a while to figure out what this was doing but it looks good! 👍

This comment has been minimized.

Copy link
@besolov

besolov Jun 8, 2018

Author Contributor

Do you want me to simplify it somehow?

This comment has been minimized.

Copy link
@gschier

gschier Jun 8, 2018

Contributor

If you can, but not a big deal. I can't think of anything obvious that would simplify it.

This comment has been minimized.

Copy link
@besolov

besolov Jun 8, 2018

Author Contributor

Me too, actually(

@gschier
gschier approved these changes Jun 8, 2018
Copy link
Contributor

gschier left a comment

Looks good. Thanks for doing this! 👍

@gschier gschier merged commit beef9b8 into Kong:develop Jun 9, 2018
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
luizmariz pushed a commit to luizmariz/insomnia that referenced this pull request Jan 22, 2020
)

* issue-959 Fix "Copy Curl" to respect "encode url" option

* issue-959 Fix tests

* issue-959 Fix review comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.