e-dant / watcher

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

Stopping the watcher #14

Closed ypujante closed 1 year ago

ypujante commented 1 year ago

Thank you for the library. I have a one comment:

https://github.com/e-dant/watcher/blob/release/sinclude/watcher/watcher.hpp#L838

  static auto callback_hook = callback; // here
  const auto callback_adapter =
      [](ConstFSEventStreamRef, /* stream_ref
                                   (required) */
         auto*,                 /* callback_info (required) */

You can simply pass the hook in the lambda:

  const auto callback_adapter =
      [&callback_hook = callback](ConstFSEventStreamRef, /* stream_ref
                                   (required) */
         auto*,                 /* callback_info (required) */
e-dant commented 1 year ago

Right on both points! Thanks for spotting the first issue.

The second issue is already on my todo list.

Do you think these fancy GitHub discussions would be a good place to put that list?

e-dant commented 1 year ago

I think I can get this done during or before the weekend. There's no reason this should be hard.

ypujante commented 1 year ago

I have never used the discussion section, so I don't really have any opinion. There is also a wiki section which might be more appropriate for a to-do list, maybe?

e-dant commented 1 year ago

Check out the next branch. Let me know if it works for you.

ypujante commented 1 year ago

Thank you for taking my comments into consideration. After looking more into it, I don't think I can use your library for my project as I need to be able to have multiple watchers. The way you have implemented die as a static function that sets a static variable guarantees that there can only be one watcher at a time. There is nothing wrong with this approach, but it just won't work for my particular use case.

e-dant commented 1 year ago

I'll close this as completed when the next branch is merged. I may make a new issue in the future to support several watchers.