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

feat: add example for truncate #36

Merged
merged 2 commits into from
Nov 21, 2023
Merged

feat: add example for truncate #36

merged 2 commits into from
Nov 21, 2023

Conversation

ctron
Copy link
Member

@ctron ctron commented Nov 17, 2023

I also upticked the dependency version and the rev in the patch section.

@trust-git-bot
Copy link

trust-git-bot commented Nov 17, 2023

🚀 Deployed Preview: http://patternfly-yew-quickstarts-pr-36-preview.surge.sh

@aDogCalledSpot
Copy link
Contributor

Looks good to me, it might be worth adding a comment in the docs saying that the truncate doesn't necessarily truncate num grapheme clusters but rather rounds down to the next unicode codepoint after num bytes.

I'm not sure if adding any extra complexity to the code is worth it though. At best you could maybe trim off num unicode codepoints by using s.char_indices().rev().nth(num) which will still look weird in a lot of cases and anything better than that will require an extra dependency. So a comment would probably be good, just so people aren't surprised that there are only 5 characters visible when they wanted to keep 10.

@ctron ctron merged commit eaff417 into main Nov 21, 2023
2 checks passed
@ctron ctron deleted the feature/add_truncate_1 branch November 21, 2023 09:01
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.

3 participants