Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

doc:fix default value of decodeURIComponent #9207

Closed
wants to merge 1 commit into from
Closed

doc:fix default value of decodeURIComponent #9207

wants to merge 1 commit into from

Conversation

h7lin
Copy link

@h7lin h7lin commented Feb 13, 2015

No description provided.

@misterdjules
Copy link

Thank you! LGTM, I would land that on v0.12 though, since it's also a problem in the v0.12 branch.

@misterdjules misterdjules added this to the 0.12.1 milestone Feb 20, 2015
@misterdjules
Copy link

I would also add more details to the documentation for querystring.unescape. I'm not sure if it should mention that it uses decodeURIComponent and falls back to QueryString.unescapeBuffer, as these seem to be implementation details.

@misterdjules
Copy link

Regarding my previous comment, I think mentioning that querystring.unescape uses decodeURIComponent and falls back to a safer equivalent that doesn't throw on malformed URLs is fine.

robertkowalski added a commit to robertkowalski/node-v0.x-archive that referenced this pull request Feb 21, 2015
 - add an article: `decode a non-utf8 string`
 - explain default behaviour of `querystring.unescape`
 - add a note that the baviour can change if other folks on your
   team have overridden it

related to nodejs#9207
@robertkowalski
Copy link

i added the changes in #9259 - the both commits should not conflict and should get merged together

robertkowalski added a commit to robertkowalski/node-v0.x-archive that referenced this pull request Feb 21, 2015
 - add an article: `decode a non-utf8 string`
 - explain default behaviour of `querystring.unescape`
 - add a note that the baviour can change if other folks on your
   team have overridden it

related to nodejs#9207

Conflicts:
	doc/api/querystring.markdown
@misterdjules
Copy link

@h7lin @robertkowalski Thank you both!

@h7lin Do you want to use your real name as the author name for your commit, or do you want to keep "h7lin"?

@misterdjules
Copy link

@h7lin Thank you! Landed in f1bf883.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants