-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
Implement support for string_view (attempt no. 3) #3423
Conversation
096c983
to
02b12d8
Compare
524697b
to
98bcc55
Compare
Downgrading back to draft status. I made substantial changes and want to see if they pass CI. Will update the doc next and re-request review when it's ready. |
1632077
to
fb8a93f
Compare
@nlohmann I'm trying something new and wanted to get your feedback before I continue. I've added the Example:
translates to:
Essentially, if we want to tweak the wording we can do so in one place now. Shall I continue or do you want me to roll that back? |
I'd rather have it explicit, because this makes it easier for others to edit the Markdown files. I am aware of the DRY violation etc., but I'm afraid this will add too much complexity. |
cac3cc1
to
ba13de3
Compare
Would be very nice if this could happen! |
59d8fd5
to
d659ed9
Compare
I've discovered some last minute issues yesterday that were bad enough to make me question the overall soundness of this PR. After some careful tweaks I am now cautiously optimistic that everything has been resolved (pending green CI and 100% coverage). I plan on reviewing everything once again side-by-side with the documentation to make sure they match up. Edit: CI is green and coverage is back to 100%. Looks promising. :-) |
nlohmann::ordered_map uses a different comparison function than the one provided via template parameter. * Introduce a type trait to detect if object_t has a key_compare member. * Rename object_comparator_t to default_object_comparator_t. * Add object_comparator_t to be conditionally defined as object_t::key_compare, if available, or default_object_comparator_t otherwise. * Update the documentation accordingly. Co-authored-by: Niels Lohmann <niels.lohmann@gmail.com>
Add type trait to check: * if a type is a specialization of a template. * if a type is a json_pointer. * if a type is a basic_json::{const_,}iterator. * if two types are comparable using a given comparison functor. * if a type is comparable to basic_json::object_t::key_type. * if a type has a member type is_transparent. * if a type is usable as object key. * if a type has an erase() function accepting a given KeyType. Co-authored-by: Niels Lohmann <niels.lohmann@gmail.com>
99ceaef
to
4e60bce
Compare
Rework basic_json element access member functions and operators to accept any type that meets the requirements defined by type trait detail::is_usable_as_key_type. Member functions and operators: * at() * operator[] * value() * erase() * find() * count() * contains() Update documentation to reflect these changes. Add unit tests to excercise the new functions using std::string_view. Co-authored-by: Niels Lohmann <niels.lohmann@gmail.com>
4e60bce
to
078673f
Compare
I've gone through the documentation and the code side-by-side to make sure that all documented functions are in fact there and correct. I've also added a comment to a particularly unrecognizable overload: // this is the value(const typename object_t::key_type&) overload
template < class KeyType, class ValueType, detail::enable_if_t <
std::is_same<KeyType, typename object_t::key_type>::value
&& detail::is_getable<basic_json_t, ValueType>::value
&& !std::is_same<value_t, ValueType>::value, int > = 0 >
typename std::decay<ValueType>::type value(const KeyType& key, ValueType && default_value) const
{ I believe this thing is now done and ready for final reviews. My suggestion would be to merge #3336 first, though. I'll have an easier time resolving merge conflicts, if there are any. |
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.
Looks good to me.
Thanks a lot! This is really a milestone!!! |
Based heavily on the previous 2 attempts of @nlohmann with some cleanups of mine.
Closes #1529.
Closes #2685.
Closes #3237.
Note: Depends on #3415 and needs to be rebased.Done.Add
key_compare
member toordered_map
Replace
==
withkey_compare
inordered_map
Expose the actual comparison function used by
object_t
nlohmann::ordered_map
uses a different comparison function than the one provided via template parameter.object_t
has akey_compare
member.object_comparator_t
todefault_object_comparator_t
.object_comparator_t
to be conditionally defined asobject_t::key_compare
, if available, ordefault_object_comparator_t
otherwise.Add type traits to check if a type is usable as object key
Add type trait to check:
json_pointer
.basic_json::{const_,}iterator
.basic_json::object_t::key_type
.Based on previous work by @nlohmann.
Rework
basic_json
element access to accept more key typesRework
basic_json
element access member functions and operators to accept any type that meets the requirements defined by type traitdetail::is_usable_as_key_type
.Member functions and operators:
at()
operator[]
value()
erase()
find()
count()
contains()
Update documentation to reflect these changes.
Add unit tests to excercise the new functions using
std::string_view
.Based on previous work by @nlohmann.