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

Add remaining std::string_view overloads #634

Closed

Conversation

dantargz
Copy link
Contributor

Completing the std::string_view support work defined in #626

The key new functionality is adding a helper function for checking equality between string_view and null terminated c-strings impl::stringview_equal. Using strncmp/wsncmp directly would not work, because that stops comparison at the first null, and the string_view might have a null in the middle -- we do not want a string_view of "n\0pe" to be considered equal to a node name "n" for lookups. string_view has a comparison function that has an overload for const char* (null terminated c-string) that would give the comparison we want. However, that helper works by constructing a string_view from the const char* and then comparing to the new string_view, which would trigger an extra iteration over the const char*.

These overloads are created by copying and pasting the original and substituting the new equality function:

xml_node xml_node::child(string_view_t name) const;
xml_attribute xml_node::attribute(string_view_t name) const;
xml_node xml_node::next_sibling(string_view_t name) const;
xml_node xml_node::previous_sibling(string_view_t name) const;
xml_attribute xml_node::attribute(string_view_t name, xml_attribute& hint) const;

Added some unit tests for child(string_view_t) and attribute(string_view_t). Also added unit tests documenting the behavior of child and attribute lookup when the node/attribute has null name -- searching does not find these nodes, even if an empty argument name is provided.

These other functions also gained overloads, but they just pass the string_view_t to another overloaded function. No new unit tests were added for these functions.

xml_attribute& xml_attribute::operator=(string_view_t rhs);

xml_attribute xml_node::append_attribute(string_view_t name);
xml_attribute xml_node::prepend_attribute(string_view_t name);
xml_attribute xml_node::insert_attribute_after(string_view_t name, const xml_attribute& attr);
xml_attribute xml_node::insert_attribute_before(string_view_t name, const xml_attribute& attr);
xml_node xml_node::append_child(string_view_t name);
xml_node xml_node::prepend_child(string_view_t name);
xml_node xml_node::insert_child_after(string_view_t name, const xml_node& node);
xml_node xml_node::insert_child_before(string_view_t name, const xml_node& node);
bool xml_node::remove_attribute(string_view_t name);
bool xml_node::remove_child(string_view_t name);

xml_text& xml_text::operator=(string_view_t rhs);

Documentation will be updated to cover the new overloads after the opt-in define PUGIXML_STRING_VIEW is removed

@dantargz
Copy link
Contributor Author

I will re-open after I get a better way to test locally

@dantargz dantargz closed this Oct 23, 2024
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.

1 participant