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.

Sign up for GitHub

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

Jump to bottom

Clarify "fs.readFile", "fs.appendFile", "fs.writeFile" doc… #7561

Closed
wants to merge 5 commits into from
Closed

Clarify "fs.readFile", "fs.appendFile", "fs.writeFile" doc… #7561

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e4a8d56
refs #7560 Clarify "fs.readFile", "fs.appendFile", "fs.writeFile" doc…
kibertoad Jul 6, 2016
a851058
refs #7560 Use backticks instead of single quotes.
kibertoad Jul 6, 2016
40a5750
refs #7560 Merge two notes, wrap at 80 symbols.
kibertoad Jul 6, 2016
7e617e0
refs #7560 Add missing "the".
kibertoad Jul 9, 2016
b5baaa2
refs #7560 Removed excessive detail about internal implementation.
kibertoad Jul 17, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions doc/api/fs.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,9 @@ fs.appendFile('message.txt', 'data to append', 'utf8', callback);

Any specified file descriptor has to have been opened for appending.

_Note: Specified file descriptors will not be closed automatically._
_Note: If a file descriptor is specified as the `file`, it will not be closed
automatically. If a filename is specified as the `file`, the internally opened
file descriptor will be closed automatically._

## fs.appendFileSync(file, data[, options])
<!-- YAML
Expand Down Expand Up @@ -1130,7 +1132,9 @@ fs.readFile('/etc/passwd', 'utf8', callback);

Any specified file descriptor has to support reading.

_Note: Specified file descriptors will not be closed automatically._
_Note: If a file descriptor is specified as the `file`, it will not be closed
automatically. If a filename is specified as the `file`, the internally opened
file descriptor will be closed automatically._

## fs.readFileSync(file[, options])
<!-- YAML
Expand Down Expand Up @@ -1662,7 +1666,9 @@ Note that it is unsafe to use `fs.writeFile` multiple times on the same file
without waiting for the callback. For this scenario,
`fs.createWriteStream` is strongly recommended.

_Note: Specified file descriptors will not be closed automatically._
_Note: If a file descriptor is specified as the `file`, it will not be closed
automatically. If a filename is specified as the `file`, the internally opened
file descriptor will be closed automatically._
Copy link
Contributor

Choose a reason for hiding this comment

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

If the same text has to be repeated multiple times, wouldn't it be better to have it as a separate section and link it wherever necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not such a long text and it didn't bother anyone about the original text. Wouldn't it unnecessarily complicate things? Are there any existing precedents of similar treatment of three-liners?

Copy link
Member

Choose a reason for hiding this comment

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

Btw, while that second sentence is true, I’m not sure it’s actually relevant; that writing to a file based on its name includes opening and closing a file descriptor seems like an implementation detail to me which the user doesn’t need to know anything about.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough. Do you think that the second sentence should be removed altogether? I'm not entirely sure there is no value in explicitly telling the user that he doesn't need to worry about releasing resources in some of the use cases (as opposed to another documented case when he, indeed, should worry), but if you think that it's already implied in a clear enough way in the first sentence, I'm fine with dropping it.

Copy link
Member

Choose a reason for hiding this comment

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

I’d be pretty fine with dropping it, too. The first sentence seems clear enough to me.


## fs.writeFileSync(file, data[, options])
<!-- YAML
Expand Down