-
Notifications
You must be signed in to change notification settings - Fork 103
Support new formatter rules introduced in fmt v10 and v11 #174
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
base: master
Are you sure you want to change the base?
Conversation
|
I have formatted the commit, please review again. the commit is tested with both fmt v8.0.1(the original) and fmt v11.1.4(the newest). Updating the fmt also requires a compatible version of spdlog. Changes:
|
|
Thank you for your contribution. We are excited to announce an open-source rewards event—every valid merge request qualifies for a reward! For more details, please join the Feishu and WeChat groups listed in our documentation or contact us directly. All prizes will be distributed collectively at the end of the event. |
JackLau1222
left a comment
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.
First of all, thanks very much for your great work.
However, this multi-version compatibility issue involves API changes. It may be more appropriate to get the version number in the cmake build and then implement it according to the version in the code.
|
The patch is compatible with fmt versions ranging from v8 to the latest v11. Therefore, I believe there is no need to modify the CMake build script. |
|
UPDATE: support both char and wchar_t |
JackLau1222
left a comment
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.
there're some conflicts with latest commit, may you rebase this patch?
1. make custom formatter const 2. make default enum formatter for non-stringfy type 3. fmt::join is moved to <fmt/ranges.h> 4. some source files are not properly include the enum formatter
|
rebase to main with no conflicts |
|
In the GPU environment, compilation fails with fmt versions 10.0.0 and 11.0.2. It's likely that some fmt functions used exclusively in the GPU context require version-specific adaptations. |
|
include the header <hmp/format.h> resolves the cudaError_enum issue. |
|
still have compilation errors when using fmt 10.0.0 (tag a0b8a92e3d1532361c2f7feb63babc5c18d00ef2) in the GPU environment |
Add the line By the way, this may probably reduce the final package size. I GUESS. |
|
Hi! 👋 This patch would be very helpful for conda-forge, where we generally only build for one fmt version across the entire ecosystem, and we've been on fmt v11 for a long time already. The fact that bmf is not yet compatible with this makes it difficult to keep the package up-to-date and usable. I've tried backporting this patch in conda-forge/bmf-feedstock#36 (on top of v0.1.0), and it still fails. |
This PR updates our codebase to comply with the new formatting rules and API changes introduced in fmtlib v10+ while keeping the compatity of fmt v8.
Key changes include:
relate issue: #173