bug#71469: font-lock does not apply standard faces and their descendants

bug#71469: font-lock does not apply standard faces and their descendants

[Konstantin Kharlamov]

While trying to add a face to a major mode I found that both standard
faces¹ and inherited ones are ignored completely.

In steps below I create a simple major mode and it uses an inherited
face, but face does not get applied. NOTE: if you replace the inherited
face with, for example, `font-lock-constant-face`, it will get
highlighted. I.e. the problem somehow bound to faces.

# Steps to reproduce

1. Create `test.el` file as follows:

     (defface test-face
       '((t (:inherit bold)))
       "Test face.")

     (define-derived-mode my-mode fundamental-mode "My Mode"
       "A minimal mode that highlights 'hello world' text."
       (font-lock-add-keywords nil '(("hello world" 0 test-face)))
         (font-lock-flush))
     (add-to-list 'auto-mode-alist (cons "test.txt" 'my-mode))
     (provide 'my-mode)

   And `test.txt` as follows:
   
     ==> hello world <==

2. Launch Emacs as `emacs -Q -l test.el test.txt`
3. Put a caret over the word `hello` and evaluate M-x describe-char

## Expected

The description buffer mentions that `face` is `test-face`.

## Actual

There is no `face` property at all.

# Additional information

Versions tested: Emacs built from February master and a stable 29.3

The problem actually doesn't seem to be related to `defface`, because
passing `bold` to the `font-lock-add-keywords` had similarly no effect
for some reason. More likely it's related to the "standard faces".

1: https://www.gnu.org/software/emacs/manual/html_node/emacs/Standard-Faces.html

bug#71469: font-lock does not apply standard faces and their descendants

[Eli Zaretskii]

tags 71469 notabug
thanks

> From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
> Date: Mon, 10 Jun 2024 14:59:37 +0300
> 
>      (defface test-face
>        '((t (:inherit bold)))
>        "Test face.")
> 
>      (define-derived-mode my-mode fundamental-mode "My Mode"
>        "A minimal mode that highlights 'hello world' text."
>        (font-lock-add-keywords nil '(("hello world" 0 test-face)))

From the ELisp manual:

   Each element of ‘font-lock-keywords’ should have one of these forms:
   [...]
  ‘(MATCHER . FACESPEC)’
       In this kind of element, FACESPEC is an expression whose value
       specifies the face to use for highlighting.  In the simplest case,
       FACESPEC is a Lisp variable (a symbol) whose value is a face name.
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

IOW, there's a difference between a symbol of a variable whose value
is a face name, and that face name itself.

This works for me:

  (defface test-face
    '((t (:inherit bold)))
    "Test face.")
  (defvar test-face 'test-face
    "Face name to use for My Mode.")

  (define-derived-mode my-mode fundamental-mode "My Mode"
    "A minimal mode that highlights 'hello world' text."
    (font-lock-add-keywords nil '(("hello world" 0 test-face)))
    (font-lock-flush))
  (add-to-list 'auto-mode-alist (cons "test.txt" 'my-mode))
  (provide 'my-mode)

bug#71469: [PATCH] Re: bug#71469: font-lock does not apply standard faces and their descendants

[Konstantin Kharlamov]

[-- Attachment #1: Type: text/plain, Size: 2328 bytes --]

On Mon, 2024-06-10 at 15:55 +0300, Eli Zaretskii wrote:
> tags 71469 notabug
> thanks
> 
> > From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
> > Date: Mon, 10 Jun 2024 14:59:37 +0300
> > 
> >      (defface test-face
> >        '((t (:inherit bold)))
> >        "Test face.")
> > 
> >      (define-derived-mode my-mode fundamental-mode "My Mode"
> >        "A minimal mode that highlights 'hello world' text."
> >        (font-lock-add-keywords nil '(("hello world" 0 test-face)))
> 
> From the ELisp manual:
> 
>    Each element of ‘font-lock-keywords’ should have one of these
> forms:
>    [...]
>   ‘(MATCHER . FACESPEC)’
>        In this kind of element, FACESPEC is an expression whose value
>        specifies the face to use for highlighting.  In the simplest
> case,
>        FACESPEC is a Lisp variable (a symbol) whose value is a face
> name.
>       
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> IOW, there's a difference between a symbol of a variable whose value
> is a face name, and that face name itself.
> 
> This works for me:
> 
>   (defface test-face
>     '((t (:inherit bold)))
>     "Test face.")
>   (defvar test-face 'test-face
>     "Face name to use for My Mode.")
> 
>   (define-derived-mode my-mode fundamental-mode "My Mode"
>     "A minimal mode that highlights 'hello world' text."
>     (font-lock-add-keywords nil '(("hello world" 0 test-face)))
>     (font-lock-flush))
>   (add-to-list 'auto-mode-alist (cons "test.txt" 'my-mode))
>   (provide 'my-mode)

Ooh, I see, thank you! So using e.g. a `'test-face` also makes it work.

I'm wondering if it would be okay to mention such nuance in the
"standard faces" documentation, such as with the attached patch? I
think it would be really helpful, because the nuance of how it works is
not obvious at all (it would be much easier if none of the faces such
as font-lock-keyword-face, would be defining a variable). I've spent
for about an hour on this trying in different ways to make it work, and
I also think it wasn't the first time I stumbled upon this. Having the
"standard faces" mention that interaction nuance I think could be
helpful for people in the future.

[-- Attachment #2: 1.patch --]
[-- Type: text/x-patch, Size: 1060 bytes --]

From 02811b6400259c0d9b2dedcca76fec771dcd839a Mon Sep 17 00:00:00 2001
From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
Date: Mon, 10 Jun 2024 16:34:30 +0300
Subject: [PATCH] Mention interaction of standard faces with font-lock-keywords

* doc/emacs/display.texi (standard faces): mention that these faces do
not produce variables and so can't be passed to font-lock-keywords as
is.
---
 doc/emacs/display.texi | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/doc/emacs/display.texi b/doc/emacs/display.texi
index 8f22e3c88da..bcbe68d21ea 100644
--- a/doc/emacs/display.texi
+++ b/doc/emacs/display.texi
@@ -647,7 +647,9 @@ Standard Faces
 @cindex standard faces
 
   Here are the standard faces for specifying text appearance.  You can
-apply them to specific text when you want the effects they produce.
+apply them to specific text when you want the effects they produce. Note
+that these faces do not define variables, so to pass such face to
+@code{font-lock-keywords} you have to quote it.
 
 @table @code
 @item default
-- 
2.45.2

bug#71469: [PATCH] Re: bug#71469: font-lock does not apply standard faces and their descendants

[Eli Zaretskii]

> From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
> Cc: 71469@debbugs.gnu.org
> Date: Mon, 10 Jun 2024 16:41:53 +0300
> 
> I'm wondering if it would be okay to mention such nuance in the
> "standard faces" documentation, such as with the attached patch? I
> think it would be really helpful, because the nuance of how it works is
> not obvious at all (it would be much easier if none of the faces such
> as font-lock-keyword-face, would be defining a variable). I've spent
> for about an hour on this trying in different ways to make it work, and
> I also think it wasn't the first time I stumbled upon this. Having the
> "standard faces" mention that interaction nuance I think could be
> helpful for people in the future.
> 
> 
> From 02811b6400259c0d9b2dedcca76fec771dcd839a Mon Sep 17 00:00:00 2001
> From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
> Date: Mon, 10 Jun 2024 16:34:30 +0300
> Subject: [PATCH] Mention interaction of standard faces with font-lock-keywords
> 
> * doc/emacs/display.texi (standard faces): mention that these faces do
> not produce variables and so can't be passed to font-lock-keywords as
> is.
> ---
>  doc/emacs/display.texi | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/emacs/display.texi b/doc/emacs/display.texi
> index 8f22e3c88da..bcbe68d21ea 100644
> --- a/doc/emacs/display.texi
> +++ b/doc/emacs/display.texi
> @@ -647,7 +647,9 @@ Standard Faces
>  @cindex standard faces
>  
>    Here are the standard faces for specifying text appearance.  You can
> -apply them to specific text when you want the effects they produce.
> +apply them to specific text when you want the effects they produce. Note
> +that these faces do not define variables, so to pass such face to
> +@code{font-lock-keywords} you have to quote it.

I don't think I agree with this addition, so I added Stefan to this
discussion.

Stefan, any comments?

bug#71469: [PATCH] Re: bug#71469: font-lock does not apply standard faces and their descendants

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> I'm wondering if it would be okay to mention such nuance in the
>> "standard faces" documentation, such as with the attached patch? I
>> think it would be really helpful, because the nuance of how it works is
>> not obvious at all (it would be much easier if none of the faces such
>> as font-lock-keyword-face, would be defining a variable).

No face defines a variable.

Some faces (mostly font-lock faces) have an associated variable whose
content is "the face name" (which is also the variable's name), but it's
not a result of the face itself.  It's a separate explicit definition of
a variable with the same name as the face.

And I agree that it would be simpler if we didn't have those, but
they've been with us since `font-lock.el` in Emacs-19, so it's not easy
to get rid of them.

>>    Here are the standard faces for specifying text appearance.  You can
>> -apply them to specific text when you want the effects they produce.
>> +apply them to specific text when you want the effects they produce. Note
>> +that these faces do not define variables, so to pass such face to
>> +@code{font-lock-keywords} you have to quote it.

You *always* need to quote the face name.  Otherwise you're not
referring to the face but to a variable of the same name (and ideally
we'd want to get rid of the cases where this happens to work, because
the only upside is to save you from typing a quote character, while the
downside is to encourage confusion).


        Stefan