Change EventFn to take FnMut #333
Conversation
This changes EventFn to work with FnMut, rather than Fn. The advantage of this is that this allows callers to use state in the EventFn, rather than having to use interior mutability. This is helpful particularly for getting notify to work with async rust, since `futures::channel::mpsc`'s methods all take `&mut self`. The downside of this change is that it limits the ability of notify to share `EventFn` across multiple threads, but all the backends either guarantee only a single background thread, or they already use an `Arc<Mutex<...>>` to protect `EventFn`.
This adds a new example on how to lift events from the synchronous code into async rust.
There was a problem hiding this comment.
but all the backends either guarantee only a single background thread, or they already use an Arc<Mutex<...>> to protect EventFn
That looks fine but can/do we somehow guarantee/note that we now expect only a single Fn caller at any point in time in the backends?
|
Also thanks for all the changes. Based on your contributions, do you maybe want to be part of the team to maintain this crate, especially since you seem to have an apple device to test the fsevent part ? |
That should be implied by using
Thanks for the offer! I can try to help here and there, but I don't think I can commit to maintenance at this time. I think with these last couple changes addresses all my needs, and I can unblock my actual use of notify :) |
I think we'll leave that for a later PR and merge this one in the meantime.
No problem, let's see when your next issue comes up ;) |
This changes EventFn to work with FnMut, rather than Fn. The advantage of this is that this allows callers to use state in the EventFn, rather than having to use interior mutability. This is helpful particularly for getting notify to work with async rust, since
futures::channel::mpsc's methods all take&mut self.The downside of this change is that it limits the ability of notify to share
EventFnacross multiple threads, but all the backends either guarantee only a single background thread, or they already use anArc<Mutex<...>>to protectEventFn.