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

[Gregory Heytings <gregory@heytings.org>]

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


>
> Not exactly what I had in mind.
>

Alas, I don't have a crystal ball (yet) 😉

What about the attached, which deprecates that backward compability shim 
(instead of stopping to support it)?

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

From 1132daf0b11bc11d34e5806cdbf500f68849ce09 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Thu, 1 Sep 2022 16:59:45 +0000
Subject: [PATCH] Display a warning for some uses of nil for face attributes.

* src/xfaces.c (HANDLE_INVALID_OR_DEPRECATED_NIL_VALUE): New macro,
which displays a warning for invalid or deprecated uses of nil as a
face attribute value.
(Finternal_set_lisp_face_attribute): Use the macro for the attributes
:foreground, :distant-foreground and :background.

* etc/NEWS: Announce the deprecation.
---
 etc/NEWS     |  6 ++++++
 src/xfaces.c | 26 +++++++++++++++++---------
 2 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 89f4cd0ac7..e3abfe76ce 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2754,6 +2754,12 @@ request the name of the ".eln" file which defined a given symbol.
 +++
 ** New macro 'with-memoization' provides a very primitive form of memoization.
 
+---
+** The value nil is now deprecated for some face attributes.
+The face attributes :background, :foreground and :distant-foreground
+could be given a value nil for backward-compatibility with Emacs 20.
+This is now deprecated, the 'unspecified value should be used instead.
+
 ** Themes
 
 ---
diff --git a/src/xfaces.c b/src/xfaces.c
index 70d5cbeb4c..486814fb38 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -3052,6 +3052,20 @@ DEFUN ("internal-copy-lisp-face", Finternal_copy_lisp_face,
 }
 
 
+#define HANDLE_INVALID_OR_DEPRECATED_NIL_VALUE(A)			\
+  if (NILP (value))							\
+    {									\
+      if (EQ (frame, Qt))						\
+	add_to_log ("Warning: invalid value nil for attribute `%s' "	\
+		    "with frame t or nil: use 'unspecified instead of " \
+		    "nil", A);						\
+      else								\
+	add_to_log ("Warning: using value nil for attribute `%s' is "	\
+		    "deprecated: use 'unspecified instead of nil", A);	\
+      /* 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 +3404,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_OR_DEPRECATED_NIL_VALUE (QCforeground);
       if (!UNSPECIFIEDP (value)
 	  && !IGNORE_DEFFACE_P (value)
 	  && !RESET_P (value))
@@ -3409,9 +3421,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_OR_DEPRECATED_NIL_VALUE (QCdistant_foreground);
       if (!UNSPECIFIEDP (value)
 	  && !IGNORE_DEFFACE_P (value)
 	  && !RESET_P (value))
@@ -3428,9 +3438,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_OR_DEPRECATED_NIL_VALUE (QCbackground);
       if (!UNSPECIFIEDP (value)
 	  && !IGNORE_DEFFACE_P (value)
 	  && !RESET_P (value))
-- 
2.35.1