make level_enum from_str() case insensitive #3525#3531
make level_enum from_str() case insensitive #3525#3531neundorf wants to merge 1 commit intogabime:v1.xfrom
Conversation
|
@gabime This patch makes from_str() case-insensitive. |
|
IMO. Common symbols used in spdlog are defined in Another way: Since |
This patch changes the behaviour of from_str() so that the level names are compared case-insensitive. This has the side effect of making load_levels() work if the names were set to UPPER oder CamelCase in tweakme.h. Beside that, now also the special cases for "off", "err" and "warn" now check that the names are actually "off", "error" and "warning" and have not been set to something else. It would be strange behaviour if I would set the name of the error level e.g. to "Fehler" and spdlog would recognize "fehler" and also "err", but not "error" as that level. The test also checks a mixed case string now, and also "err".
neundorf
force-pushed
the
CaseInsensitiveFromStr
branch
from
7e1f05e to
d490998
Compare
|
@tt4g what do you think about my current version ? |
|
@neundorf I'm concerned that the EDIT: This API might be reused in other spdlog code, so it might be better to have a declaration. |
|
Hmm, to_lower() is currently only used locally in strings_equal_ci(),which is in common.h. |
|
Yes. Whether to export |
| #if __cplusplus >= 201703L | ||
| constexpr | ||
| #endif | ||
| SPDLOG_API char to_lower(char ch) { |
There was a problem hiding this comment.
why SPDLOG_API. I would expect SPDLOG_INLINE
| // | ||
| // Returns true if two strings are equal, case-insensitive | ||
| // | ||
| SPDLOG_API bool strings_equal_ci(string_view_t s1, string_view_t s2); |
There was a problem hiding this comment.
Why SPDLOG_API ? i would expect SPDLOG_INLINE
There was a problem hiding this comment.
I can change that, no problem.
But if I didn't mix things up, there is a second merge request for the same issue:
#3535
If you prefer the other one, I wouldn't put more work into my merge request.
There was a problem hiding this comment.
Not sure yet. The other merge seems bit leaner indeed
3 participants
This patch changes the behaviour of from_str() so that the level names are compared case-insensitive.
This has the side effect of making load_levels() work if the names were set to UPPER oder CamelCase in tweakme.h.
Beside that, now also the special cases for "off", "err" and "warn" now check that the names are actually "off", "error" and "warning" and have not been set to something else.
It would be strange behaviour if I would set the name of the error level e.g. to "Fehler" and spdlog would recognize "fehler" and also "err", but not "error" as that level.
The test also checks a mixed case string now, and also "err".