doc: general improvements to querystring.md copy #7023
Conversation
jasnell
added
doc
| * `str` {String} The URL query string to parse | ||
| * `sep` {String} The substring used to delimit key+value pairs in the | ||
| query string. Defaults to `'&'`. | ||
| * `eq` {String}. The substring used to delimit keys and values in the |
There was a problem hiding this comment.
I'm wondering if anyone ever actually uses these sep and eq parameters :)
|
LGTM |
| Optionally override the default separator (`'&'`) and assignment (`'='`) | ||
| characters. | ||
| * `str` {String} The URL query string to parse | ||
| * `sep` {String} The substring used to delimit key+value pairs in the |
|
Nits addressed |
| The `querystring.escape()` method is used by `querystring.stringify()` and is | ||
| generally not expected to be used directly. It is exported primarily to allow | ||
| application code to provide a replacement percent-encoding implementation if | ||
| necessary by assigning `querystring.escape` to an alternative function. | ||
|
|
||
| ## querystring.parse(str[, sep][, eq][, options]) |
There was a problem hiding this comment.
FWIW I'm not sure the optional indicators here accurately reflect how the actual implementation does argument checking.
There was a problem hiding this comment.
Yeah, I was looking at that also. I need to go back in and verify this.
|
Updated |
|
@mscdex ... ping ... I'm still going to go back and check the args against the actual args checking but I'd like to get this PR landed and if there are changes necessary I'll do those separately. Does this LGTY? @nodejs/documentation |
|
@jasnell except for the args issues, LGTM |
jasnell
force-pushed
the
doc-querystring-copy
branch
from
0e7fcb4 to
b9e5e68
Compare
|
@mscdex ... I corrected the arg issues. Getting this landed! |
PR-URL: #7023 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net>
|
Landed in 80f1fbb |
PR-URL: #7023 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net>
Checklist
Affected core subsystem(s)
doc (querystring)
Description of change
General improvements to querystring.md copy
@nodejs/documentation @mscdex