search

e-dant / watcher

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

is C++17 standard no longer supported? #32

Closed saukijan closed 10 months ago

saukijan commented 1 year ago

Hello @toge and @e-dant,

I wanted to use this project as a header-only library via conan-center, but found out that the minimum C++ standard is set to C++20 and not C++17.

in v0.2.2 changelog, it is stated that:

C++20 Concepts are removed for being very interesting but not beneficial enough to an API with a very small surface.

However, in e28bd3c commit wtr.watcher.cmake the minimum standard was changed from 17 to 20. Is this only for the standalone program or is it also important for the header-only library files?

e-dant commented 1 year ago

Hi there. Thanks for your interest.

I think C++17 should work for this project mostly as-is. I can try to get a release out which says so in the minimum C++ version.

Coroutines were something I thought about from time to time. I heard they work well with boost.asio. It might have been an avenue to support folks who want to run this in an asio context without wrapping it up, thread and all, into something asio likes.

I haven't explored coroutines too much, and C++17 is worth supporting.

I can try to get a release out with support for C++17 within a few week(end)s.

Thanks for your interest in the project.

e-dant commented 1 year ago

I think this is the only line which prevents C++17 from working.

I can fix that.

saukijan commented 1 year ago

Hey @e-dant, thanks for the quick reply! Looking forward to the next release.

Coroutines were something I thought about from time to time. I heard they work well with boost.asio. It might have been an avenue to support folks who want to run this in an asio context without wrapping it up, thread and all, into something asio likes.

I don't have any experience with coroutines myself, but I have used standalone asio before. It's a nice library (especially, since you can use it without boost, if you don't need anything else from it), though it can be a bit confusing at times due to its age. I think coroutines were introduced back in 2010 in asio, so they might look and feel a bit different than the C++20 coroutines.

saukijan commented 1 year ago

Regarding this issue, should I close it now or do you want to close it yourself when C++17 compatibility is available?

e-dant commented 1 year ago

Regarding this issue, should I close it now or do you want to close it yourself when C++17 compatibility is available?

Let's close it later on, when support is properly available.

Should have the next release out soon.

e-dant commented 1 year ago

Will release after a few votes on this poll:

https://github.com/e-dant/watcher/discussions/34

The next branch is otherwise stable.

Shouldn't be much more work than a search and replace if those field names change around.

I don't think we'll be up on Conan until after the release. IIUC, that's what you're waiting on.

saukijan commented 11 months ago

Hi @e-dant, I noticed that there is a new release, does this mean this issue is resolved?

Also, do you need any help releasing the conan package?

e-dant commented 11 months ago

Hi @e-dant, I noticed that there is a new release, does this mean this issue is resolved?

Also, do you need any help releasing the conan package?

The latest release should allow support for C++17, but that version isn't up on Conan yet. I think there's a PR for it, waiting on conan center to approve or request changes.

saukijan commented 11 months ago

Ahhh, thanks for the info. I did not check the PRs section on conan-center, silly me.

You are right, PR #19512 holds all the latest changes and seems to be done. It looks like it will be merged in soon.

This issue is resolved from my side then. Feel free to close it and thanks for your great work.