Improve watcher #113
Merged
Improve watcher #113
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Profpatsch
force-pushed
the
improve-watcher
branch
3 times, most recently
from
ad46dad to
b1bbb2a
Compare
The tests were kinda written in a weird way, instead of using the `recv_timeout` function they would just hope events have arrived when the `collect_all` function is called. Note that we fix the idea behind the `macos_eat_late_notifications` code by removing the `.is_empty()` again (which we don’t want!). Instead we really just call `try_iter` to throw away all intermediate messages.
Profpatsch
force-pushed
the
improve-watcher
branch
from
d5d33ee to
43f5409
Compare
We would `panic!` if the watch implementation encountered a problem reading a file event etc., but really we shouldn’t care. Instead, just throw a warning to make it debuggable, but continue on watching. I inlined the unnecessary `log_event` helper.
We only return `false` for Modify events in our gcroots dir, otherwise the `path_match` result.
It’s just a better environment that way … sigh
really trivial function, can get rid of the test as well.
All the times we add a path to `watches()`, it is first canonicalized. So we document this invariant, and can remove the extra canonicalization step from `path_match()`. I also inline the function let binding.
This was not at all clear to me, but now it should be good for the next person.
These two structs are only used for preventing automatic cleaning of these resources, so their content is never used. Newer rust will warn about that for structs as well.
Profpatsch
force-pushed
the
improve-watcher
branch
from
43f5409 to
f69b612
Compare
We need to create a subdir, otherwise the tests will try to watch `/tmp` … This was hidden before because the tests were buggy in the way they handled events.
I just cannot figure out how to fix this test. I think it was always kinda broken, but it’s hard to see what is wrong, because it might be the filtering we do or some race condition in how events are generated.
|
Ah great, I forgot to mark this as ready for review. |
Merged
|
I extended this in #125 such that it passes the Vim tests as well. I think those changes might also marginally reduce the number of builds Lorri does |
|
Okay, I’m gonna merge here and rebase the other one since that builds on this one. 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
release.nix(seerelease.nixfor instructions)