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

doc: some fs doc improvements #17692

Closed
wants to merge 3 commits into from
Closed

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Dec 15, 2017

Some improvements

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

fs doc

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. labels Dec 15, 2017
doc/api/fs.md Outdated
following is prone to error:
Note that there is no guaranteed ordering when using asynchronous methods.
So the following is prone to error because the `fs.stat()` operation may
completely before the `fs.rename()` operation.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be completely -> complete instead?

doc/api/fs.md Outdated
* Returns: {boolean}

Returns `true` if the `fs.Stats` object describes a character device.
### stats.isDirectory()
Copy link
Member

Choose a reason for hiding this comment

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

Nit: can we add a new line before this for consistency?

doc/api/fs.md Outdated
* Returns: {boolean}

Returns `true` if the `fs.Stats` object describes a file system directory.
### stats.isFIFO()
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

doc/api/fs.md Outdated

* Value: {number}

The timestamp indicating the last time this files status was changed expressed
Copy link
Member

Choose a reason for hiding this comment

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

:%s/files/file

Copy link
Member

Choose a reason for hiding this comment

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

this files -> the file or this file's

doc/api/fs.md Outdated

* Value: {Date}

The timestamp indicating the last time this files status was changed.
Copy link
Member

Choose a reason for hiding this comment

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

:%s/files/file

Copy link
Member

Choose a reason for hiding this comment

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

this files -> the file or this file's

doc/api/fs.md Outdated
The `fs` module provides an API for interacting with the file system in a
manner closely modeled around standard POSIX functions.

To use this module, use:
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt Dec 15, 2017

Choose a reason for hiding this comment

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

A nit: I am not sure, but are these two use a bit monotonous?

doc/api/fs.md Outdated
Most `fs` operations accept filepaths that may be specified in the form of
a string, a [`Buffer`][], or a [`URL`][] object using the `file:` protocol.

String form paths are interpretted as UTF-8 character sequences identifying
Copy link
Contributor

Choose a reason for hiding this comment

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

interpretted -> interpreted ?

doc/api/fs.md Outdated
Example using an absolute path on POSIX:

```js
fs.open(Buffer.from('/open/some/file.txt'), (err, fd) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing flag? ('r'?)

doc/api/fs.md Outdated
operations use these file descriptors to identify and track each specific
file. Windows systems use a different but conceptually similar mechanism for
tracking resources. To simplify things for users, Node.js abstracts away the
specific differences between operating sytems and assigns all open files a
Copy link
Contributor

Choose a reason for hiding this comment

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

sytems -> systems


The object itself emits these events:
All `fs.FSWatcher` objects are [`EventEmitter`][]'s that will emit a `'change'`
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing reference?

doc/api/fs.md Outdated
@@ -321,20 +401,12 @@ changes:
description: Added times as numbers.
-->

An `fs.Stats` object provides information about a file.
Copy link
Contributor

Choose a reason for hiding this comment

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

An -> A ?

Returns `true` if the `fs.Stats` object describes a symbolic link.

*Note*: This method is only valid when using [`fs.lstat()`][]

Copy link
Contributor

@vsemozhetbyt vsemozhetbyt Dec 15, 2017

Choose a reason for hiding this comment

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

A nit: delete the extra new line?

doc/api/fs.md Outdated
@@ -1715,6 +1936,11 @@ a colon, Node.js will open a file system stream, as described by
Functions based on `fs.open()` exhibit this behavior as well. eg.
`fs.writeFile()`, `fs.readFile()`, etc.

*Note:* On Windows, opening an existing hidden file using the `w` flag (either
through `fs.open` or `fs.writeFile`) will fail with `EPERM`. Existing hidden
Copy link
Contributor

Choose a reason for hiding this comment

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

fs.open -> fs.open() ?
fs.writeFile -> fs.writeFile() ?

doc/api/fs.md Outdated
@@ -1715,6 +1936,11 @@ a colon, Node.js will open a file system stream, as described by
Functions based on `fs.open()` exhibit this behavior as well. eg.
`fs.writeFile()`, `fs.readFile()`, etc.

*Note:* On Windows, opening an existing hidden file using the `w` flag (either
through `fs.open` or `fs.writeFile`) will fail with `EPERM`. Existing hidden
files can be opened for writing with the `r+` flag. A call to `fs.ftruncate` can
Copy link
Contributor

Choose a reason for hiding this comment

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

fs.ftruncate -> fs.ftruncate() ?

doc/api/fs.md Outdated
@@ -3324,6 +3325,7 @@ The following constants are meant for use with the [`fs.Stats`][] object's
[`URL`]: url.html#url_the_whatwg_url_api
[`UV_THREADPOOL_SIZE`]: cli.html#cli_uv_threadpool_size_size
[`WriteStream`]: #fs_class_fs_writestream
[`EventEmitter']: events.html
Copy link
Contributor

Choose a reason for hiding this comment

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

Erroneous single quote instead of backtick?

@jasnell
Copy link
Member Author

jasnell commented Dec 18, 2017

jasnell added a commit that referenced this pull request Dec 18, 2017
PR-URL: #17692
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
@jasnell
Copy link
Member Author

jasnell commented Dec 18, 2017

Landed in 400e73a

@jasnell jasnell closed this Dec 18, 2017
MylesBorins pushed a commit that referenced this pull request Jan 8, 2018
PR-URL: #17692
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 9, 2018
PR-URL: #17692
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jan 10, 2018
@MylesBorins
Copy link
Contributor

Are these changes applicable to 6.x or 8.x?

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. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants