notify
notify copied to clipboard
Add method for native watcher initialization with fallback
Ads a new method for initializing the recommended watcher and falling back to pollwatcher on failure to do so
Fixes #423
This is a draft because:
- I'd like to also change the
recommended_watcher
function to just call the new one, giving it more value, but would have to change its result value. Not sure if we can do that semver compatible (or just ignore it). - I've opted for an enum type instead of
Box<dyn Watcher>
, I could also imagine using that. - I've only handled the specific #423 error. We could also fallback for any error.
- Do we want to report the original error with the fallback ? Add some additional error kind "unavailable API" to at least report it?
- This requires
EventHandler
to be declared eitherCopy
orClone
for this fallback function. Even more of a SemVer break forrecommended_watcher
Todo:
- [ ] Add docs entry in the issue for using that
- [ ] Change example to use the new function
- [ ] allow debouncer init with fallback (maybe add an easier init)
CC @samuelcolvin as possible consumer
- I'd like to also change the
recommended_watcher
function to just call the new one, giving it more value, but would have to change its result value. Not sure if we can do that semver compatible (or just ignore it).
I'd call it a breaking change if it changes behavior.
- I've opted for an enum type instead of
Box<dyn Watcher>
, I could also imagine using that.
I think it's totally fine while we have take_boxed
.
- I've only handled the specific
Function not implemented (os error 38)
when running on docker in M1 MacOS #423 error. We could also fallback for any error.
These could come later :)
- Do we want to report the original error with the fallback ? Add some additional error kind "unavailable API" to at least report it?
I don't have a strong opinion on that, maybe we could prepare an optional value?
- This requires
EventHandler
to be declared eitherCopy
orClone
for this fallback function. Even more of a SemVer break forrecommended_watcher
Hmm, that sounds like a rather difficult issue and I cannot prepare a solution for that soon (I'm now fixing my dev env and cannot check your code deeply).
Please review again, the addition of Deref/DerefMut allows direct access to the watcher in our enum, making the whole stack allocation worthwhile, such that boxing is not required. Example
Open Questions:
- Do we deprecate
recommended_watcher()
to reflect that it's kind of unhelpful as a function (in light of RecommendedWatcher) ? - Do we migrate all examples (except for one) and the docs to recommend using
recommended_watcher_fallback
overRecommendedWatcher
? Technically it is the only way which we can get to actually work on all systems (and can work around future issues). In a way comptime-selection was fun while it lasted. On the other hand a lot of people don't seem to care about the specific issue on M1 + Docker (you can also say people should not expect such weird emulator setups to work), but I don't see a drawback for people already usingRecommendedWatcher
to migrate to the dynamic approach. - Bikeshed: Is there a better name than
recommended_watcher_fallback
? It's kinda long when for normal users I'd say it is the recommended way. - I have no good answer to the copy/clone requirement of the fallback logic. We need it for
recommended_watcher_fallback
, so it's an additional requirement. We may just leave it as that.
Bikeshed: Is there a better name than recommended_watcher_fallback ? It's kinda long when for normal users I'd say it is the recommended way. @0xpr03
How about ensure_watcher
? From my experience, ensure has a stronger implication that a resource will be obtained.