doc: improve buf.indexOf() documentation style #19861
Conversation
nodejs-github-bot
added
buffer
|
@nodejs/documentation |
vsemozhetbyt
added
the
fast-track
vsemozhetbyt
added
the
author ready
| @@ -1258,10 +1258,10 @@ changes: | |||
|
|
|||
| * `value` {string|Buffer|Uint8Array|integer} What to search for. | |||
| * `byteOffset` {integer} Where to begin searching in `buf`. **Default:** `0`. | |||
| * `encoding` {string} If `value` is a string, this is its encoding. | |||
| * `encoding` {string} The encoding of `value` if `value` is a string. | |||
There was a problem hiding this comment.
nit: I’d say something like If `value` is a string, this encoding is used to look it up in the buffer., or something else that doesn’t sound like the encoding is a inherent to the string value itself
There was a problem hiding this comment.
@addaleax Maybe this? If `value` is a string, this is the encoding used to convert `value` to a binary representation before searching for `value` in `buf`?
There was a problem hiding this comment.
Hmmm...except my wording possibly sounds like value is mutated...
There was a problem hiding this comment.
@Trott Yeah, good point. How about: If `value` is a string, this function looks for the representation of `value` in `encoding`.? It’s less ambiguous and arguably captures the semantics better, but it might also be harder to understand what “representation” means here?
doc/api/buffer.md
Outdated
| that coerce to `NaN` or 0, like `{}`, `[]`, `null` or `undefined`, will search | ||
| the whole buffer. This behavior matches [`String#indexOf()`]. | ||
| If `byteOffset` is not a number, it will be coerced to a number. If an argument | ||
| is coerced to `NaN` or `0`, then the entire buffer will be searched. This |
There was a problem hiding this comment.
In the first part of the sentance, this looks a little less clear to me with the «coerced» word.
Perhaps, «For arguments that coerce to … the entire buffer will be searched» or something similar?
There was a problem hiding this comment.
byteOffset is the only argument here for which the statement is applicable so maybe this?
If
byteOffsetis not a number, it will be coerced to a number. If it is
coerced toNaNor0, then the entire buffer will be searched. This
...of this?:
If
byteOffsetis not a number, it will be coerced to a number. If the result
of coercion isNaNor0, then the entire buffer will be searched. This
* improve usage of "coerce" in buffer.md * reduce informal tone in buffer.md * wrap line at 80 characters in buffer.md * wrap value in backtick for consistency
* improve usage of "coerce" in buffer.md * reduce informal tone in buffer.md * wrap line at 80 characters in buffer.md * wrap value in backtick for consistency PR-URL: nodejs#19861 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
|
Landed in 28e5c46 |
* improve usage of "coerce" in buffer.md * reduce informal tone in buffer.md * wrap line at 80 characters in buffer.md * wrap value in backtick for consistency PR-URL: #19861 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
* improve usage of "coerce" in buffer.md * reduce informal tone in buffer.md * wrap line at 80 characters in buffer.md * wrap value in backtick for consistency PR-URL: nodejs#19861 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
Checklist