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

Implement sf::String in terms of std::u32string #2480

Merged
merged 2 commits into from Apr 4, 2023

Conversation

ChrisThrasher
Copy link
Member

@ChrisThrasher ChrisThrasher commented Mar 29, 2023

Description

Closes #2470

This will make it easier to do things like use UTF-32 string literals since sf::String will have a constructor that perfectly matches that type.

@ChrisThrasher ChrisThrasher added this to the 3.0 milestone Mar 29, 2023
@codecov
Copy link

codecov bot commented Mar 29, 2023

Codecov Report

Merging #2480 (e5fcb91) into master (7004db1) will decrease coverage by 1.81%.
The diff coverage is 64.28%.

❗ Current head e5fcb91 differs from pull request most recent head 66c70af. Consider uploading reports for the commit 66c70af to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2480      +/-   ##
==========================================
- Coverage   25.84%   24.03%   -1.81%     
==========================================
  Files         226      226              
  Lines       19419    19405      -14     
  Branches     4714     4714              
==========================================
- Hits         5018     4664     -354     
- Misses      13868    14259     +391     
+ Partials      533      482      -51     
Impacted Files Coverage Δ
include/SFML/System/String.hpp 100.00% <ø> (ø)
src/SFML/Network/Packet.cpp 32.93% <0.00%> (ø)
src/SFML/Window/OSX/HIDInputManager.mm 1.08% <0.00%> (ø)
src/SFML/Window/Unix/KeySymToUnicodeMapping.cpp 0.00% <0.00%> (ø)
src/SFML/Window/Unix/KeyboardImpl.cpp 0.00% <0.00%> (ø)
src/SFML/System/String.cpp 100.00% <100.00%> (+0.50%) ⬆️

... and 14 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7004db1...66c70af. Read the comment docs.

Copy link
Member

@Bromeon Bromeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Mostly good, 2 minor comments 🙂

src/SFML/Network/Packet.cpp Show resolved Hide resolved
src/SFML/System/String.cpp Show resolved Hide resolved
test/System/String.test.cpp Show resolved Hide resolved
test/System/String.test.cpp Outdated Show resolved Hide resolved
@vittorioromeo
Copy link
Member

Generally looks good to me, I'm in favour of this PR.

@ChrisThrasher ChrisThrasher merged commit f371a99 into SFML:master Apr 4, 2023
78 checks passed
@ChrisThrasher ChrisThrasher deleted the modernize_sf_string branch April 4, 2023 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Use std::char32_t for UTF-32 character type in sf::String
3 participants