-
Notifications
You must be signed in to change notification settings - Fork 352
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
replaced encodeURI with sdk.Url, issue #435 #437
base: develop
Are you sure you want to change the base?
Conversation
One of test in golang is failing because the generated code snippet includes Can someone help me with this? @webholik |
Made a few changes in test files as mentioned in #438 |
codegens/golang/lib/index.js
Outdated
codeSnippet += `func main() {\n\n${indent}url := "${encodeURI(request.url.toString())}"\n`; | ||
finalUrl = new sdk.Url(request.url.toString()); | ||
// URL encoding each part of Url individually | ||
finalUrl = `${finalUrl.protocol}://${finalUrl.getRemote()}${finalUrl.getPathWithQuery(true)}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A url may not have protocol present. For example if the url is postman.com/post
, then finalURL.protocol
will return undefined
and the finalUrl will be undefined://postman.com/post
which is invalid.
Infact I don't see why we need to manually construct the final URL. url.toString() does that for us.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, sorry. I was unfamiliar with url.toString()
. I will make a necessary changes as soon as possible.
@webholik I have made the necessary changes, however |
@webholik the test fails as it doesn't URL encode I think a better approach would be to parse URLs using Something like |
Replaced
encodeURI
function withsdk.Url(urlString)
frompostman-collection
to parse the URL