Skip to content

fix: calculate display width with unicode-width to handle emoji#539

Open
cedricpinson wants to merge 1 commit into
altsem:masterfrom
cedricpinson:fix/string-width-emoji
Open

fix: calculate display width with unicode-width to handle emoji#539
cedricpinson wants to merge 1 commit into
altsem:masterfrom
cedricpinson:fix/string-width-emoji

Conversation

@cedricpinson

Copy link
Copy Markdown

Problem

String display width was computed with grapheme cluster count (graphemes(true).count()). A single emoji such as 🚀 is one grapheme cluster but occupies two terminal columns. As a result the layout underestimated the width by one column per emoji, shifting and overlapping the spans that follow on the same line.

Fix

Use the actual terminal display width from the unicode-width crate wherever a string's rendered width is needed:

  • layout_span in src/ui.rs (the central function used for rendering)
  • the text() helper in src/ui/layout/mod.rs
  • span_width (overflow detection) in src/screen/mod.rs, and the ellipsis truncation, which now accumulates display width instead of grapheme count so a wide character is never split and the column is correctly reserved

unicode-width was already a dev-dependency (used in test helpers); it is moved to [dependencies] since it is now needed at runtime.

Tests

  • New emoji_display_width test: a single emoji measures 2 columns and the following span starts at column 2.
  • Full suite passes (395 tests), clippy clean.

String width was computed with grapheme cluster count, but a single
emoji is one grapheme yet occupies two terminal columns. This made the
layout underestimate width by one column per emoji, shifting/overlapping
following spans. Use unicode-width's display width in layout_span, the
text() helper and the overflow/truncation logic, and make truncation
accumulate display width instead of grapheme count.
@cedricpinson

Copy link
Copy Markdown
Author

Hi,
I was using the tool but always had some lines cut because of how the length of lines was computed. This PR is done by me, but the code comes from Claude. It's not a lot of code, but it’s worth mentioning the source of the fix.

Thanks

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.

1 participant