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

Improved string_view support #102

Merged
merged 1 commit into from
Feb 20, 2021
Merged

Improved string_view support #102

merged 1 commit into from
Feb 20, 2021

Conversation

phprus
Copy link
Contributor

@phprus phprus commented Feb 16, 2021

  1. Removed use of the __cpp_lib_string_view macro, because the __cpp_lib_string_view macro requires <version>(C++20) or <string_view> (In the libc++: only since version 8.0: llvm/llvm-project@f56972e)
  2. Added support of legacy std::experimental::string_view (if the std::string_view not found).
  3. Added selector to use a custom, user provided string_view implementation (by defining the GHC_HAS_USER_STRING_VIEW macro and add using custom::basic_string_view into ghc::filesystem namespace).

Maybe the GHC_HAS_USER_STRING_VIEW macro should be renamed?

@gulrak
Copy link
Owner

gulrak commented Feb 17, 2021

Thank you for the effort, I really like the idea! Only the removal of __cpp_lib_string_view (that also is required to be delivered by <string> besides <string_view>) is something I rather not prefer, as this solution would force the compilers or their libs down to only the three explicitly handled ones, and my main concern is any conforming compiler should work without explicitly being named when possible (there are more out there than these three, like EDG, or Embarcadero C++). So I can see the need to additionally detect the corner cases (like other code in this project, detecting cases where MSVC does not deliver a usable __cplusplus) to enable more string_view chances, but not the removal of the standard conforming detection. It could be used to also set GHC_HAS_STD_STRING_VIEW to fit to the changes.

(I can not really think of a better name for GHC_HAS_USER_STRING_VIEW, besides maybe GHC_HAS_CUSTOM_STRING_VIEW, but both would be fine.)

As this (and the last) week are very busy job-wise, I wouldn't be able to merge this before weekend anyway.
Thanks again for your PR and support! 👍

@gulrak gulrak added the enhancement New feature or request label Feb 17, 2021
@phprus
Copy link
Contributor Author

phprus commented Feb 17, 2021

  1. Added checking of the __cpp_lib_string_view macro before checking the versions of libraries and compilers.
  2. Renamed GHC_HAS_USER_STRING_VIEW to GHC_HAS_CUSTOM_STRING_VIEW.

@gulrak gulrak added this to the v1.5.2 milestone Feb 17, 2021
@gulrak gulrak merged commit 0e5f2f5 into gulrak:master Feb 20, 2021
@gulrak gulrak added the available on master Fix is done on master branch, issue closed on next release label Feb 20, 2021
@phprus phprus deleted the string_view branch April 16, 2022 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
available on master Fix is done on master branch, issue closed on next release enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants