-
-
Notifications
You must be signed in to change notification settings - Fork 168
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: std::format support for dpp::snowflake #1203
Conversation
✅ Deploy Preview for dpp-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
…or mac os clang14 by disabling formatting
tested this on Windows and got |
So with last commit i have introduced usage of Cmake feature called configure_file which allows us autogenerate headers with some definitions when build dpp, this way we can remove need to define DPP_HAS_FORMAT as it will be defined in it when compiling it. In theory same way we can eliminate need to specify -DDPP_CORO when compiling bot with dpp with coro enabled in lib. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
theres a massive problem with this!
If cmake detects that C++20 is available it will always enable std::format. This means the user is forced to use -std=c++20 for all their bots, even where they dont want to use C++20. This fundamentally breaks compatibility.
Nothing tells the user they must use C++20 anywhere, and furthermore, if they have a need for C++17 they simply cannot ever use C++17 any more.
If lib is build with c++20 then ye, bots will be required to use c++20, I can easily fix it by wrapping cmake define into check of c++ version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy with it but need to see what @braindigitalis thinks about the autoconfig.h
idea.
I have tested changes locally on my wsl2 (ubuntu) and tests passed, also compiling bot with -DPP_FORMATTERS and without had required behavior |
This pr adds std::format support for dpp::snowflake, if we do not have c++20 or have no include for it will be automatically disabled for compilation.
Test case was implemented to check formatting.
Code change checklist