charmbracelet / lipgloss

Style definitions for nice terminal layouts 👄
MIT License
8.06k stars 229 forks source link

Width issue with certain emoji #55

Closed davidcelis closed 5 months ago

davidcelis commented 2 years ago

While checking out the ssh git.charm.ssh demo, I noticed that the window showing the "Home" tab ends up rendering an incorrect border when certain emoji are used in a line:

image-2

Hard to know exactly which emoji would cause this, but these two being used in the demo definitely are!

meowgorithm commented 2 years ago

Thank you!

I believe this is because widths of newer emojis with genders and skin tones (in this case 💅🏻 and 💇🏻‍♀️) are being misread and/or are likely not in the tables in go-runewidth and rivo/uniseg upstream. We should be able to put in exceptions for these until they're fixed upstream, though we should also bear in mind that many systems will only have partial support for those emojis for awhile, too.

makew0rld commented 2 years ago

I experienced this issue with the not-so-new emoji 🎙️ U+1F399 U+FE0F. Removing the U+FE0F fixed the rendering issue for me. This makes the emoji unqualified, but doesn't mess up color rendering (at least on my system) because there is no standard line-art fallback.

As an additional issue, on the terminals I tested, U+1F399 is has width of two, not one. But the box rendering doesn't break, so I'm not sure what's up there.

meowgorithm commented 2 years ago

So after working through this some more I believe the issue here is that these emojis are comprised of multiple runes (i.e. U+1F399 U+FE0F, above) but internally we're evaluating them on a rune-by-rune basis, which is throwing the counts off. Evaluating widths on the string level, whilst continuing to ignore ANSI, should solve the issue.

FIGBERT commented 2 years ago

I'm experiencing the same issue with ⭐️ U+2B50 U+FE0F. I'd love to contribute a fix – where should I start poking about?

meowgorithm commented 2 years ago

Hi! We're actually mid-way through what should be a solid fix for this. I'll keep this thread updated with progress.

lukecarbis commented 2 years ago

@meowgorithm Any update on this?

robinovitch61 commented 2 years ago

Also interested in this, as I'm experiencing the same bug in my project (not due to lipgloss) and wondering how others are approaching it :)

meowgorithm commented 2 years ago

Basically, some emojis are one single rune, and others consist of a sequence of runes. Some emojis are ten runes long! The error, as you might expect, occurs when getting the width of emojis that consist of more than one rune.

There are a few ways to solve for this, with the most thorough method probably being to parse the official emoji sequences to build a lookup table, and then scan for those emojis accordingly.

On the Lip Gloss side, we're still working on this one. No ETA yet as it's part of a slightly larger endeavor, but I would personally like to see this issue closed soon.

mikelorant commented 1 year ago

I believe I may have a solution (or a partial solution) to this problem without requiring significant changes to Lipgloss. This issue relates to subtle differences between printable rune width and string width.

Pull request #163 has been created.

Test case and examples: https://github.com/mikelorant/emoji-alignment

meowgorithm commented 1 year ago

Just to shed some light on the subject, as mentioned earlier, the issue has to do with go-runewidth mis-reporting the width of emoji sequences longer than one rune.

For reference see the following documents:

https://www.unicode.org/Public/emoji/15.0/emoji-sequences.txt https://www.unicode.org/Public/emoji/15.0/emoji-zwj-sequences.txt

mikelorant commented 8 months ago

Just wanted to add that this issue has not been forgotten and the work has already commenced to replace go-runewidth with uniseg. It has been a lot more complex as it was not possible to search and replace the functions. Anything involving runes had to be refactored and flowed all the way back to lipgloss dependencies (such as reflow).

It is also clear that the majority of terminals are also doing the wrong thing as well. So the combination is making it a very slow and painful process.

Any code examples showing cases of failures would be appreciated so we can test this against the changes we already have. The signs are there that many alignment issues have been reduced and performance is way up especially for complex components like textarea.

Shout out to @meowgorithm and @maaslalani for putting up with all my refactor pull requests. There are so many changes required and their patience and advice has been appreciated.

IGLOU-EU commented 8 months ago

