Skip to content
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

Optionally use std::format in LOG4CXX_XXXX_FMT macros instead of fmt::format #291

Merged
merged 7 commits into from
Nov 18, 2023

Conversation

swebb2066
Copy link
Contributor

@swebb2066 swebb2066 commented Nov 16, 2023

This PR allows the following usage options:

  1. Add -DLOG4CXX_FORMAT_NAMESPACE=std to the cmake configure step when building Log4cxx
  2. Add #define LOG4CXX_FORMAT_NS std before #include <log4cxx/logger.h>

@rm5248
Copy link
Contributor

rm5248 commented Nov 16, 2023

Would it make sense to be able to switch this at compile time for the user? We can turn off location information by defining the preprocessor argument LOG4CXX_DISABLE_LOCATION_INFO, we could do something similar here. That can also let the user select between the two if needed, since fmt has more comprehensive options than std::format.

At least part of this could be done automatically by checking for __cpp_lib_format

@swebb2066
Copy link
Contributor Author

swebb2066 commented Nov 17, 2023

Would it make sense to be able to switch this at compile time for the user?

Yes, which is done by adding #define LOG4CXX_FORMAT_NS std before #include <log4cxx/logger.h> or LOG4CXX_FORMAT_NS=std as a preprocessor argument

@rm5248
Copy link
Contributor

rm5248 commented Nov 17, 2023

Would it make sense to be able to switch this at compile time for the user?

Yes, which is done by adding #define LOG4CXX_FORMAT_NS std before #include <log4cxx/logger.h> or LOG4CXX_FORMAT_NS=std as a preprocessor argument

It doesn't look like it checks for that though, I think you would get a warning/error about multiple defines. I would assume it actually needs to be this:

#define LOG4CXX_USING_STD_FORMAT @LOG4CXX_USE_STANDARD_FORMAT@
#if !defined(LOG4CXX_FORMAT_NS) && LOG4CXX_USING_STD_FORMAT
#define LOG4CXX_FORMAT_NS std
#elif !defined(LOG4CXX_FORMAT_NS)
#define LOG4CXX_FORMAT_NS fmt
#endif

This information should probably also be added to the macros influencing log4cxx page.

@swebb2066
Copy link
Contributor Author

swebb2066 commented Nov 17, 2023

Updated website is available for review

@swebb2066 swebb2066 merged commit 21f0f12 into master Nov 18, 2023
24 checks passed
@swebb2066 swebb2066 deleted the support_std_format branch November 18, 2023 03:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants