Filter watch targets (+ Windows and kqueue support) #717
Conversation
…r-watch-targets # Conflicts: # notify/src/fsevent.rs # notify/src/inotify.rs # notify/src/lib.rs
boenrobot
force-pushed
the
filter-watch-targets
branch
from
43633b5 to
b91b6c8
Compare
boenrobot
changed the title
|
|
||
| for path in add_watches { | ||
| self.add_watch(path, true).ok(); | ||
| self.add_watch(path, true, WatchFilter::accept_all()).ok(); |
There was a problem hiding this comment.
I can't put my finger on the exact scenario, but this feels wrong... I feel like I should be inferring the filter out of a previously saved copy in self.watches... But then if that is what should be happening, I'm not sure how to efficiently determine the correct watch entry to fetch it from, and combined with the lack of being able to test locally, I'm reluctant to try playing around.
EDIT: I think I may have accidentally also stumbled upon the cause of #644. Like I said, I'm not sure how to fix it, but I think that if this line can somehow take the recursive (and now the filter) out of the originally added watch, this inconsistency will be cleared.
boenrobot
force-pushed
the
filter-watch-targets
branch
from
2a72d78 to
1b1ad1d
Compare
boenrobot
force-pushed
the
filter-watch-targets
branch
from
140a942 to
a0227a1
Compare
boenrobot
force-pushed
the
filter-watch-targets
branch
from
70acf95 to
c775a64
Compare
JohnTitor
left a comment
JohnTitor
left a comment
There was a problem hiding this comment.
I'd accept only if this has tests, and if you don't have env to test, it's better to start with envs you know. We can expand support later.
2 participants
Picks up where #632 left off.
I resolved the conflicts, and in a separate commit, also added support for Windows... I'm not sure if that Windows supports really works properly to be honest, as there's no tests in #632, and I'm not sure how to even write a proper test for this (I'm new to Rust...).
But certainly all the tests pass and there's no warnings (except one in windows.rs that was there from before this PR and not introduced by #632 either), so... that's a good start.
EDIT: OK, I see CI shows kqueue breaking due to lack of implementation in both #632 and this one... I'll see if I can implement it, though I should note I am only using Windows, and have WSL... No FreeBSD or MacOS available, so I'll rely on the CI...Also implemented kqueue, though once again, without any tests... Just the build and existing tests passing.