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

Add option to ignore paths when setting watches. #107

Closed
wants to merge 1 commit into from

Conversation

imsodin
Copy link
Contributor

@imsodin imsodin commented Aug 21, 2016

This is a modified version of Zillode#3 and adds (some of) the capability requested in #79. syncthing-inotify uses the mentioned fork with this functionality and it works fine. The main advantage over the status quo (filtering events out after generation) is that this saves watch handles (and is slightly more efficient).

The changes are completely backward compatible. Nothing changes for the existing Watch function. The new functionality is included in a new function WatchWithIgnoring. This function takes the function variable doNotWatch in addition to the usual arguments. This function should return true if given a path (string) that is to be ignored. This is implemented by returning different functions in recFunc depending on whether Watch (no change) or WatchWithIgnoring (check doNotWatch at beginning) is used.

I would appreciate your feedback.

@ppknap
Copy link
Collaborator

ppknap commented Feb 28, 2017

We cannot merge this PR since it is not cross-platform - doNotWatch functor will not work on pure-recursive watchers like Windows's ReadDirectoryChangesW.

@imsodin
Copy link
Contributor Author

imsodin commented Feb 28, 2017

Thanks for responding!
I understand that my implementation is only partial (i.e. only having an effect on nonrecursive trees and behaving just like a normal watch on recursive ones) and thus not mergeable. I looked at the recursive tree and it looks like it would be much more involved to do something similar there. I don't really have time right now to look into that.

@ppknap
Copy link
Collaborator

ppknap commented Nov 14, 2017

@imsodin I'm closing this PR for now. We can reopen it again when a cross-platform solution is implemented.

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