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

Use char32_t instead of uint32_t for UniChar #7423

Merged
merged 1 commit into from
Feb 25, 2024

Conversation

biodranik
Copy link
Member

That allows to use std::u32string_view in the code, adding useful std::basic_string_view methods

@biodranik biodranik requested a review from vng February 18, 2024 13:00
@biodranik
Copy link
Member Author

@zyphlar PTAL, this can make your code simpler.

indexer/indexer_tests/trie_test.cpp Outdated Show resolved Hide resolved
indexer/indexer_tests/trie_test.cpp Outdated Show resolved Hide resolved
indexer/trie.hpp Outdated Show resolved Hide resolved
base/string_utils.hpp Outdated Show resolved Hide resolved
@biodranik biodranik force-pushed the ab-unichar-to-char32_t branch 2 times, most recently from b2c36df to cac8dee Compare February 25, 2024 15:52
std::istringstream stream((std::string(*semicolon)));
strings::UniChar uc;
const std::string iString{*semicolon};
std::istringstream stream(iString);
Copy link
Member

Choose a reason for hiding this comment

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

nit: What was wrong with previous std::istringstream stream((std::string(*semicolon))) ?

@vng
Copy link
Member

vng commented Feb 25, 2024

glyph_manager.cpp fix should be simple, just read uint32_t

drape/glyph_manager.cpp Outdated Show resolved Hide resolved
That allows to use u32string_view in the code, adding useful std::basic_string_view methods

Signed-off-by: Alexander Borsuk <me@alex.bio>
@biodranik biodranik merged commit f05be9b into master Feb 25, 2024
14 of 15 checks passed
@biodranik biodranik deleted the ab-unichar-to-char32_t branch February 25, 2024 22: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.

None yet

2 participants