Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

readable: Update docs for a case with a binary safe string reading #5155

Closed
wants to merge 1 commit into from

Conversation

TriAnMan
Copy link

@TriAnMan TriAnMan commented Feb 9, 2016

readable.setEncoding(null) - may be the most preferable way to proxy a binary data without any encoding/decoding overhead

See nodejs/readable-stream#180 for details

@mscdex mscdex added doc Issues and PRs related to the documentations. stream Issues and PRs related to the stream subsystem. labels Feb 9, 2016
as strings, always use this method.
called [`buf.toString(encoding)`][] on them. Also you can disable
any encoding at all with `readable.setEncoding(null)`.
If you want to read the data as strings, always use this method.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would flip these last two sentences around, or move the new sentence to a separate paragraph.

@jasnell
Copy link
Member

jasnell commented Feb 10, 2016

LGTM with a nit.

@TriAnMan TriAnMan force-pushed the doc-readable-encoding-fix branch from d184b48 to eb748a2 Compare February 26, 2016 16:03
@TriAnMan
Copy link
Author

@jasnell Could you review?

@@ -416,6 +416,10 @@ potentially mangled if you simply pulled the Buffers directly and
called [`buf.toString(encoding)`][] on them. If you want to read the data
as strings, always use this method.

Also you can disable any encoding at all with `readable.setEncoding(null)`.
This approach is wery usefull if you deal with binary data or with large
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useful

@TriAnMan TriAnMan force-pushed the doc-readable-encoding-fix branch from eb748a2 to 66bb1b4 Compare February 29, 2016 09:34
@TriAnMan
Copy link
Author

@silverwind thanks! (:

@@ -416,6 +416,10 @@ potentially mangled if you simply pulled the Buffers directly and
called [`buf.toString(encoding)`][] on them. If you want to read the data
as strings, always use this method.

Also you can disable any encoding at all with `readable.setEncoding(null)`.
This approach is wery useful if you deal with binary data or with large
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very

`readable.setEncoding(null)` - may be the most preferable way to proxy a binary data without any encoding/decoding overhead
@TriAnMan TriAnMan force-pushed the doc-readable-encoding-fix branch from 66bb1b4 to 174fec5 Compare February 29, 2016 09:56
@TriAnMan
Copy link
Author

@silverwind Thanks a lot... Seems I need to install a spell checker for my Sublime. (:

@silverwind
Copy link
Contributor

Landed in 8d8fef0.

@silverwind silverwind closed this Feb 29, 2016
silverwind pushed a commit that referenced this pull request Feb 29, 2016
`readable.setEncoding(null)` - may be the most preferable way to proxy
a binary data without any encoding/decoding overhead

PR-URL: #5155
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
@TriAnMan TriAnMan deleted the doc-readable-encoding-fix branch February 29, 2016 10:18
@Fishrock123 Fishrock123 mentioned this pull request Mar 1, 2016
5 tasks
Fishrock123 pushed a commit that referenced this pull request Mar 2, 2016
`readable.setEncoding(null)` - may be the most preferable way to proxy
a binary data without any encoding/decoding overhead

PR-URL: #5155
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
@MylesBorins
Copy link
Contributor

I'm 99% sure this applied to v4. Going to give a day before landing for people to chime in and let me know not to land it

MylesBorins pushed a commit that referenced this pull request Mar 17, 2016
`readable.setEncoding(null)` - may be the most preferable way to proxy
a binary data without any encoding/decoding overhead

PR-URL: #5155
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 21, 2016
`readable.setEncoding(null)` - may be the most preferable way to proxy
a binary data without any encoding/decoding overhead

PR-URL: #5155
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants