bug#57652: 29.0.50; Warning setting attribute of face ‘term’

bug#57652: 29.0.50; Warning setting attribute of face ‘term’

[David Ponce]

Hello,

Since below commit:

Gregory Heytings <gregory@heytings.org>	2022-09-02 22:30:25 +0200
commit	d0a66f3e0e668d8c12c54436740c62f8e238a664
Display a warning for some uses of nil in face attributes.
* src/xfaces.c (HANDLE_INVALID_NIL_VALUE): New macro, which displays
a warning for invalid uses of nil as a face attribute value.
(Finternal_set_lisp_face_attribute): Use the macro for the attributes
:foreground, :distant-foreground and :background.

When opening a term window (M-x term) for the 1st time, the below 
messages are displayed in the *Messages* buffer:

Warning: setting attribute ‘:foreground’ of face ‘term’: nil value is 
invalid, use ‘unspecified’ instead.
Warning: setting attribute ‘:background’ of face ‘term’: nil value is 
invalid, use ‘unspecified’ instead.

The small patch below fixed the issue for me:

diff --git a/term.el b/term.el
index 797fb18074..c564a5429a 100644
--- a/term.el
+++ b/term.el
@@ -755,17 +755,19 @@ ansi-term-color-vector
     term-color-bright-cyan
     term-color-bright-white])

-(defcustom term-default-fg-color nil
-  "If non-nil, default color for foreground in Term mode."
+(defcustom term-default-fg-color 'unspecified
+  "Default color for foreground in Term mode."
    :group 'term
-  :type '(choice (const nil) (string :tag "color")))
+  :type '(choice (const unspecified)
+                 (string :tag "color")))
  (make-obsolete-variable 'term-default-fg-color "use the face `term' 
instead."
                          "24.3")

-(defcustom term-default-bg-color nil
-  "If non-nil, default color for foreground in Term mode."
+(defcustom term-default-bg-color 'unspecified
+  "Default color for foreground in Term mode."
    :group 'term
-  :type '(choice (const nil) (string :tag "color")))
+  :type '(choice (const unspecified)
+                 (string :tag "color")))
  (make-obsolete-variable 'term-default-bg-color "use the face `term' 
instead."
                          "24.3")


Thanks!

Configured using:
  'configure --with-x-toolkit=gtk3 --with-cairo
  PKG_CONFIG_PATH=/usr/local/lib/pkgconfig:/usr/lib/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
   value of $LC_TIME: fr_FR.utf8
   value of $LANG: fr_FR.UTF-8
   locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
   shell-dirtrack-mode: t
   tooltip-mode: t
   global-eldoc-mode: t
   eldoc-mode: t
   show-paren-mode: t
   electric-indent-mode: t
   mouse-wheel-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   blink-cursor-mode: t
   line-number-mode: t
   indent-tabs-mode: t
   transient-mark-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t

Load-path shadows:
None found.

bug#57652: 29.0.50; Warning setting attribute of face ‘term’

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

David Ponce <da_vid@orange.fr> writes:

> diff --git a/term.el b/term.el
> index 797fb18074..c564a5429a 100644
> --- a/term.el
> +++ b/term.el
> @@ -755,17 +755,19 @@ ansi-term-color-vector
>     term-color-bright-cyan
>     term-color-bright-white])
>
> -(defcustom term-default-fg-color nil
> -  "If non-nil, default color for foreground in Term mode."
> +(defcustom term-default-fg-color 'unspecified
> +  "Default color for foreground in Term mode."
>    :group 'term
> -  :type '(choice (const nil) (string :tag "color")))
> +  :type '(choice (const unspecified)
> +                 (string :tag "color")))

Doesn't that change behavior?

i.e. wouldn't it be better to test that term-default-fg-color is not nil
wherever the face is actually made that color?

bug#57652: 29.0.50; Warning setting attribute of face ‘term’

[David Ponce]

On 08/09/2022 07:02, Po Lu wrote:
> David Ponce <da_vid@orange.fr> writes:
> 
>> diff --git a/term.el b/term.el
>> index 797fb18074..c564a5429a 100644
>> --- a/term.el
>> +++ b/term.el
>> @@ -755,17 +755,19 @@ ansi-term-color-vector
>>      term-color-bright-cyan
>>      term-color-bright-white])
>>
>> -(defcustom term-default-fg-color nil
>> -  "If non-nil, default color for foreground in Term mode."
>> +(defcustom term-default-fg-color 'unspecified
>> +  "Default color for foreground in Term mode."
>>     :group 'term
>> -  :type '(choice (const nil) (string :tag "color")))
>> +  :type '(choice (const unspecified)
>> +                 (string :tag "color")))
> 
> Doesn't that change behavior?
> 
> i.e. wouldn't it be better to test that term-default-fg-color is not nil
> wherever the face is actually made that color?

