doc: fix about decodeStrings property of stream.Writable
#19752
Conversation
This fixes negation ommitted at 80ea0c5.
nodejs-github-bot
added
doc
mandel59
changed the title
decodeStrings property of stream.WritabledecodeStrings property of stream.Writable
|
I've noticed that the phrase "and |
|
I wonder what "the character encoding of the string" is. It should be UTF-16 in JavaScript, shouldn't it? |
mandel59
changed the title
decodeStrings property of stream.WritabledecodeStrings property of stream.Writable
mscdex
added
wip
doc/api/stream.md
Outdated
| options, then `chunk` will remain the same object that is passed to `.write()`, | ||
| and may be a string rather than a Buffer. In that case, the `encoding` argument | ||
| will indicate the character encoding of the string. Otherwise, the `encoding` | ||
| argument can be safely ignored. |
There was a problem hiding this comment.
If you want, feel free to mention that it’s always going to be 'buffer' in that case
There was a problem hiding this comment.
What about object mode streams? I just tried and got 'utf8' as encoding. Is it intended?
It is generally whatever is being passed to
JS strings don’t really have a character encoding – the way that JS strings work, it is reasonable for the engine to store them using UTF-16, but that’s transparent to the user and it’s not universally true. The character encoding in this context is not some intrinsic property of the string; it has to be passed along as separate metadata. For example, |
|
"The character encoding of the string" actually means "the character encoding which |
Yup, exactly that. :)
Yes, that’s somewhat unfortunate – Node.js puts both character encodings (sequence of characters → seq of bytes) and binary-to-text encodings (seq of bytes → seq of characters) behind the same API, because that happens to work out fine. So, yeah, you’re right, the API that one would use to encode UTF-8 or UTF-16 in Node.js would be the API that decodes hex and base64. It’s confusing, especially for newbies, and if you have suggestions for improving the documentation on this, that would be really really neat. |
doc/api/stream.md
Outdated
| object that is passed to `.write()`. | ||
| If the `decodeStrings` property is explicitly set to `false` in the constructor | ||
| options, then `chunk` will remain the same object that is passed to `.write()`, | ||
| and may be a string rather than a Buffer. This is to support implementations |
There was a problem hiding this comment.
s/Buffer/Buffer (surround with backticks) ... this should be consistent throughout the doc if it's not already.
This adds the sentence missed at 8a354c8
mandel59
force-pushed
the
fix-doc-decodeStrings
branch
from
626d237 to
0e9e2e2
Compare
BridgeAR
added
the
author ready
|
Landed in 8788046 🎉 |
This fixes negation ommitted in a former commit. It also simplifies the text in general. PR-URL: nodejs#19752 Refs: nodejs@80ea0c5 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
This fixes negation ommitted in a former commit. It also simplifies the text in general. PR-URL: #19752 Refs: 80ea0c5 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
This fixes negation ommitted at 80ea0c5.
Corresponding sentence at 5292a13 (before 80ea0c5):
Checklist