doc: make Buffer documentation styles consistent #4873
Conversation
TimothyGu
changed the title
mscdex
added
buffer
| * `offset` Number | ||
| * `noAssert` Boolean, Optional, Default: false | ||
| * Return: Number | ||
| * `offset` {Number} `0 <= offset <= buf.length` |
There was a problem hiding this comment.
0 <= offset <= buf.length is very "techy", but I can't think of a better way to describe this requirement.
Offset of < 0 or > byteLength would throw new RangeError('Index out of range')
Maybe a paragraph about this error (not sure where) instead of a "techy" requirement expression?
There was a problem hiding this comment.
Well, I'd say that anybody reading documentation on a module designed to do binary stuff "techy" :P
It should be fairly obvious and intuitive that one cannot read (or write) out of range, so I'd argue against having a separate paragraph for such a simple constraint.
How about:
offsetNumber Must be between 0 and the length of the buffer, inclusive
I don't think it is any clearer than the pseudo-JavaScript expression however.
There was a problem hiding this comment.
Shouldn't it actually be 0 <= offset < buf.length? Anyways, I'd reduce the description to just "Zero-based byte offset" or similar.
|
@TimothyGu .. thank you for the PR! Can I ask you to please fill the PR text in with some detail about the change (just copying over the commit text would suffice). Also, it looks like the PR needs to be rebased and updated. |
| @@ -319,84 +320,18 @@ console.log(bufA.length); | |||
| ### Class Method: Buffer.isBuffer(obj) | |||
|
|
|||
| * `obj` Object | |||
|
@jasnell, PR updated. All comments addressed as well. |
|
Awesome! Thank you! LGTM |
| * `end` Number, Optional | ||
| * `value` {String or Number} | ||
| * `offset` {Number} Default: 0 | ||
| * `end` {Number} Default: `buffer.length` |
There was a problem hiding this comment.
Return value is missing
- Maintain alphabetical order - Add documentation for `offset` and `value` where absent - Add return value documentation where absent - Remove redundant "Optional" - Move defaults to parameter enumerations
- Maintain alphabetical order - Add documentation for `offset` and `value` where absent - Add return value documentation where absent - Remove redundant "Optional" - Move defaults to parameter enumerations PR-URL: #4873 Reviewed-By: James M Snell <jasnell@gmail.com>
|
Landed in 6ad1f7b |
- Maintain alphabetical order - Add documentation for `offset` and `value` where absent - Add return value documentation where absent - Remove redundant "Optional" - Move defaults to parameter enumerations PR-URL: #4873 Reviewed-By: James M Snell <jasnell@gmail.com>
|
Blocked from backporting until other PR's are merged first |
- Maintain alphabetical order - Add documentation for `offset` and `value` where absent - Add return value documentation where absent - Remove redundant "Optional" - Move defaults to parameter enumerations PR-URL: #4873 Reviewed-By: James M Snell <jasnell@gmail.com>
- Maintain alphabetical order - Add documentation for `offset` and `value` where absent - Add return value documentation where absent - Remove redundant "Optional" - Move defaults to parameter enumerations PR-URL: #4873 Reviewed-By: James M Snell <jasnell@gmail.com>
- Maintain alphabetical order - Add documentation for `offset` and `value` where absent - Add return value documentation where absent - Remove redundant "Optional" - Move defaults to parameter enumerations PR-URL: #4873 Reviewed-By: James M Snell <jasnell@gmail.com>
- Maintain alphabetical order - Add documentation for `offset` and `value` where absent - Add return value documentation where absent - Remove redundant "Optional" - Move defaults to parameter enumerations PR-URL: nodejs#4873 Reviewed-By: James M Snell <jasnell@gmail.com>
offsetandvaluewhere absent