Correct and document the WinAPI queue logic

[dkorpel]

Doesn't fix the redundant update messages, but it does fix the empty event list inbetween succesive polls. The problem was that after receiving messages from GetOverlappedResult, queued was set to false and no queue was immediately opened afterwards, so an extra getEvents() call was needed before anything could be received.

I also documented the logic and added some comments for future contributors, and removed a redundant path.exist call.

[dkorpel]

I don't really remember anymore why I put that queued in there in the first place, it seems that it should be possible without it too

If I remove queued=false and startWatchQueue it just repeats the last event over en over.

The tests all still pass I assume?

Mmm, no. Messages used to be:

[FileChangeEvent(createSelf, ".", "")]
[FileChangeEvent(create, "a.txt", "")]
[]
[FileChangeEvent(modify, "a.txt", ""), FileChangeEvent(modify, "a.txt", "")]
[]
[FileChangeEvent(rename, "a.txt", "b.txt")]
[]
[FileChangeEvent(remove, "b.txt", "")]
[FileChangeEvent(removeSelf, ".", "")]

Now they are:

[FileChangeEvent(createSelf, ".", "")]
[FileChangeEvent(create, "a.txt", "")]
[FileChangeEvent(modify, "a.txt", "")]
[FileChangeEvent(modify, "a.txt", "")] // < rename expected
[FileChangeEvent(rename, "a.txt", "b.txt")] // < remove b.txt expected
[FileChangeEvent(removeSelf, ".", "")] 

So the old implementation alternated between queued = true and false, where every even getEvents() would detect self changes, and every odd would receive the OverlappedResult in batches of two (?). The new implementation doesn't show empty lists, but it flattens the double modification and the removal of "b.txt" gets overshadowed by the removal of the folder.

[WebFreak001]

hm it's important that the removal of files inside the folder still triggers, in case applications do something based on that event

This is how it currently outputs on linux for reference:

[FileChangeEvent(createSelf, "test", "")]
[FileChangeEvent(create, "a.txt", ""), FileChangeEvent(modify, "a.txt", "")]
[FileChangeEvent(modify, "a.txt", "")]
[FileChangeEvent(rename, "a.txt", "b.txt")]
[FileChangeEvent(remove, "b.txt", "")]
[FileChangeEvent(removeSelf, ".", "")]

The createSelf seems broken, I need to look into that, otherwise it's as expected

[dkorpel]

hm it's important that the removal of files inside the folder still triggers, in case applications do something based on that event

It is, it is just one getEvents() behind. I don't know if you like my test 'fix', maybe the empty arrays inbetween are the lesser of the two evils.

[dkorpel]

I currently put the GetOverlappedResult block in a foreach(_; 0..2) so it does checks again if it found something. If nothing was found by GetOverlappedResult it breaks. Still not very elegant, but the tests pass.

[WebFreak001]

ok sorry I got it wrong with the tests. I'm fine with merging this without the foreach (_; 0 .. 2) "hack", it's just important that the order of events is mostly right, if events trigger multiple times it's fine.

The test function would probably need to check a few more calls of getEvents to make sure there are no events left on the queue.

[dkorpel]

I'm fine with merging this without the foreach (_; 0 .. 2) "hack"

Do you mind it though? I think it gives the best results. I think it solves issue 16 (assuming also receiving notifications from OS-modifications is expected behavior).

The test function would probably need to check a few more calls of getEvents to make sure there are no events left on the queue.

But still return the first message? The test could also check that the right message is among the list of messages.

[WebFreak001]

hm alright, the tests produce the correct result now then? The test cases should really be updated to check lengths eventually

[dkorpel]

the tests produce the correct result now then?

on my machine they succeed

[WebFreak001]

I will push this as 0.4.0 so people can check if anything breaks with the update