bug#57499: Documentation bug in the docstring of set-face-attribute?

[Gregory Heytings <gregory@heytings.org>]

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


>
> It isn't deprecated, it's invalid.  There's no need to have anything 
> like this in NEWS, because all our documentation says the color 
> attributes can have only string values (in addition to 'unspecified').
>

Okay (I thought you wanted to retain the "undocumented but not really 
invalid" use of nil).

>
> Warning: nil value for `%s' is invalid, use 'unspecified instead.
>

I added the name of the face in the message, which is I think useful to 
locate the cause of the warning.

Should this go to master or to emacs-28?

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Display-a-warning-for-some-uses-of-nil-in-face-attri.patch --]
[-- Type: text/x-diff; name=Display-a-warning-for-some-uses-of-nil-in-face-attri.patch, Size: 2531 bytes --]

From cbe76f8959bc1c7713ae84ab657adb3bbf0c4fde Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Thu, 1 Sep 2022 19:31:34 +0000
Subject: [PATCH] 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.
---
 src/xfaces.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/xfaces.c b/src/xfaces.c
index 70d5cbeb4c..539f50bce9 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -3052,6 +3052,15 @@ DEFUN ("internal-copy-lisp-face", Finternal_copy_lisp_face,
 }
 
 
+#define HANDLE_INVALID_NIL_VALUE(A,F)					\
+  if (NILP (value))							\
+    {									\
+      add_to_log ("Warning: setting attribute `%s' of face `%s': nil "	\
+		  "value is invalid, use 'unspecified instead.", A, F); \
+      /* Compatibility with 20.x.  */					\
+      value = Qunspecified;						\
+    }
+
 DEFUN ("internal-set-lisp-face-attribute", Finternal_set_lisp_face_attribute,
        Sinternal_set_lisp_face_attribute, 3, 4, 0,
        doc: /* Set attribute ATTR of FACE to VALUE.
@@ -3390,9 +3399,7 @@ DEFUN ("internal-set-lisp-face-attribute", Finternal_set_lisp_face_attribute,
     }
   else if (EQ (attr, QCforeground))
     {
-      /* Compatibility with 20.x.  */
-      if (NILP (value))
-	value = Qunspecified;
+      HANDLE_INVALID_NIL_VALUE (QCforeground, face);
       if (!UNSPECIFIEDP (value)
 	  && !IGNORE_DEFFACE_P (value)
 	  && !RESET_P (value))
@@ -3409,9 +3416,7 @@ DEFUN ("internal-set-lisp-face-attribute", Finternal_set_lisp_face_attribute,
     }
   else if (EQ (attr, QCdistant_foreground))
     {
-      /* Compatibility with 20.x.  */
-      if (NILP (value))
-	value = Qunspecified;
+      HANDLE_INVALID_NIL_VALUE (QCdistant_foreground, face);
       if (!UNSPECIFIEDP (value)
 	  && !IGNORE_DEFFACE_P (value)
 	  && !RESET_P (value))
@@ -3428,9 +3433,7 @@ DEFUN ("internal-set-lisp-face-attribute", Finternal_set_lisp_face_attribute,
     }
   else if (EQ (attr, QCbackground))
     {
-      /* Compatibility with 20.x.  */
-      if (NILP (value))
-	value = Qunspecified;
+      HANDLE_INVALID_NIL_VALUE (QCbackground, face);
       if (!UNSPECIFIEDP (value)
 	  && !IGNORE_DEFFACE_P (value)
 	  && !RESET_P (value))
-- 
2.35.1