Naive debouncer

[tau-mask]

Hello,

This PR is an attempt at providing a simple debouncer that could be good enough for the project to move on.

As far as I can tell, any debouncer that tries to check for the files' presence on the filesystem is bound to fail eventually. By the time the Haskell library receives an event and tries to access the filesystem, one should assume that a number of other changes might have happened. This is why I removed the check from flushEvents.

I could have followed the state machine as described in #55 but adding more logic to start from an unknown state (since we shouldn't bother with the filesystem) and to cater for the recent event type additions like CloseWrite would have quickly gotten out of hand. I'll let smarter people come up with smarter debouncers.

To that end, I also opted for a Debouncer class, in the hope that other people can plug in their own ideas of debouncers. I do not know if the interface is good enough for reuse. I'm a Haskell beginner so it's hard to tell.

[thomasjm]

Thanks for the contribution @tau-mask ! This looks promising. Any chance you could provide some tests?

I understand the rationale for not handling CloseWrite etc., but it's a little concerning. On Linux, CloseWrite is one of the most useful events because it tells you someone has just written a file and you can now process it. It looks like NaiveDebouncer will debounce a [Modified, CloseWrite] into just Modified, which is unfortunate. If we want to publish a debouncer that does this we'll have to communicate very clearly about these caveats.

Not trying to over-complicate your design, but I was wondering whether it could short-circuit when an event like CloseWrite occurs? I.e. just emit the event it's gathered so far, then emit CloseWrite, and then continue processing the remaining events. This might result in the client getting more events than expected, but might be more correct?

[thomasjm]

What does coherent mean? IIRC you can get some pretty strange event sequences on some platforms. I feel like we can remove this sentence, it should go without saying that a state machine like this should do its best to be sensible with the events it receives.

[tau-mask]

What I was trying to convey is that, in contrast to the original proposal, the NaiveDebouncer will not try to detect any incoherent sequence of events, let's say Added followed by Added, and will not throw, ever. I will make a second pass on the comments to clarify this.

[tau-mask]

Thanks for the contribution @tau-mask ! This looks promising. Any chance you could provide some tests?

I've been looking at the tests already there and I think I can manage to write some for the debouncing logic. This might take me a few days.

I understand the rationale for not handling CloseWrite etc., but it's a little concerning. On Linux, CloseWrite is one of the most useful events because it tells you someone has just written a file and you can now process it. It looks like NaiveDebouncer will debounce a [Modified, CloseWrite] into just Modified, which is unfortunate. If we want to publish a debouncer that does this we'll have to communicate very clearly about these caveats.

Agreed, I was actually trying to come up with a small reloader logic for a web server and not having CloseWrite was what prompted me to look deeper into this project. I think that there's no "one size fits all" when it comes to debouncers, and that's why I made Debouncer a type class. So writing a CloseWriteDebouncer (totally made-on-the-spot name, I hope I can think of something better) could be a solution for waiting for files to have been updated, and it could be set as default.

Not trying to over-complicate your design, but I was wondering whether it could short-circuit when an event like CloseWrite occurs? I.e. just emit the event it's gathered so far, then emit CloseWrite, and then continue processing the remaining events. This might result in the client getting more events than expected, but might be more correct?

I think this is possible if I change the signature of combineEvents to a -> FilePath -> [Event] -> [Event], and some adjustments are made to flushEvent (assuming I correctly understood how makeDebouncedAction works). This looks like a better solution overall, I'm going to give it a try.

[thomasjm]

Btw, I was thinking we could also support ModifiedAttributes (and even future modification types) by adding bits to the StModified* constructors. Something like this:

data InternalState
  = ...
  | StModifiedFirst ModificationInfo
  | StModifiedThen ModificationInfo

data ModificationInfo = ModificationInfo { 
  modificationInfoContents :: Bool
  modificationInfoAttributes :: Bool
}

So these bits get flipped on in response to the corresponding event types, and at the of the day we emit an event for each on bit.

[thomasjm]

Hey @tau-mask, just checking in -- I'd love to get this merged, is it ready for another review?

[tau-mask]

Hi,

sorry I've been quite busy lately and had not much time to spare for this. I want to come up with a solid design, but I lack experience in this area so it's taking some time to mature.

[tau-mask]

Here is my attempt at writing a debouncer focused on file reloading. I can't help but feel that it is not quite ready yet. My main concern is about the CloseWrite event being available exclusively on Linux. Other OSes might see little benefit here and that might not be the right choice for a default debouncer. I also tried to take into account your remarks, but every time I try to do something more detailed, the complexity of the state machine increases too much for a mechanism that should remain simple.

Also, I do not have tests for the code, running the complete test suite takes several minutes on my machine and I couldn't figure out how to make the tests run in parallel (or how to focus on a subset of these).