-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Fix range suppresor formatter #4660
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?
Fix range suppresor formatter #4660
Conversation
db94827 to
20e213d
Compare
… of adaptor formatting
4a3a37e to
d9c2339
Compare
d9c2339 to
d149787
Compare
|
@vitaut Tried to fix the problem, check once and please let me know if I need to update anything |
|
Ping! |
vitaut
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.
Thanks for the PR! I don't think we should introduce another trait for this. Have you tried applying the suggestion from #4123 (comment)?
I considered the approach from #4123, but it would disable formatting for standard adaptors like std::stack, std::queue, and std::priority_queue. I introduced the Let me know if I missed something or if you'd like me to proceed differently! |
Fixes #4123.
Problem
The
formatterspecialization for container adaptors (likestd::stack,std::queue,std::priority_queue) was overly aggressive. It matched any type with acontainer_typemember alias, causing ambiguous partial specializations when someone tried to define a custom formatter for their own types that happened to have acontainer_type.Solution
added a
fmt::is_container_adaptor<T>trait. This defaults tostd::true_typebut allows users to specialize it tostd::false_typefor their custom types. This removes the library's adaptor formatter from the overload set via SFINAE, resolving the ambiguity and allowing custom formatters to work.Verification
Added a regression test in
test/ranges-test.cc(container_adaptor_partial_specialization) to test the fix and allthanks .. please let me know if anything need to be changed