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

When adding a previously-deleted file, emit 'add', not 'change' #382

Closed
wants to merge 1 commit into from

Conversation

erezarnon
Copy link
Contributor

Turns out #379 is an issue for files as well as directories, but manifests differently.

@erezarnon
Copy link
Contributor Author

I've fixed the issue on linux. My test fails on osx, but I can't fix it because I don't have a mac. First version of this pull request was with commit erezarnon@db6f928, which failed even more osx tests than the current commit. I hoped maybe the issue didn't exist on osx, so tried to fix with c5a3c2e by checking for !this.options.useFsEvents. But now the issue still exists on osx.

@es128
Copy link
Collaborator

es128 commented Nov 2, 2015

This scenario (almost) is already tested. https://github.com/paulmillr/chokidar/blob/master/test.js#L352

If a file is unlinked and re-added in quick succession, it is represented as a change event deliberately by default (with atomic: true).

So perhaps basing the change on if (!this.options.atomic) would be the appropriate approach.

@erezarnon
Copy link
Contributor Author

The mapping of add->change still works. If we base it on this.options.atomic, the bug will exist when there's a delay between unlink and add. Also, I think Chokidar should understand internally that the event is file creation, even if it will map the event before emitting it.

Either way, this would still fail the test cases, right?

@es128
Copy link
Collaborator

es128 commented Nov 2, 2015

Ok, right. I'll need to investigate the Mac situation.

@es128
Copy link
Collaborator

es128 commented Nov 11, 2015

Thanks! Landed as 5a6315f

@es128 es128 closed this Nov 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants