Skip to content

docs: clarify eventType in fs.watch #9318

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

Closed
seishun wants to merge 4 commits into from
Closed

docs: clarify eventType in fs.watch #9318

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d627fc2
docs: clarify eventType in fs.watch
seishun Oct 27, 2016
7591c57
nits
seishun Oct 28, 2016
54cd5ec
more nits
seishun Oct 29, 2016
6a88f95
hopefully last nit
seishun Oct 31, 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
7 changes: 5 additions & 2 deletions doc/api/fs.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1702,8 +1702,11 @@ The listener callback gets two arguments `(eventType, filename)`. `eventType` i
`'rename'` or `'change'`, and `filename` is the name of the file which triggered
the event.

Please note the listener callback is attached to the `'change'` event
fired by [`fs.FSWatcher`][], but they are not the same thing.
Note that on most platforms, `'rename'` is also emitted when a file is deleted or added.
Copy link
Member

@Trott Trott Oct 28, 2016

Choose a reason for hiding this comment

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

Nit: remove also

Copy link
Contributor Author

@seishun seishun Oct 28, 2016

Choose a reason for hiding this comment

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

I used "also" because it's in addition to being renamed.

Copy link
Member

@Trott Trott Oct 29, 2016

Choose a reason for hiding this comment

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

I suggested removing it because I think it could be interpreted to mean that both a rename and a change event will be emitted.

I think someone might think it means "A change event is emitted and a rename event is also emitted."

I understand that you mean it as "A rename event is emitted in this case and a rename event is also emitted in this other case."

I'm fine with leaving it the way you have it if you have a strong preference for that. But if it doesn't make much of a difference to you either way, I'd prefer it removed.

Copy link
Contributor Author

@seishun seishun Oct 29, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I feel like with your suggestion the two sentences somewhat contradict each other...

How about a compromise? Just combine them into one sentence: "Note that on most platforms, 'rename' is emitted whenever a filename appears or disappears in the directory."

Copy link
Member

@Trott Trott Oct 29, 2016

Choose a reason for hiding this comment

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

Yes, that works for me.

In other words, it is emitted whenever a filename appears or disappears in the directory.
Copy link
Member

@Trott Trott Oct 28, 2016

Choose a reason for hiding this comment

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

Nit: add on most platforms here too to be as explicit as possible: In other words, on most platforms, it is emitted...


Also note the listener callback is attached to the `'change'` event fired by
[`fs.FSWatcher`][], but it is not the same thing as the `'change'` value of `eventType`.
Copy link
Member

@jasnell jasnell Oct 30, 2016

Choose a reason for hiding this comment

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

nit: long line here.


### Caveats

Expand Down