console-rs / console

A rust console and terminal abstraction
MIT License
953 stars 113 forks source link

Moved from unicode-width to unicode-display-width for visual grapheme width estimation #210

Closed TapGhoul closed 7 months ago

TapGhoul commented 7 months ago

This change had to be made in both crates in order to be updated correctly. The console-rs crate has been bumped an extra version as a result of a behavior change of char_width() with the unicode-display-width feature enabled - it now returns always 1 or 2, never 0 as it used to. The indicatif crate may also need to be bumped a full version as a result of the dependency change, but I'm not sure if you'd consider this a breaking change to the same degree, as it's not an advertised feature.

Related issue: https://github.com/console-rs/indicatif/issues/638 Related PR: https://github.com/console-rs/indicatif/pull/639

TapGhoul commented 7 months ago

Pulling in @djc as requested

TapGhoul commented 7 months ago

Worth an extra note: I'm not sure if truncate_str() needs to be adjusted - it'll depend on how ANSI is handled here, and how much we need to care about grapheme clusters. I'm not 100% sure if we need to change if unicode_display_width::is_double_width(c) { into if c == '\u{FEFF}' || unicode_display_width::is_double_width(c) { or not, as a result, as per how unicode_display_width::get_grapheme_width() operates. Unfortunately, I don't have a test case on hand to check this - do you have one that can be used to validate things?