bug#49797: 28.0.50; Setting face to custom fontset doesn't work

[Yuan Fu]

Again, thanks for spending your time on this.

> 
>> Problem 1: The following doesn’t work as intended:
>> 
>> (set-face-attribute 'variable-pitch nil :font "fontset-serif”)
>> 
>> where “fontset-serif” contains two fonts: Charter for Latin and Source Han 
>> Serif for CJK. 
>> 
>> The current effect is, Latin characters in ‘variable-pitch works (displayed 
>> in Charter), CJK characters don’t (not displayed in Source Han Serif). With 
>> my fix, CJK characters work (displayed in Source Han Serif).
>> 
>> Problem 2: ‘default face cannot use fontset. Even if problem 1 is fixed, 
>> setting the fontset for the default face still doesn’t display CJK 
>> characters correctly. With the fix, setting a fontset for ‘default works as 
>> intended.
>> 
>> The problems are not limited to CJK characters, any other non-ASCII 
>> character that requires a font different from the Latin characters has the 
>> same problem.
> 
> The thing is, what you describe as "problems" is how this stuff was
> designed to work, at least AFAIU the code.  Specifically, the face we
> define and see in Lisp specifies the font only for the ASCII
> characters.  When Emacs needs to display a non-ASCII character with
> some face (including the 'default' face), it calls face_for_char.
> That function will use the same face if its font supports the
> character, but if not, it will create a new "derivative" face.  That
> derivative face shares all the face attributes with the original one,
> but has a different font.  These derivative faces are never exposed to
> Lisp, and don't have names, so a few people even know they exist, but
> they are very visible on the C level: they have a distinct face ID.
> 
> You want to force Emacs to use the face's font for non-ASCII
> characters, but that's not how this stuff was designed, AFAIU.  If I'm
> not missing something, it means what you perceive as bugs are at best
> design bugs, not implementation bugs, and they cannot be fixed with a
> few lines of tweaking.  That might work in your relatively simple use
> case, but I don't see how we can trust that not to break important
> features, because working against the design always runs that risk.

The comment in face_for_char literally says:

         ...so
         that users could still setup fontsets to force Emacs to use
         specific fonts for characters from other scripts, because
         choice of fonts is frequently affected by cultural
         preferences and font features, not by font coverage…

Later in that function, we take the fontset of that face

        fontset = FONTSET_FROM_ID (face->fontset);

Then find an appropriate font from it

        rfont_def = fontset_font (fontset, c, face, id);

So Emacs definitely is designed to support using fontsets to assign different 
fonts for different characters to faces. In fact, you can try this right now 
(without my patch):

        (set-face-attribute 'variable-pitch :fontset "fontset-serif”)

(Notice I used the :fontset attribute, not :font attribute.) And the 
variable-pitch face will work as intended, use CJK font for CJK characters and 
Latin font for Latin characters, specified in “fontset-serif”. The only problem 
is 1) :fontset attribute is not documented and 2) this doesn’t work with 
‘default face.

> 
> For example, this part of your patch:
> 
>> @@ -4040,7 +4055,17 @@ DEFUN ("internal-get-lisp-face-attribute", 
>> Finternal_get_lisp_face_attribute,
>>   else if (EQ (keyword, QCextend))
>>     value = LFACE_EXTEND (lface);
>>   else if (EQ (keyword, QCfont))
>> -    value = LFACE_FONT (lface);
>> +    {
>> +      /* We prefer to return fontset, if it is specified, because font
>> +     are derived from fontset when the user sets the
>> +     fontset. I.e., if the fontset is specified, that's probably
>> +     the one that the user set, and it is intuitive to get back
>> +     what you put in.  */
>> +      if (!UNSPECIFIEDP (LFACE_FONTSET (lface)))
>> +    value = LFACE_FONTSET (lface);
>> +      else
>> +    value = LFACE_FONT (lface);
>> +    }
> 
> This is completely ad-hoc, and could be wrong in some cases: the
> caller wanted the font, but you provide it with a fontset, which is a
> completely different Lisp object.  And there are other similarly
> ad-hoc changes of the semantics of the face attributes.

It is ad-hoc, but that’s just to fit my interpretation of the manual that we 
don’t want to expose :fontset attribute and want to use :font for both :fontset 
and :font. If that interpretation is wrong, we can of course fix the problem in 
other non-ad-hoc way.

> 
> Maybe we can extend the design to support "face-specific" fontsets,
> but I'm quite sure that will need changes in font.c and fontset.c as
> well, because the current design is implicitly assumed there.

Emacs already has an elaborate implementation to support fontsets, it is just 
hindered by the manual and bugs in the Lisp interface.

> 
>> My fix is based on the assumption that we want to use :font attribute for 
>> both font and fontsets, which is what the manual says, and is what Handa-san 
>> said what RMS wanted.
> 
> That's one way of interpreting what the manual says, but it is not the
> only one.  If you look at what the code does, you will arrive at
> another interpretation: Emacs allows you to specify a fontset as the
> value for the :font attribute, but what it does in that case is take
> from the fontset the font for ASCII characters, and then use it as if
> you specified that font, not a fontset.  IOW, the fontset in that case
> is just used as a method of specifying the ASCII font.

I agree, another way is to document the :fontset attribute, document that 
passing a fontset to :font attribute only sets the ASCII font, and fix the bug 
where setting :fontset attribute for ‘default face doesn’t work.

Yuan