Skip to content

ukify: don't attempt to print emoji#35785

Closed
dimich-dmb wants to merge 1 commit intosystemd:mainfrom
dimich-dmb:main
Closed

ukify: don't attempt to print emoji#35785
dimich-dmb wants to merge 1 commit intosystemd:mainfrom
dimich-dmb:main

Conversation

@dimich-dmb
Copy link
Contributor

Fixes #35784

@github-actions github-actions bot added uki please-review PR is ready for (re-)review by a maintainer labels Dec 30, 2024
@bluca
Copy link
Member

bluca commented Dec 30, 2024

Why remove this? Sounds like a utf issue in your terminal?

@bluca bluca added needs-reporter-feedback ❓ There's an unanswered question, the reporter needs to answer and removed please-review PR is ready for (re-)review by a maintainer labels Dec 30, 2024
@dimich-dmb
Copy link
Contributor Author

dimich-dmb commented Dec 30, 2024

Why remove this? Sounds like a utf issue in your terminal?

In my terminal I can see all UTF characters I need: Cyrillic, Greek, math, pseudographic, etc because I have installed corresponding fonts. To see emoji I need to find and install more fonts. I won't.
Moreover, Linux virtual console, for example, supports no more than 512 glyphs and they are limited in size.
Moreover, not all terminals are unicode-capable.

Unprintable characters in system utility output are confusing: it's hard to understand whether it is an important warning failed to print or just someone's joke.

@YHNdnzj
Copy link
Member

YHNdnzj commented Dec 30, 2024

Sorry, but we generally assume emojis just work if not on Linux VT and locale is in UTF-8 (see emoji_enabled()), hence dropping this wholly is not desirable. However, I think it's acceptable to teach ukify to honor $SYSTEMD_EMOJI like our C tools.

@YHNdnzj YHNdnzj added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed needs-reporter-feedback ❓ There's an unanswered question, the reporter needs to answer labels Dec 30, 2024
@dimich-dmb
Copy link
Contributor Author

dimich-dmb commented Dec 30, 2024

Sorry, but we generally assume emojis just work if not on Linux VT and locale is in UTF-8 (see emoji_enabled())

Sorry, but this assumption is wrong. Charmap "UTF-8" in locale doesn't mean that terminal is able to display any arbitrary unicode character.

However, I think it's acceptable to teach ukify to honor $SYSTEMD_EMOJI like our C tools.

I don't care how the issue will be fixed. I just proposed the simplest, and in my opinion, the most optimal solution.

@dimich-dmb
Copy link
Contributor Author

dimich-dmb commented Dec 31, 2024

Ah, I looked into C code and got your point now. I couldn't even imagine it has special code to display emojis 🤦 Fortunately I've never seen them in output of other systemd utilites. Of course, in this case removing garbage from warning message doesn't fit into overall style of this software. The PR can be closed.
However, the issue #35784 is still relevant: ukify displays .

@dimich-dmb dimich-dmb closed this Dec 31, 2024
@github-actions github-actions bot removed the reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks label Dec 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

ukify should check whether emojis are available akin to emoji_enabled()

3 participants