-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
Adding clarity to buffer toString docs. #8984
Conversation
As discussed in nodejs#8971, the docs for this method could be improved. Fixes: nodejs#8971
// Prints: tés | ||
console.log(buf.toString('utf8', 0, 3)); | ||
// Prints: 74c3a97374 | ||
console.log(buf2.toString('hex')); |
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.
I know this isn’t mentioned in #8971, but the console.log(buf.toString(…))
statements should probably refer to buf1
, not buf
, so it would make sense to fix that, too?
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, totally missed that. Sure.
@@ -1811,8 +1811,8 @@ added: v0.1.90 | |||
--> | |||
|
|||
* `encoding` {String} The character encoding to decode to. **Default:** `'utf8'` | |||
* `start` {Integer} Where to start decoding. **Default:** `0` | |||
* `end` {Integer} Where to stop decoding (not inclusive). **Default:** [`buf.length`] | |||
* `start` {Integer} They byte offset to start decoding at. **Default:** `0` |
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.
typo: They
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.
Silly me. Resolved!
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.
LGTM
* `start` {Integer} Where to start decoding. **Default:** `0` | ||
* `end` {Integer} Where to stop decoding (not inclusive). **Default:** [`buf.length`] | ||
* `start` {Integer} The byte offset to start decoding at. **Default:** `0` | ||
* `end` {Integer} The byte offset to stop decoding at (not inclusive). **Default:** [`buf.length`] |
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.
nit: long line
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.
LGTM with a nit
c133999
to
83c7a88
Compare
Landed in 366fc7a. fixed nits on landing |
Checklist
Affected core subsystem(s)
Update to the docs
Description of change
As discussed in #8971, the docs for this method could be improved. This is my attempt at clarification.
Fixes: #8971