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?