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

fs: set encoding on fs.createWriteStream #1844

Closed
wants to merge 1 commit into from
Closed

fs: set encoding on fs.createWriteStream #1844

wants to merge 1 commit into from

Conversation

yosuke-furukawa
Copy link
Member

I just separated #1412 .
Much time passed since I proposed the pull request. Sorry.

I would like to introduce the pull request again.

According to the createWriteStream api docs, encoding option could be found.

{ flags: 'w',
  encoding: null,
  fd: null,
  mode: 0666 }

But I set the encoding option to fs.createWriteStream('foo.txt', {encoding: 'utf8'}), the encoding option is ignored. I fixed the problem.

And I added the encoding description in this doc. and I fixed test to avoid linter error.

@yosuke-furukawa
Copy link
Member Author

Please re-review for document and test.

@mscdex mscdex added the fs Issues and PRs related to the fs subsystem / file system. label May 30, 2015
const secondEncoding = 'binary';

const examplePath = path.join(common.fixturesDir, 'x.txt');
const dummyPath = path.join(common.tmpDir, '/x.txt');
Copy link
Contributor

Choose a reason for hiding this comment

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

There is an extra / in the second argument to path.join().

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks. fixed.

@yosuke-furukawa
Copy link
Member Author

Thank you. Fixed.

@yosuke-furukawa
Copy link
Member Author

@yosuke-furukawa
Copy link
Member Author

I will land this when CI is healthy.

yosuke-furukawa added a commit that referenced this pull request Jun 2, 2015
Enable encoding option on fs.createWriteStream.

PR-URL: #1844
Reviewed-By: Colin Ihrig <[email protected]>
@yosuke-furukawa
Copy link
Member Author

landed 8357c50
Thank you

andrewdeandrade pushed a commit to andrewdeandrade/node that referenced this pull request Jun 3, 2015
Enable encoding option on fs.createWriteStream.

PR-URL: nodejs/node#1844
Reviewed-By: Colin Ihrig <[email protected]>
@rvagg rvagg mentioned this pull request Jun 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants