e-dant / watcher

Filesystem watcher. Works anywhere. Simple, efficient and friendly.
MIT License
639 stars 32 forks source link

Update windows watcher to send a single rename event with both the old and new path #45

Closed iwubcode closed 1 month ago

iwubcode commented 1 month ago

Here's the fix for #44 .

A general comment for you to consider in the future, could we avoid heap allocation by using an optional instead? This would also allow us to leverage the copy constructor here.

e-dant commented 1 month ago

could we avoid heap allocation by using an optional instead? This would also allow us to leverage the copy constructor here.

Something like that sounds nice. However, we can't avoid a heap allocation unless we make the event structure non-recursive. (Need some indirection in that case.)

It's recursive because:

Currently, the only possible associated event is a renamed-to event. In the future, we may store other kinds of associated events there. Ownership changes are a good candidate. This kind of structure allows more room for future changes if needed, although admittedly the intended use of associated events may be a bit less obvious to the user than I’d like.

It was meant to give us some room for future changes. There are no filesystem events that require more than a pair of events, so it might not be necessary at all.

We can probably just store a bare event lookalike with just the path/path-type/event-type/event-time in an optional or something similar.

e-dant commented 1 month ago

This can go out in a patch release soon.

Our Conan friends tend to be really on top of updates, so if that's something you use then this should be available there some time shortly after.