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

Initial support for std::string_view #633

Merged
merged 2 commits into from
Oct 22, 2024

Conversation

dantargz
Copy link
Contributor

@dantargz dantargz commented Oct 22, 2024

Based on work from @brandl-muc

Initial PR for supporting std::string_view as laid out in #626

  • Add a define guard to make initial availability opt-in
  • Add includes, typedefs, and support for WCHAR mode
  • Add overloads for set_name/set_value/set functions, which already have support for const char* + size
  • Add changes to build files & CI configurations
  • Adapt existing unit tests for set_name/set_value/set overloads that take const char* + size to also exercise string_view path, including exercising cases where the data pointer in the string_view is nullptr

For CI config, the new define was added to the matrix testing configuration. The new code is only exercised when using c++17 std lib or later, so an additional build & run was added to all platforms. After the code settles, the expectation is for PUGIXML_STRING_VIEW to be retired, which will help reduce CI run bloat.

Appveyor configs are not updated as part of this change. The appveyor runs are determined to be slow, the majority of new runs created by adding this to the matrix would be redundant due to most of the matrix not having c++17, and we will gain coverage automatically when the opt-in define is retired.

This change causes a new internal define PUGIXML_HAS_STRING_VIEW to be conditionally defined by the header, and it is never undefined.

src/pugixml.hpp Outdated Show resolved Hide resolved
@zeux zeux merged commit a0db6e2 into zeux:master Oct 22, 2024
27 checks passed
@zeux
Copy link
Owner

zeux commented Oct 22, 2024

Thanks!

@dantargz dantargz deleted the issue-626-add-stringview-initial branch October 25, 2024 17:18
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.

2 participants