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

Incomplete FSWatcher documentation. #7504

Closed
cpojer opened this issue Jul 1, 2016 · 3 comments
Closed

Incomplete FSWatcher documentation. #7504

cpojer opened this issue Jul 1, 2016 · 3 comments
Labels
doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system.

Comments

@cpojer
Copy link

cpojer commented Jul 1, 2016

Only the change and error events are currently documented but there are other events, such as rename.

See: https://nodejs.org/docs/latest/api/fs.html#fs_event_change

Also, I don't quite understand why a git branch change would cause a rename on existing files – anyone care to educate me? :)

@mscdex mscdex added doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. labels Jul 1, 2016
@Trott
Copy link
Member

Trott commented Jul 1, 2016

I think you are misunderstanding the situation/documentation, which is understandable because both the situation and the documentation are a little confusing.

If I recall correctly, there is no rename event on FSWatcher. However, the callback to fs.watch() (that's a link to the relevant part of the doc right there, which may be worth following along with) receives a string (not an event, but confusingly called event in the doc) and that string can be either 'change' or 'rename'. However those are not events. Those are merely strings describing the type of (get ready to be confused) change event that was fired under the hood.

At least that's my understanding/recollection. Correction welcome. And if I'm not wrong, a pull request that made the docs easier to understand would also be welcome.

/cc @nodejs/documentation

@cpojer
Copy link
Author

cpojer commented Jul 1, 2016

You are right, I was jumping from fs.watch to fs.FSWatcher in the docs and missed this.

@claudiorodriguez
Copy link
Contributor

claudiorodriguez commented Jul 1, 2016

Made a PR but I'd appreciate feedback on whether it actually clarifies the issue.

claudiorodriguez added a commit to claudiorodriguez/node that referenced this issue Jul 2, 2016
The name 'event' for the argument of the listener in
fs.watch was confusing considering FSWatcher also had
events. This changes the name of the argument to
eventType.

Fixes: nodejs#7504
PR-URL: nodejs#7506
evanlucas pushed a commit that referenced this issue Jul 18, 2016
The name 'event' for the argument of the listener in
fs.watch was confusing considering FSWatcher also had
events. This changes the name of the argument to
eventType.

Fixes: #7504
PR-URL: #7506
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
evanlucas pushed a commit that referenced this issue Jul 20, 2016
The name 'event' for the argument of the listener in
fs.watch was confusing considering FSWatcher also had
events. This changes the name of the argument to
eventType.

Fixes: #7504
PR-URL: #7506
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
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 a pull request may close this issue.

4 participants