-
Notifications
You must be signed in to change notification settings - Fork 7.3k
streams: set default encoding for writable streams #8483
Conversation
@jrayaustin Thank you! I haven't checked the code, but you'll need to make your commit conform to the commit messages guidelines. More specifically "The first line must be no more than 50 characters". |
This commit adds the ability to set the default encoding for writable streams. Fixes nodejs#7159
5e61e47
to
cf5dfcd
Compare
Ah, got it. Just made the change thanks for your guidance on this @misterdjules |
Add generic functions for (U)Int read/write operations on Buffers. These support up to and including 48 bit reads and writes. Include documentation and tests. Additional work done by Trevor Norris to include 40 and 48 bit write support. Because bitwise operations cannot be used on values greater than 32 bits, the operations have been replaced with mathematical calculations. Regardless, they are still faster than floating point operations. Reviewed-by: Trevor Norris <trev.norris@gmail.com>
* `util.inspect` cannot accept es6 symbol primitive * It will throw exception if do `util.inspect(Symbol())` * This also affects repl, console.log, etc. Reviewed-by: Trevor Norris <trev.norris@gmail.com> Reviewed-by: Chris Dickinson <christopher.s.dickinson@gmail.com>
Associates link to dns.lookup() with proper URL. Fixes: nodejs#8018 Reviewed-by: Trevor Norris <trev.norris@gmail.com> Reviewed-by: Chris Dickinson <christopher.s.dickinson@gmail.com>
The docs for readLine.pause are misleading. I seriously spent hours on this. If it isn't a bug, at least it should be well documented. Someone else stumbled on this too: http://stackoverflow.com/questions/21341050/pausing-readline-in-node-js Reviewed-by: Trevor Norris <trev.norris@gmail.com> Reviewed-by: Chris Dickinson <christopher.s.dickinson@gmail.com>
Fixes: nodejs#8458 Reviewed-by: Trevor Norris <trev.norris@gmail.com> Reviewed-by: Chris Dickinson <christopher.s.dickinson@gmail.com>
Writable.prototype.setDefaultEncoding = function(encoding) { | ||
if (encoding) { | ||
this._writableState.defaultEncoding = encoding; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few things. We're being better about naming the functions. Make sure the encoding is a string. Also, no need for the braces in a single line if
.
This commit adds the ability to set the default encoding for writable streams. Fixes nodejs#7159
@trevnorris take another look. In addition to checking for a string type i added a return value of true if the encoding was successfully changed otherwise a false value is returned. Seem appropriate? |
Thanks much for the contribution! Squashed and merged in f04f3a0. Made a couple changes:
|
Sweeet! |
The "return boolean" behavior seems inconsistent with the likes of |
Interesting point @mscdex if @trevnorris thinks it's appropriate I'll gladly make the change. |
@jrayaustin I submitted #8529 yesterday. |
Oh nice, thanks @mscdex |
This commit adds the ability to set the default encoding for writable streams.
Fixes #7159