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
Support for std::string_view in sf::String? #2445
Comments
P.S. - I'm aware that I can just enclose the sv in a std::string when passing it. I just wanted to propose the feature. |
SFML 3 (in development in I'm open to this idea. Can we replace the |
I'd need to set up cmake and all that for it, which I haven't done in a few years, but I have tomorrow off. I'll take a whack at it. It's possible to substitute string_view for string, since sf::String makes an internal copy anyways. std::string_view has a conversion from std::string. Should the PR be for the 2.6.x branch? |
No, as |
Where should a PR for SFML3 be submitted?
…On Tue, Mar 14, 2023, 2:09 AM Lukas Dürrenberger ***@***.***> wrote:
Should the PR be for the 2.6.x branch?
No, as std::string_view is a C++17 feature and SFML 2.6 remains on C++03
—
Reply to this email directly, view it on GitHub
<#2445 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACXDSQBYBVN7SUPAMYQGQTDW4AYWJANCNFSM6AAAAAAVZ4464U>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Target the |
@ChrisThrasher, Sorry for missing that when you said it in your initial post. I wasn't really on top of my game last night. I was able to implement this, but in testing I found that functions expecting a const sf::String& weren't able to carry out the two stares of conversion required to go from std::string to std::string_view to sf::String. I was able to resolve this by adding delegating constructors with the original signature, but those are the extra overloads that Chris was concerned about. I don't think there's a simpler way to get around the issue. I've posted what I've got for now and I'll wait to hear back. |
Noticed today when trying to load an sf::Text that when a function requires an sf::String argument, std::string_view will not work, since there's no conversion. I'm not sure what the status is in terms of what C++ version most users are on, but it would be handy to have an overload for sf::String to accept std::string_view as an initialization argument. I reckon it could be segregated into a preprocessor block that checks for support before including it.
This would imply overloads for
...and the inclusion of <string_view> in the header.
Thinking about it for a minute, the only potential issue that jumps out at me is that it would require some code repetition. This could potentially be avoided by having the acting overload accept std::string_view (or variant) as the relevant argument, then having the overloads simply forward their arguments into that function, since std::string_view will convert from the relevant types. If compatibility is a concern, then the preprocessor checks could just be in the header file, blocking the inclusion of <string_view> and the declaration of the overloads that use it in that scope, effectively making it an internal implementation detail, although that could potentially be an issue for users who compile the library themselves.
I'm not sure what the posture of the project is towards this kind of thing, but I figured I'd make the suggestion.
Cheers.
The text was updated successfully, but these errors were encountered: