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 an ANSI-aware string truncation function #22

Merged
merged 9 commits into from
Oct 31, 2020
Merged

Implement an ANSI-aware string truncation function #22

merged 9 commits into from
Oct 31, 2020

Conversation

meowgorithm
Copy link
Contributor

Tests are also included. Also, I haven't implemented this on Buffer yet.

I abstracted out the code for detecting ANSI start and end sequences for sake of reuse, which impacts performance very slightly (thanks @kiyonlin for the benchmarking tests). We could instead manually inline that function if we wanted for a slight performance boost.

This closes #21.

@meowgorithm
Copy link
Contributor Author

Just noticed a small correction needed on one of my comments; I’ll fix that up in the morning.

@meowgorithm
Copy link
Contributor Author

Alright, corrected. Let me know you if you have any thoughts or questions.

@meowgorithm
Copy link
Contributor Author

Closing as I'm noticing some odd behavior with this. I'll reopen should I sort the issue out.

@meowgorithm meowgorithm reopened this Oct 22, 2020
@meowgorithm
Copy link
Contributor Author

Fixed the odd behavior by adding a reset sequence after performing a truncation. Working great on my end now.

ansi/buffer.go Outdated Show resolved Hide resolved
ansi/buffer.go Outdated Show resolved Hide resolved
@muesli
Copy link
Owner

muesli commented Oct 23, 2020

We should probably also move this into a separate package and provide a writer, as we do for the other operations. I can try to do it this weekend, unless one of you feels up for the task. I hear @kiyonlin has a bit of a routine going at this point 😉

@kiyonlin
Copy link
Contributor

We should probably also move this into a separate package and provide a writer, as we do for the other operations. I can try to do it this weekend, unless one of you feels up for the task. I hear @kiyonlin has a bit of a routine going at this point 😉

@muesli No problem, I can move them into a separate package after this PR is done.

But the truncate package will have two extra functions StringWithTail and BytesWithTail, right?

ansi/buffer.go Outdated Show resolved Hide resolved
@muesli
Copy link
Owner

muesli commented Oct 23, 2020

But the truncate package will have two extra functions StringWithTail and BytesWithTail, right?

Correct, that sounds reasonable. Thank you @kiyonlin!

@muesli muesli added the enhancement New feature or request label Oct 23, 2020
@meowgorithm
Copy link
Contributor Author

There are still some test cases I'd like to write, but I've otherwise reworked the truncation function using Truncate in mattn/runewidth as a model.

ansi/buffer_test.go Outdated Show resolved Hide resolved
@muesli muesli merged commit 738d000 into muesli:master Oct 31, 2020
@muesli
Copy link
Owner

muesli commented Oct 31, 2020

Thank you @meowgorithm, @kiyonlin!

Merged and ready to be extracted into a separate package!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Truncate support
3 participants