I confirm this issue is still present. I use emoji with my lipgloss with .Render(t.Icon+" "+t.Name))

screenshot-2024-02-09-02-21-09

Thanks :)

IoakeimKaltsidis commented 5 months ago

Any updates on this? @mikelorant

meowgorithm commented 5 months ago

This is fixed as of v0.11.0, with the big caveat that most terminals can't accurately render many emojis, and they often don't implement grapheme clustering which is necessary for width calculation.

For example, the following code…

package main

import (
    "fmt"

    "github.com/charmbracelet/lipgloss"
)

func main() {
    a := lipgloss.NewStyle().
        Border(lipgloss.RoundedBorder()).
        BorderForeground(lipgloss.Color("63")).
        Padding(0, 1).
        SetString("You look great 💅🏿")
    b := a.SetString("New year, new you 💇🏻‍♀️")
    c := a.SetString("So steamy 🧖🏽‍♂️")

    fmt.Println(
        lipgloss.JoinHorizontal(
            lipgloss.Top,
            a.String(),
            b.String(),
            c.String(),
        ),
    )
}

…renders differently on different terminals:

Ghostty (Private Beta):

Screenshot 2024-05-29 at 12 39 06 PM

Wezterm:

Screenshot 2024-05-29 at 12 54 43 PM

Kitty:

Screenshot 2024-05-29 at 12 40 34 PM

Alacritty:

Screenshot 2024-05-29 at 12 42 08 PM

I'm afraid there’s nothing we can do in these cases other than encouraging terminal authors to improving emoji rendering and supporting grapheme clustering.

For details see Mitchell Hashimoto’s writeup on the subject, particularly the notes about mode 2027.

mikelorant commented 4 months ago

This is also correct in the newly released iTerm version 3.5. iTerm2

At this time, consider Ghostty, iTerm2 and WezTerm as reference implementations of correct emoji output.

There are known issues with other terminals (including Kitty) that will cause inconsistencies.

I'd recommend running ucs-detect if you want to verify accuracy of your terminal.

Big shoutout to @aymanbagabas for his work on the new Charm ANSI/text package that has fixed this for the Charm suite of tools!

aronchick commented 2 months ago

Thank you so much for this - is there a list of emojis that work? Or for emojis that don't work, is there a set of things we can do (like set a width on a column) such that we won't have a problem? Nd how would we figure out that width?

That is to say, I'm happy to restrict the emojis, and/or pre calculate widths for those that I allow

meowgorithm commented 2 months ago

As illustrated above, it really varies from terminal to terminal, but I’d generally say safer bets are older emojis that have been around awhile, and emojis that are only one rune.

In terms of rune counts there are documents on Unicode.org (like this one) you can look at for reference.

For example 💔 is just one rune: U+1F494 whereas ❤️‍🔥 is four runes: U+2764 U+FE0F U+200D U+1F525.

Also keeping in mind that the OS doing the rendering needs to have up-to-date emojis as well so, for example, I’d expect a lot of Linux distros to be missing 🙂‍↔️ and 🐦‍🔥 at the moment.

aronchick commented 2 months ago

I really appreciate the help - and that link is super helpful.

I guess my question is since i can pre-choose the emojis that i want to work - if i choose an emoji that is (for example) 2 runs wide, and another that is 3 wide, can i put them in two columns side by side and have them line up by doing something manually? e.g. setting the width of something to 3? or setting the padding to zero on a certain column? or whatever.

meowgorithm commented 2 months ago

All emojis are two cells wide 😊

aronchick commented 2 months ago

i really appreciate the help :) what's driving me nuts right now is this alignment. I set the columns to be 4 wide, but the text is not aligned with the header :(

CleanShot 2024-08-16 at 12 51 36

meowgorithm commented 2 months ago

@aronchick wanna post some source code for that?

aronchick commented 2 months ago

Ok, you got it :) https://github.com/aronchick/bubble-tea-experiment

Download and run this:

with EMOJIS_ENABLED=true go run . - spacing off. with go run . (text only) - spacing correct!

(Running in iTerm 2 on the mac)