PathsMut breaking changes discussion

[riberk]

@dfaust @JohnTitor

I want to add some breaking changes but I don't know the policy about that.

It's about the new feature PathsMut introduced in #692

In the current implementation dropping without commit has an unspecified behavior and for fsevents it keeps a watcher stopped BUT if we call watch / unwatch in a row a watcher will be started. I think it's quite bad and inconsistent.

I think it's not a big breaking change, because not really a lot of users implement Watcher by themselves.

I suggest adding api described below into the Watcher:

trait Watcher {
    // We can implement it once there and implement special case for fsevents / pool
    fn update_watches(&mut self, ops: Vec<WatchOp>) -> std::Result<(), UpdateWatchesError>;

    // We could implement it with no breaking change in the trait, but the implementation
    // in that case would be really strange, like in the PR above, when the method returns 
    // a Box<dyn PathsMut<'me>> - a confusing signature (IMO)
    fn paths_mut<'me>(&'me mut self) -> PathsMut<'me>;
}

/// Error provided by [`Watcher::update_watches`]
pub struct UpdateWatchesError {
    
    /// What the operation by what the index caused the error
    pub error_on: (usize, WatchOp),
    
    /// The error
    pub error: Error,

    /// What operations haven't been applied
    pub rest: Vec<WatchOp>,
}

pub enum WatchOp {
    Watch(PathBuf, RecursiveMode),
    Unwatch(PathBuf),
}

/// Just a helper to call update_watches
pub struct PathsMut<'a> {
    ops: Vec<WatchOp>,
    watches: &'a mut dyn Watcher,
}

impl<'a> PathsMut<'a> {
    pub fn new<W: Watcher + 'a>(watcher: &'a mut W) { ... }
    pub fn watch(path: PathBuf, recursive_mode: RecursiveMode) { ... }
    pub fn unwatch(path: PathBuf) { ... }
    pub fn commit(self) -> std::Result<(), UpdateWatchesError> { 
        self.watcher.update_watches(self.ops)
    }
}

update_watches takes a Vec instead of a slice because I want to reimplement this pr #632 and it will require taking some Filter by value. And the code does so much file system stuff that some memory allocation doesn't matter.

If you agree with that, I'll implement it tomorrow and make a PR.

[riberk]

Oh, I've just noticed the Watcher::kind, there couldn't be any Watcher implementation outside the crate, then it's not breaking until previous version of PathsMut is released

Of course an outside implementation could return inconsistent kind, so, it's still breaking