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: Ansi slice function #101

Merged
merged 1 commit into from
Jun 24, 2024
Merged

Conversation

sukus21
Copy link
Contributor

@sukus21 sukus21 commented Jun 22, 2024

Based on the truncate function.
Specify a start point (inclusive) and end point (exclusive) in cell-width units, and get a string back at that exact size.
Handles ansi-codes and multi-length characters.

@ccoVeille
Copy link
Contributor

I like the code, the quality of comments, the extensive tests. 🥰

ansi/slice.go Outdated Show resolved Hide resolved
Based on the truncate function.
Specify a start point (inclusive) and end point (exclusive) in cell-width units, and get a string back at that exact size.
Handles ansi-codes and multi-length characters.
Copy link
Contributor

@ccoVeille ccoVeille left a comment

Choose a reason for hiding this comment

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

I find the code clearer now, BTW.

Thanks for baring with my remarks

Copy link
Member

@aymanbagabas aymanbagabas left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @sukus21 for your contribution!

@aymanbagabas aymanbagabas merged commit bdd314f into charmbracelet:main Jun 24, 2024
5 checks passed
@aymanbagabas
Copy link
Member

aymanbagabas commented Jun 25, 2024

@sukus21, I've reverted this PR. I think we need to think through the implication of adding space paddings to the sliced string. I'm not sure if that's the right approach here. Take string slicing for example, something like s[1:3] will slice the string as if it was a byte slice and breaking utf8 codepoints.
Using something like golang.org/x/exp/utf8string Slice accounts for runes and returns the sliced string with respect to utf8 codepoints (runes) and the start and end arguments resemble utf8 rune indices.

With that being said, I'm not sure how we should handle slicing wide characters and I don't think padding with spaces is the right solution, especially if end > StringWidth(s).

Could you send a new draft PR so we can discuss this more?

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