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

Fixed cell width issues when using ANSI color codes. #79

Merged
merged 2 commits into from
Aug 9, 2018

Conversation

liautaud
Copy link
Contributor

@liautaud liautaud commented May 18, 2018

This PR adds a utils::display_width function, which is just a wrapper around UnicodeWidthStr::width which also takes ANSI color codes into account.

This is required when creating cells from strings which are already colored using ANSI color codes (instead of coloring the cells using styles). Since color codes are of the form \u{1b}[ ... m, but UnicodeWidthStr::width only takes the first \u{1b} into account, this would create cell width issues.

This solves issue #46.

This commit adds a `utils::display_width` function, which is just a wrapper
around `UnicodeWidthStr::width` which also takes ANSI color codes into account.

This is required when creating cells from strings which are already colored
using ANSI color codes (instead of coloring the cells using styles). Since
color codes are of the form \u{1b}[ ... m, but UnicodeWidthStr::width only
takes the first \u{1b} into account, this would create cell width issues.
@phsym phsym self-requested a review August 9, 2018 15:15
@phsym phsym self-assigned this Aug 9, 2018
@@ -75,6 +75,36 @@ pub fn print_align<T: Write + ?Sized>(out: &mut T,
Ok(())
}

/// Return the display width of a unicode string.
/// This functions takes ANSI-escaped color codes into account.
pub fn display_width(text: &str) -> usize {
Copy link
Owner

Choose a reason for hiding this comment

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

Would be even better with some tests

@phsym
Copy link
Owner

phsym commented Aug 9, 2018

Thanks a lot for that. Again, sorry for the delay. I've been quite busy over the past months

@phsym phsym merged commit 8a40e2a into phsym:master Aug 9, 2018
@phsym phsym added this to the v0.8.0 milestone Sep 16, 2018
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.

2 participants