Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

doc update to encodings supported by fs.ReadStream setEncoding #2911

Closed
mvolkmann opened this issue Mar 12, 2012 · 5 comments
Closed

doc update to encodings supported by fs.ReadStream setEncoding #2911

mvolkmann opened this issue Mar 12, 2012 · 5 comments
Labels

Comments

@mvolkmann
Copy link

The docs at http://nodejs.org/docs/latest/api/stream.html#stream_stream_setencoding_encoding
say that the encoding "can be 'utf8', 'ascii', or 'base64'".
Add 'binary', 'hex' and 'ucs2' to the list.

@bnoordhuis
Copy link
Member

Mailing list thread: http://groups.google.com/group/nodejs/browse_thread/thread/ba9f57e1f45953ca

This is really two issues:

  1. fs.ReadStream.setEncoding() accepts more than just 'utf8', 'ascii' and 'base64'
  2. stream.Stream.setEncoding() doesn't really exist. The Stream interface is a protocol, it's up to implementing classes to provide the concrete implementation.

The first one would be easy to fix were it not for the fact that fs.ReadStream.setEncoding() is not separately documented (it links to the streams API page).

@isaacs: re the second issue: what are the plans for the stream API? .setEncoding() seems like an unfortunate addition.

@koichik
Copy link

koichik commented Mar 12, 2012

Currently, 'ucs2' is not supported.

Because 'ucs2' is multi byte character encoding, StringDecoder should handle an incomplete character
(when a chunk breaks off at first 1 byte of a character, it is necessary to be combined with the next chunk),but does not.

Refs: #1681.

@isaacs
Copy link

isaacs commented Mar 16, 2012

I think perhaps setEncoding should just be removed from the Stream doc, and added in the documentation of the various streams where it's actually used.

A more thorough streams cleanup is planned for v0.9. A setEncoding method would be ok, perhaps using the string_decoder under the hood if necessary.

@papandreou
Copy link

Now that you're mentioning the rewrite -- in my opinion StringDecoder should be exposed as a standalone filter (read/writable stream) rather than being built into every readable stream. That would make it more composable and get rid of setEncoding:

readableStream.pipe(new StringDecoder('utf-8')).on('data', function (text) {...});

@chrisdickinson
Copy link

Encoding is now mentioned in the streams docs as referring to valid buffer encodings.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants