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

docs: clarify url.format documentation #8807

Closed

Conversation

misterdjules
Copy link

The original documentation was slightly confusing. It seemed that the
list of items described the properties of the urlObj object, while it
was actually describing the formatting process. This change makes this
clearer.

Fixes #8796.

The original documentation was slightly confusing. It seemed that the
list of items described the properties of the urlObj object, while it
was actually describing the formatting process. This change makes this
clearer.

Fixes nodejs#8796.
@misterdjules
Copy link
Author

@chrisdickinson @trevnorris @orangemocha @cjihrig Could you please take a look?

@chrisdickinson
Copy link

This LGTM as-is.

Sidenote: it might be helpful to illustrate how these keys work in the docs. How would you feel about including the following diagram:

+-----------------------------------------------------------------------------------+
| href                                                                              |
+----------+--+-----------+--------------------+-----------------------------+------+
| protocol |* | auth      | host               | path                        | hash |
|          |  |           +-------------+------+----------+------------------+      |
|          |  |           | hostname    | port | pathname | search           |      |
|          |  |           |             |      |          +-+----------------+      |
|          |  |           |             |      |          | | query          |      |
" http:     //   user:pass@some.host.com:65535 /path/name ? search=1&continues#hash "
|          |  |           |             |      |          | |                |      |
+----------+--+-----------+-------------+------+----------+-+----------------+------+
(all spaces in the "" line should be ignored -- they're purely for formatting)
*: given by "slashes" key

Where keys higher in the diagram override all keys underneath them. This is borrowed from #8755, which introduced the path param -- my comment. If you'd like I can also clean this up and turn it into an SVG and include it in a subsequent PR.

@misterdjules
Copy link
Author

@chrisdickinson I like the diagram a lot! I think it would make a great addition to the documentation.
I think the ASCII diagram already looks good, and I don't know how a SVG rendering would look like.

I suggest landing this PR as is and adding the diagram in another PR. Does it work for you?

@misterdjules
Copy link
Author

Landed in 0603c83, thanks @chrisdickinson!

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