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

Improve maximum width of characters in a string #257

Merged
merged 1 commit into from
Feb 13, 2024

Conversation

mikelorant
Copy link
Contributor

The function maxRuneWidth was using a flawed approach by only evaluating strings based on each rune. The uniseg package provides a more accurate method based on grapheme clusters.

This change switches over to the improved implementation which should achieve better results when determining the maximum width of all grapheme clusters in a string.

@mikelorant mikelorant force-pushed the fix/maximum-width branch 3 times, most recently from 9137b47 to 261e1bd Compare January 31, 2024 09:09
@mikelorant
Copy link
Contributor Author

Had to bump up to Go 1.18 as a requirement of uniseg.

@charmbracelet Can I get a review of this improvement. Thanks.

go.mod Outdated Show resolved Hide resolved
The function `maxRuneWidth` was using a flawed approach by only
evaluating strings based on each rune. The `uniseg` package provides a
more accurate method based on grapheme clusters.

This change switches over to the improved implementation which should
achieve better results when determining the maximum width of all
grapheme clusters in a string.

Signed-off-by: Michael Lorant <michael.lorant@nine.com.au>
@mikelorant
Copy link
Contributor Author

@maaslalani Changes made as requested.

@mikelorant
Copy link
Contributor Author

@maaslalani You are now my PR buddy as I move onto the next one 😆

@mikelorant
Copy link
Contributor Author

@maaslalani Any further changes required or can we merge this now?

@meowgorithm
Copy link
Member

Nice work, sir. Let’s get this in.

@meowgorithm meowgorithm merged commit 2745d8a into charmbracelet:master Feb 13, 2024
9 checks passed
@mikelorant mikelorant deleted the fix/maximum-width branch February 13, 2024 20:09
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