Re: [PATCH] emoji on TUI emacs
Eli Zaretskii <[email protected]> Sat, 01 Aug 2026 13:37:48 +0300
| Newsgroups | gmane.emacs.devel |
|---|---|
| Message-ID | <[email protected]> |
> Date: Fri, 31 Jul 2026 16:29:34 +0200 > Cc: [email protected] > From: Kai Ma <[email protected]> > > >> The proposed patch does not fix this problem, and I think this is > >> orthogonal to the width calculation. That's why I mentioned the case of > >> correct widths. > > The "correct widths" case is a small minority. > > Suppose we fix the width problem in the end, we still need to add the > padding glyphs, don't we? Yes. > FWIW, I also ran a quick analysis on an emoji catalog I'm maintaining: > > - Total number of unique emojis: ~11704 (many are variations or > combinations) > > - string-width=1: ~824 What does CHARACTER_WIDTH produce for the first codepoint of this group of Emoji? does it produce 1 or a larger value? > - string-width=2: ~2886 > > - string-width=2 and has fe0f: 553 And here, does the first codepoint always produce 2 as CHARACTER_WIDTH? IOW, it would be good to know how many width mistakes are there, after applying the patch you propose, at least for Emoji. > Thanks for the pointer! I attached a patch using this approach. PTAL. Thanks, a few comments to the code below. > One detail: I assume we want to handle static composition in the same > way. Deducing from handle_composition_prop in xdisp.c, I'm using it->c > for the static composition branch. Yes, that's correct, IMO. > --- a/src/term.c > +++ b/src/term.c > @@ -567,6 +567,13 @@ encode_terminal_code (struct glyph *src, int src_len, > nchars = 0; > while (src < src_end) > { > + /* We must skip glyphs to be padded for a wide character. */ > + if (CHAR_GLYPH_PADDING_P (*src)) > + { > + src++; > + continue; > + } > + > if (src->type == COMPOSITE_GLYPH) > { > struct composition *cmp; > @@ -632,8 +639,7 @@ encode_terminal_code (struct glyph *src, int src_len, > nchars++; > } > } > - /* We must skip glyphs to be padded for a wide character. */ > - else if (! CHAR_GLYPH_PADDING_P (*src)) > + else > { Is this part really needed? AFAICT, it doesn't change anything, because the loop already unconditionally increments 'src', for the padding and non-padding cases alike. Or what am I missing? > + for (i = 0; > + i < width && glyph < end; > + ++i) > { > - glyph->u.cmp.automatic = 1; > + glyph->type = COMPOSITE_GLYPH; > + glyph->pixel_width = it->pixel_width; ^^^^^^^^^^^^^^^ I think this should be "it->pixel_width - padding", so that the summary width of all the glyphs produced for the composition remains the same. > static void > produce_composite_glyph (struct it *it) > { > + bool padding; I'd prefer 'padding' to be an int, not bool. First, it could be more than 1 in the future, and second, we are adding it here: > + padding = (it->pixel_width >= 2) && (CHARACTER_WIDTH (first_char) >= 2); > + it->nglyphs = 1 + padding; While it is okay in C to perform arithmetics on bool values, I'd prefer not to do so. Finally, there's one problem I see after applying the patch: displaying sequences like "\x2699\xfe0f" produce incorrect cursor movement, because Emacs thinks the resulting Emoji is 1-column wide, whereas what the terminal here shows is a 2-column Emoji. So maybe we need a special treatment for Emoji produced by appending #xFE0F to a 1-column character: consider that to produce a 2-column Emoji. WDYT?