Support for std::string_view in sf::String?

[khatharr]

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

• String::String(const std::string& ansiString, const std::locale& locale)
• String::String(const wchar_t* wideString)
• String::String(const std::basic_string& utf32String)

...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.

[khatharr]

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.

[ChrisThrasher]

I'm not sure what the status is in terms of what C++ version most users are on

SFML 3 (in development in master) uses C++17 so this is available to us and we've already started using std::string_view in a few places, mostly internally.

I'm open to this idea. Can we replace the std::string parameters with string_view parameters to avoid having to add many more overloads?

[khatharr]

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?

[eXpl0it3r]

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

[khatharr]

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: ***@***.***>

[ChrisThrasher]

Target the master branch

[khatharr]

@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.