Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

fs: Support `close` option for streams (default=true) #4389

Open
isaacs opened this Issue · 7 comments

4 participants

@isaacs
Owner

It's annoying that there's no way to make a fs stream not close when it reaches the end, especially when you're providing a fd from a pool that you intend to re-use.

Add a close flag. This defaults to true if no fd is passed in, or false otherwise. If it's anything other than boolean false, then call fs.close(fd) when the EOF is reached.

I'm unsure how this should interact with #4387. Should it still close on error if you've set close: false?

@coltrane

I'd suggest a more descriptive name -- perhaps something like close_on_eof.

Then, if it's desirable to control the behavior when an error is encountered, then a separate flag (close_on_error) could be used.

These names have the additional benefit of being mostly self-documenting.

@Mithgol

Their names should probably contain actual spaces instead of underscores, such as {"close on eof": "true"}.

@isaacs
Owner

You guys are crazy. No, we are not putting underscores in option names, and definitely not putting spaces in option names. Our options shall be camelCase as god intended.

@isaacs
Owner

But really, I want a single "close" option that covers both. If you're sharing fd's between multiple streams, you definitely don't want them to be closed.

@coltrane

camelCase is good. If it's going to be one single option, then I'd still suggest something more descriptive than close. How about: autoClose?

var r = fs.createReadStream("somefile", {autoClose: false});

then, from the name alone, it's clear: if I disable automatic closing then I'll need to close the fd manually. Otherwise, presumably, it'll be managed for me. Even if I'm unfamiliar with this particular feature, I can understand enough from the option's name to grasp what its function must be.

@bnoordhuis

autoClose is fine. First one to submit the PR gets a cookie.

@isaacs
Owner

If the pull request isn't based on the streams2 branch, then the first one to submit a PR gets their commit clobbered ;)

@alFReD-NSH alFReD-NSH referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@alFReD-NSH alFReD-NSH referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@alFReD-NSH alFReD-NSH referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@alFReD-NSH alFReD-NSH referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@alFReD-NSH alFReD-NSH referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@alFReD-NSH alFReD-NSH referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@alFReD-NSH alFReD-NSH referenced this issue from a commit in alFReD-NSH/node
@alFReD-NSH alFReD-NSH Add autoClose=true option to fs.createReadStream
Closes #4389.
4f3d3cd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.