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

doc, url: various improvements to WHATWG API #11330

Closed
wants to merge 5 commits into from

Conversation

TimothyGu
Copy link
Member

Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc, url

@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Feb 13, 2017
@TimothyGu TimothyGu added doc Issues and PRs related to the documentations. whatwg-url Issues and PRs related to the WHATWG URL implementation. labels Feb 13, 2017
doc/api/url.md Outdated

#### urlSearchParams.toString()

* Returns: {String}

Returns the search parameters serialized as a URL-encoded string.
Returns the search parameters serialized as a percent-encoded string.
Copy link
Member

Choose a reason for hiding this comment

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

Probably "as percent-encoded strings" or "as a string of percent-encoded characters", or even "percent-encoded bytes"? "a percent-encoded string" sounds a bit weird given that they would be not just be encoded but also joined with & and =s.

Another way to explain it would be to mention that it is somewhat similar to querystring.stringify().

Copy link
Member Author

Choose a reason for hiding this comment

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

How about:

-Returns the search parameters serialized as a URL-encoded string.
+Returns the search parameters serialized as a string, with characters
+percent-encoded where necessary.

doc/api/url.md Outdated
value returned is equivalent to that of [`url.href`][].

Because of the need for standard compliance, this method does not allow for
customization on the serialization process of the URL. For more flexibility,
Copy link
Member

Choose a reason for hiding this comment

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

nit: "does not allow users to customize the.." seems easier to read?

doc/api/url.md Outdated
@@ -298,6 +298,35 @@ forward slash (`/`) character is encoded as `%3C`.
The `url` module provides an *experimental* implementation of the
[WHATWG URL Standard][] as an alternative to the existing `url.parse()` API.

A comparison between this API and `url.parse()` is given below. Above the URL
`'http://user:pass@host.com:8080/p/a/t/h?query=string#hash'`, properties of an
object returned by `url.parse()` are given. Beneath it are properties of a
Copy link
Member

Choose a reason for hiding this comment

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

nit: "Below it"?(as opposed to "Above it")

which characters to percent-encode may vary somewhat from what the
[`url.parse()`][] and [`url.format()`][] methods would produce.
are [percent-encoded][]. Note that the selection of which characters to
percent-encode may vary somewhat from what the [`url.parse()`][] and
Copy link
Member

Choose a reason for hiding this comment

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

I know it's already there but technically, doesn't url.parse() percent-decode things..?

Copy link
Member Author

Choose a reason for hiding this comment

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

url.parse() also encodes certain things:

> url.parse('http://\n.com').pathname
'%0A.com'

@jasnell
Copy link
Member

jasnell commented Feb 15, 2017

@TimothyGu
Copy link
Member Author

Landed in 6cea5dd and bd07c8f.

@TimothyGu TimothyGu closed this Feb 16, 2017
@TimothyGu TimothyGu deleted the url-components-doc branch February 16, 2017 05:57
TimothyGu added a commit that referenced this pull request Feb 16, 2017
PR-URL: #11330
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
TimothyGu added a commit that referenced this pull request Feb 16, 2017
Also remove executable bit from doc/api/url.md's mode.

PR-URL: #11330
Fixes: 4757ddc "doc: add basic documentation for WHATWG URL API"
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
TimothyGu added a commit to TimothyGu/node that referenced this pull request Feb 18, 2017
TimothyGu added a commit to TimothyGu/node that referenced this pull request Feb 18, 2017
Also remove executable bit from doc/api/url.md's mode.

Backport-of: nodejs#11330
Fixes: 84e2ff3 "doc: add basic documentation for WHATWG URL API"
italoacasas pushed a commit to italoacasas/node that referenced this pull request Feb 20, 2017
italoacasas pushed a commit to italoacasas/node that referenced this pull request Feb 20, 2017
Also remove executable bit from doc/api/url.md's mode.

Backport-of: nodejs#11330
Fixes: 84e2ff3 "doc: add basic documentation for WHATWG URL API"
italoacasas pushed a commit that referenced this pull request Feb 22, 2017
italoacasas pushed a commit that referenced this pull request Feb 22, 2017
Also remove executable bit from doc/api/url.md's mode.

Backport-of: #11330
Fixes: 84e2ff3 "doc: add basic documentation for WHATWG URL API"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory. whatwg-url Issues and PRs related to the WHATWG URL implementation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants