| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
There was a problem hiding this comment.
Couple of nits but otherwise LGTM
Sorry, something went wrong.
There was a problem hiding this comment.
For consistency, please s/_Note_/*Note*:
Sorry, something went wrong.
There was a problem hiding this comment.
This isn’t specific to base64, it applies at least to hex, too:
> Buffer.byteLength('aa ', 'hex')
2
> Buffer.from('aa ', 'hex').length
1(I think it’s only those two encodings but I didn’t double-check.)
Sorry, something went wrong.
There was a problem hiding this comment.
@addaleax oh indeed. That makes wording it a bit more complicated...
Sorry, something went wrong.
There was a problem hiding this comment.
nit: s/aren't/are not
Sorry, something went wrong.
There was a problem hiding this comment.
nit: Maybe mention that the method doesn’t account for whitespace in the input … for some people that might be included in “invalid”, but that’s kind of subjective…
Sorry, something went wrong.
|
Done, but now I have trouble coming up with a descriptive commit message. Any better suggestions? |
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM
“doc: clarify the behavior of byteLength” (or “doc: clarify the behavior of Buffer.byteLength” if you feel like it) sounds perfectly fine to me :)
Sorry, something went wrong.
|
|
||
| *Note* that for `'base64'` and `'hex'`, this function assumes valid input. For | ||
| strings that contain non-Base64/Hex-encoded data (e.g. whitespace), the return | ||
| value might be greater than the length of a `Buffer` created from the string. |
There was a problem hiding this comment.
The return value could be smaller too (e.g. Buffer.byteLength('1345 4 ', 'hex') === 3, Buffer.from('1345 4 ').byteLength === 7), but then since the data is invalid it's just hard to say. Maybe just say something like the return value is meaningless and should not be used?
Sorry, something went wrong.
There was a problem hiding this comment.
Hmm, wait, I forgot to pass hex to Buffer.from('1345 4 '), that one would just throws TypeError: Invalid hex string, so there is nothing to compare :/
Sorry, something went wrong.
There was a problem hiding this comment.
Yeah, logically there is no way for it to be smaller. That would imply that you can encode more data by using an invalid string than a valid string.
It's not completely meaningless - e.g. if you allocate a Buffer with the size at least Buffer.byteLength, then the actual buffer will fit in it. Although I can't imagine a use case for it.
Sorry, something went wrong.
PR-URL: nodejs#11238 Refs: nodejs#11165 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Michaël Zasso <targos@protonmail.com>
PR-URL: nodejs#11238 Refs: nodejs#11165 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Michaël Zasso <targos@protonmail.com>
| Back | FazBrowse Home | New Git URL |
Ref: #11165
Checklist
Affected core subsystem(s)
doc