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

Fix usage of ranges and to_hex in the same compile unit #2195

Merged
merged 2 commits into from
Dec 1, 2021
Merged

Fix usage of ranges and to_hex in the same compile unit #2195

merged 2 commits into from
Dec 1, 2021

Conversation

patrickroocks
Copy link
Contributor

Hello!

First of all, thanks a lot for this great package!

I encountered a compile problem when using ranges and bin_to_hex in the same compile unit. After doing some research I came to the conclusion that probably the only way so solve this is to hide the iterators used in bin_to_hex from the formatter in ranges.

My change in detail:

When trying to use spdlog/fmt/bin_to_hex.h in the same compile unit as spdlog/fmt/bundled/ranges.h you got a compile error because there was a multiple definitions for iterable classes. This fix renames the begin() and end() getters in dump_info into getBegin()/getEnd() in order to avoid this collision.

Added an example of ranges in example.cpp to show that it actually works (a to_hex example was already there)

What do you think?

All the best
Patrick

When trying to use spdlog/fmt/bin_to_hex.h in the same compile unit as spdlog/fmt/bundled/ranges.h you got a compile error because there was a multiple definitions for iterable classes. This fix renames the begin() and end() getters in dump_info into getBegin()/getEnd() in order to avoid this collision.

Added an example of ranges in example.cpp to show that it actually works (an to_hex example was already there)
@gabime
Copy link
Owner

gabime commented Dec 1, 2021

Thanks. Please use snake_case names (get_begin(), get_end())

@patrickroocks
Copy link
Contributor Author

patrickroocks commented Dec 1, 2021

fixed code style

@gabime gabime merged commit 0c611af into gabime:v1.x Dec 1, 2021
@gabime
Copy link
Owner

gabime commented Dec 1, 2021

Thanks @patrickroocks

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.

None yet

2 participants