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

Possibly fixes issue #44 -- Improves how file system watcher handles replaced files #2158

Merged
merged 2 commits into from Jul 16, 2019

Conversation

@gauauu
Copy link
Contributor

gauauu commented Jul 16, 2019

It appears that when this bug ( #44 ) happens, the watcher starts handling the filesystem change before the file actually gets recreated. So the new version of the file doesn't exist, and never gets added back to the watcher.

This PR moves the functionality of re-adding the file to the watcher into the timeout handler. In my tests, this gives it enough time for the file to exist again, and thus gets re-added.

I'm not well-versed in QT, so if I should be handling the iteration with a different type of Iterator, or use a different style here, please let me know.

gauauu and others added 2 commits Jul 15, 2019
* Reuse the list that is created to ease iteration.
* Re-watch the files before emitting filesChanged (probably doesn't
  matter, but I think it's a better behavior regarding outside visibility)
* Make sure we don't re-watch removed files by checking mWatchCount.
@bjorn bjorn force-pushed the gauauu:fix_file_watcher branch from a2c759e to b1ab96e Jul 16, 2019
@bjorn

This comment has been minimized.

Copy link
Owner

bjorn commented Jul 16, 2019

I've made some tweaks as you can see and I've removed the merge commits. If you need help getting rid of those yourself let me know on Discord. :-)

I'm pretty confident this change will fix issue #44, thanks for looking into it!

@bjorn bjorn merged commit 2899664 into bjorn:master Jul 16, 2019
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.