* Re: master d0a66f3e0e: Display a warning for some uses of nil in face attributes. [not found] ` <20220902203100.0C8D8C0088A@vcs2.savannah.gnu.org> @ 2022-11-27 17:07 ` Stefan Kangas 2022-11-27 18:04 ` Gregory Heytings 0 siblings, 1 reply; 4+ messages in thread From: Stefan Kangas @ 2022-11-27 17:07 UTC (permalink / raw) To: Gregory Heytings, emacs-devel Gregory Heytings <gregory@heytings.org> writes: > branch: master > commit d0a66f3e0e668d8c12c54436740c62f8e238a664 > Author: Gregory Heytings <gregory@heytings.org> > Commit: Gregory Heytings <gregory@heytings.org> > > 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..5e3a47d7f8 100644 > --- a/src/xfaces.c > +++ b/src/xfaces.c > @@ -3052,6 +3052,15 @@ The value is TO. */) > } > > > +#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; \ > + } > + Thanks, it makes sense to warn users using Emacs 20 face specifications. But should the newly introduced warning be called out in NEWS? Also, why not make this into a function? Is it performance critical enough to warrant using a macro? > 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 @@ FRAME 0 means change the face on all frames, and change the default > } > 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 @@ FRAME 0 means change the face on all frames, and change the default > } > 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 @@ FRAME 0 means change the face on all frames, and change the default > } > 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)) ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: master d0a66f3e0e: Display a warning for some uses of nil in face attributes. 2022-11-27 17:07 ` master d0a66f3e0e: Display a warning for some uses of nil in face attributes Stefan Kangas @ 2022-11-27 18:04 ` Gregory Heytings 2022-11-27 18:20 ` Stefan Kangas 0 siblings, 1 reply; 4+ messages in thread From: Gregory Heytings @ 2022-11-27 18:04 UTC (permalink / raw) To: Stefan Kangas; +Cc: emacs-devel > > But should the newly introduced warning be called out in NEWS? > I don't think so, the nil value there was supported for compatibility with Emacs 20 but was not documented (and the message seems clear enough). > > Also, why not make this into a function? Is it performance critical > enough to warrant using a macro? > It's not a macro for performance reasons, it's a macro because it didn't seem worth to me to make it a function. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: master d0a66f3e0e: Display a warning for some uses of nil in face attributes. 2022-11-27 18:04 ` Gregory Heytings @ 2022-11-27 18:20 ` Stefan Kangas 2022-11-27 18:39 ` Eli Zaretskii 0 siblings, 1 reply; 4+ messages in thread From: Stefan Kangas @ 2022-11-27 18:20 UTC (permalink / raw) To: Gregory Heytings; +Cc: emacs-devel Gregory Heytings <gregory@heytings.org> writes: > It's not a macro for performance reasons, it's a macro because it didn't > seem worth to me to make it a function. I don't follow your reasoning here. I would make it into a function unless there is some specific reason to make it into a macro. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: master d0a66f3e0e: Display a warning for some uses of nil in face attributes. 2022-11-27 18:20 ` Stefan Kangas @ 2022-11-27 18:39 ` Eli Zaretskii 0 siblings, 0 replies; 4+ messages in thread From: Eli Zaretskii @ 2022-11-27 18:39 UTC (permalink / raw) To: Stefan Kangas; +Cc: gregory, emacs-devel > From: Stefan Kangas <stefankangas@gmail.com> > Date: Sun, 27 Nov 2022 10:20:05 -0800 > Cc: emacs-devel@gnu.org > > Gregory Heytings <gregory@heytings.org> writes: > > > It's not a macro for performance reasons, it's a macro because it didn't > > seem worth to me to make it a function. > > I don't follow your reasoning here. I would make it into a function > unless there is some specific reason to make it into a macro. It's the other way around: a function comes with the call overhead, so it cannot be the default. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-11-27 18:39 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <166215065969.27647.10737851450469499017@vcs2.savannah.gnu.org> [not found] ` <20220902203100.0C8D8C0088A@vcs2.savannah.gnu.org> 2022-11-27 17:07 ` master d0a66f3e0e: Display a warning for some uses of nil in face attributes Stefan Kangas 2022-11-27 18:04 ` Gregory Heytings 2022-11-27 18:20 ` Stefan Kangas 2022-11-27 18:39 ` Eli Zaretskii
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.