To be honest I don't know.  I reread the spec that says:

    "Apart from the values given below, each face attribute can have the
value ‘unspecified’.  This special value means that the face doesn’t
specify that attribute directly.  An ‘unspecified’ attribute tells Emacs
to refer instead to a parent face (see the description ‘:inherit’
attribute below); or, failing that, to an underlying face (*note
Displaying Faces::).  (However, ‘unspecified’ is not a valid value in
‘defface’.)"

So it seems that unspecified is not allowed in defface.  So, how to 
specify a missing attribute excepted by using nil in the defface used in 
term.el?

(defface term
   `((t
      :foreground ,term-default-fg-color
      :background ,term-default-bg-color
      :inherit default))
   "Default face to use in Term mode."
   :group 'term)

Thanks!

bug#57652: 29.0.50; Warning setting attribute of face ‘term’

[Eli Zaretskii]

> Cc: 57652@debbugs.gnu.org, gregory@heytings.org
> Date: Thu, 8 Sep 2022 10:22:31 +0200
> From: David Ponce <da_vid@orange.fr>
> 
>     "Apart from the values given below, each face attribute can have the
> value ‘unspecified’.  This special value means that the face doesn’t
> specify that attribute directly.  An ‘unspecified’ attribute tells Emacs
> to refer instead to a parent face (see the description ‘:inherit’
> attribute below); or, failing that, to an underlying face (*note
> Displaying Faces::).  (However, ‘unspecified’ is not a valid value in
> ‘defface’.)"
> 
> So it seems that unspecified is not allowed in defface.

Where does it say that unspecified is not allowed?  (And this doc
string is from set-face-attribute, not from defface.)

> So, how to specify a missing attribute excepted by using nil in the
> defface used in term.el?

If you don't have anything to say about an attribute, just don't
mention it in defface.  That's what unspecified means, as the doc
string says.  So I'm unsure why you would need "to specify a missing
attribute".

bug#57652: 29.0.50; Warning setting attribute of face ‘term’

[David Ponce]

On 08/09/2022 10:22, David Ponce wrote:
> On 08/09/2022 07:02, Po Lu wrote:
>> David Ponce <da_vid@orange.fr> writes:
>>
>>> diff --git a/term.el b/term.el
>>> index 797fb18074..c564a5429a 100644
>>> --- a/term.el
>>> +++ b/term.el
>>> @@ -755,17 +755,19 @@ ansi-term-color-vector
>>>      term-color-bright-cyan
>>>      term-color-bright-white])
>>>
>>> -(defcustom term-default-fg-color nil
>>> -  "If non-nil, default color for foreground in Term mode."
>>> +(defcustom term-default-fg-color 'unspecified
>>> +  "Default color for foreground in Term mode."
>>>     :group 'term
>>> -  :type '(choice (const nil) (string :tag "color")))
>>> +  :type '(choice (const unspecified)
>>> +                 (string :tag "color")))
>>
>> Doesn't that change behavior?
>>
>> i.e. wouldn't it be better to test that term-default-fg-color is not nil
>> wherever the face is actually made that color?
> 
> To be honest I don't know.  I reread the spec that says:
> 
>     "Apart from the values given below, each face attribute can have the
> value ‘unspecified’.  This special value means that the face doesn’t
> specify that attribute directly.  An ‘unspecified’ attribute tells Emacs
> to refer instead to a parent face (see the description ‘:inherit’
> attribute below); or, failing that, to an underlying face (*note
> Displaying Faces::).  (However, ‘unspecified’ is not a valid value in
> ‘defface’.)"
> 
> So it seems that unspecified is not allowed in defface.  So, how to 
> specify a missing attribute excepted by using nil in the defface used in 
> term.el?
> 
> (defface term
>    `((t
>       :foreground ,term-default-fg-color
>       :background ,term-default-bg-color
>       :inherit default))
>    "Default face to use in Term mode."
>    :group 'term)
> 
> Thanks!
> 

Also it seems that nil attribute is allowed in some cases in defface, 
because when I show the definition of the default face as a lisp 
expression I get:

((t
   (:family "Source Code Pro" :foundry "ADBO" :width normal :height 102 
:weight regular :slant normal :underline nil :overline nil :extend nil 
:strike-through nil :box nil :inverse-video nil :foreground "black" 
:background "white" :stipple nil :inherit nil)))

Thanks!

bug#57652: 29.0.50; Warning setting attribute of face ‘term’

[Akib Azmain Turja via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

David Ponce <da_vid@orange.fr> writes:

> On 08/09/2022 07:02, Po Lu wrote:
>> David Ponce <da_vid@orange.fr> writes:
>> 
>>> diff --git a/term.el b/term.el
>>> index 797fb18074..c564a5429a 100644
>>> --- a/term.el
>>> +++ b/term.el
>>> @@ -755,17 +755,19 @@ ansi-term-color-vector
>>>      term-color-bright-cyan
>>>      term-color-bright-white])
>>>
>>> -(defcustom term-default-fg-color nil
>>> -  "If non-nil, default color for foreground in Term mode."
>>> +(defcustom term-default-fg-color 'unspecified
>>> +  "Default color for foreground in Term mode."
>>>     :group 'term
>>> -  :type '(choice (const nil) (string :tag "color")))
>>> +  :type '(choice (const unspecified)
>>> +                 (string :tag "color")))
>> Doesn't that change behavior?
>> i.e. wouldn't it be better to test that term-default-fg-color is not
>> nil
>> wherever the face is actually made that color?
>
> To be honest I don't know.  I reread the spec that says:
>
>    "Apart from the values given below, each face attribute can have the
> value ‘unspecified’.  This special value means that the face doesn’t
> specify that attribute directly.  An ‘unspecified’ attribute tells Emacs
> to refer instead to a parent face (see the description ‘:inherit’
> attribute below); or, failing that, to an underlying face (*note
> Displaying Faces::).  (However, ‘unspecified’ is not a valid value in
> ‘defface’.)"
>
> So it seems that unspecified is not allowed in defface.  So, how to
> specify a missing attribute excepted by using nil in the defface used
> in term.el?
>
> (defface term
>   `((t
>      :foreground ,term-default-fg-color
>      :background ,term-default-bg-color
>      :inherit default))
>   "Default face to use in Term mode."
>   :group 'term)
>
> Thanks!
>
>
>
>

How about the following?

--8<---------------cut here---------------start------------->8---
(defface term
  `((t
     ,@(when term-default-fg-color
         '(:foreground ,term-default-fg-color))
     ,@(when term-default-bg-color
         '(:background ,term-default-bg-color))
     :inherit default))
  "Default face to use in Term mode."
  :group 'term)
--8<---------------cut here---------------end--------------->8---

It would not mention the attributes if the colors are nil.

-- 
Akib Azmain Turja

Find me on Mastodon at @akib@hostux.social.

This message is signed by me with my GnuPG key.  Its fingerprint is:

    7001 8CE5 819F 17A3 BBA6  66AF E74F 0EFA 922A E7F5

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

bug#57652: 29.0.50; Warning setting attribute of face ‘term’

[David Ponce]

On 08/09/2022 10:42, Eli Zaretskii wrote:
>> Cc: 57652@debbugs.gnu.org, gregory@heytings.org
>> Date: Thu, 8 Sep 2022 10:22:31 +0200
>> From: David Ponce <da_vid@orange.fr>
>>
>>      "Apart from the values given below, each face attribute can have the
>> value ‘unspecified’.  This special value means that the face doesn’t
>> specify that attribute directly.  An ‘unspecified’ attribute tells Emacs
>> to refer instead to a parent face (see the description ‘:inherit’
>> attribute below); or, failing that, to an underlying face (*note
>> Displaying Faces::).  (However, ‘unspecified’ is not a valid value in
>> ‘defface’.)"
>>
>> So it seems that unspecified is not allowed in defface.
> 
> Where does it say that unspecified is not allowed?  (And this doc
> string is from set-face-attribute, not from defface.)

I mentioned what is currently in the Emacs Lisp reference manual 
(lisp.info): (elisp)Top > Display > Faces > Face Attributes

>> So, how to specify a missing attribute excepted by using nil in the
>> defface used in term.el?
> 
> If you don't have anything to say about an attribute, just don't
> mention it in defface.  That's what unspecified means, as the doc
> string says.  So I'm unsure why you would need "to specify a missing
> attribute".

It is not me, it is how it is currently implemented in term.el:

(defcustom term-default-fg-color nil
   "If non-nil, default color for foreground in Term mode."
   :group 'term
   :type '(choice (const nil) (string :tag "color")))
(make-obsolete-variable 'term-default-fg-color "use the face `term' 
instead."
                         "24.3")

(defcustom term-default-bg-color nil
   "If non-nil, default color for foreground in Term mode."
   :group 'term
   :type '(choice (const nil) (string :tag "color")))
(make-obsolete-variable 'term-default-bg-color "use the face `term' 
instead."
                         "24.3")

(defface term
   `((t
      :foreground ,term-default-fg-color
      :background ,term-default-bg-color
      :inherit default))
   "Default face to use in Term mode."
   :group 'term)