* bug#67404: 30.0.50; Issue with :box face attribute
@ 2023-11-23 15:23 David Ponce
2023-11-26 10:27 ` Eli Zaretskii
0 siblings, 1 reply; 6+ messages in thread
From: David Ponce @ 2023-11-23 15:23 UTC (permalink / raw)
To: 67404
Hello,
The below face with specified :box attribute can be defined and further
used without problem:
(defface test-face
'((t
(:box (:style released-button :line-width (3 . 0)))
))
"Test face.")
(insert (propertize "XXXXX" 'face 'test-face))
However, it is not possible to apply any change to this face with
`customize-face', for example to change :line-width (3 . 0) to
:line-width (2 . 0), or to add a :foreground color, etc. This fails
with the message "set-face-attribute: Invalid face box: :line-width,
<value>, :style, <value>", and the :box attribute is discarded.
AFAICS, :box attribute with any zero :line-width value works as
expected: the box line with zero width is not displayed. But zero is
not considered as a valid value by `set-face-attribute' (in fact, the
internal function `internal-set-lisp-face-attribute').
For consistency, a zero :line-width value either should be accepted by
`internal-set-lisp-face-attribute', or never accepted at all.
Thanks
In GNU Emacs 30.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version
3.24.38, cairo version 1.18.0) of 2023-11-21
Repository revision: 3328c327254b5846782990f6171175bea216eba9
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12014000
System Description: Fedora Linux 39 (KDE Plasma)
Configured using:
'configure --with-x-toolkit=gtk3 --with-cairo-xcb
--with-native-compilation=no
PKG_CONFIG_PATH=/usr/local/lib/pkgconfig:/usr/lib/pkgconfig'
Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM 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 TREE_SITTER 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
^ permalink raw reply [flat|nested] 6+ messages in thread* bug#67404: 30.0.50; Issue with :box face attribute 2023-11-23 15:23 bug#67404: 30.0.50; Issue with :box face attribute David Ponce @ 2023-11-26 10:27 ` Eli Zaretskii 2023-11-30 10:46 ` Mauro Aranda 0 siblings, 1 reply; 6+ messages in thread From: Eli Zaretskii @ 2023-11-26 10:27 UTC (permalink / raw) To: David Ponce, Mauro Aranda; +Cc: 67404 > Date: Thu, 23 Nov 2023 16:23:15 +0100 > From: David Ponce <da_vid@orange.fr> > > Hello, > > The below face with specified :box attribute can be defined and further > used without problem: > > (defface test-face > '((t > (:box (:style released-button :line-width (3 . 0))) > )) > "Test face.") > > (insert (propertize "XXXXX" 'face 'test-face)) > > However, it is not possible to apply any change to this face with > `customize-face', for example to change :line-width (3 . 0) to > :line-width (2 . 0), or to add a :foreground color, etc. This fails > with the message "set-face-attribute: Invalid face box: :line-width, > <value>, :style, <value>", and the :box attribute is discarded. > > AFAICS, :box attribute with any zero :line-width value works as > expected: the box line with zero width is not displayed. But zero is > not considered as a valid value by `set-face-attribute' (in fact, the > internal function `internal-set-lisp-face-attribute'). > > For consistency, a zero :line-width value either should be accepted by > `internal-set-lisp-face-attribute', or never accepted at all. Mauro, any comments about these issues? ^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#67404: 30.0.50; Issue with :box face attribute 2023-11-26 10:27 ` Eli Zaretskii @ 2023-11-30 10:46 ` Mauro Aranda 2023-11-30 15:20 ` Eli Zaretskii 0 siblings, 1 reply; 6+ messages in thread From: Mauro Aranda @ 2023-11-30 10:46 UTC (permalink / raw) To: Eli Zaretskii, David Ponce; +Cc: 67404 On 26/11/23 07:27, Eli Zaretskii wrote: >> Date: Thu, 23 Nov 2023 16:23:15 +0100 >> From: David Ponce <da_vid@orange.fr> >> >> Hello, >> >> The below face with specified :box attribute can be defined and further >> used without problem: >> >> (defface test-face >> '((t >> (:box (:style released-button :line-width (3 . 0))) >> )) >> "Test face.") >> >> (insert (propertize "XXXXX" 'face 'test-face)) >> >> However, it is not possible to apply any change to this face with >> `customize-face', for example to change :line-width (3 . 0) to >> :line-width (2 . 0), or to add a :foreground color, etc. This fails >> with the message "set-face-attribute: Invalid face box: :line-width, >> <value>, :style, <value>", and the :box attribute is discarded. >> >> AFAICS, :box attribute with any zero :line-width value works as >> expected: the box line with zero width is not displayed. But zero is >> not considered as a valid value by `set-face-attribute' (in fact, the >> internal function `internal-set-lisp-face-attribute'). >> >> For consistency, a zero :line-width value either should be accepted by >> `internal-set-lisp-face-attribute', or never accepted at all. > > Mauro, any comments about these issues? Hi Eli, Not really, no. The widget used for line-width just assumes 0 is a valid value. I don't know if that's the case or not, it is not explicitly documented in the ELisp manual and I don't really know the xfaces.c code. For some reason, the code that rejects a value of 0 in Finternal_set_lisp_face_attribute doesn't complain (maybe it does not get to run?) when using defface, but it does complain when Customize tries to set it, by calling face-spec-set. Sorry I can't be of any more help right now. ^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#67404: 30.0.50; Issue with :box face attribute 2023-11-30 10:46 ` Mauro Aranda @ 2023-11-30 15:20 ` Eli Zaretskii 2023-11-30 16:20 ` David Ponce 0 siblings, 1 reply; 6+ messages in thread From: Eli Zaretskii @ 2023-11-30 15:20 UTC (permalink / raw) To: Mauro Aranda; +Cc: 67404, da_vid > Date: Thu, 30 Nov 2023 07:46:02 -0300 > Cc: 67404@debbugs.gnu.org > From: Mauro Aranda <maurooaranda@gmail.com> > > On 26/11/23 07:27, Eli Zaretskii wrote: > > >> Date: Thu, 23 Nov 2023 16:23:15 +0100 > >> From: David Ponce <da_vid@orange.fr> > >> > >> Hello, > >> > >> The below face with specified :box attribute can be defined and further > >> used without problem: > >> > >> (defface test-face > >> '((t > >> (:box (:style released-button :line-width (3 . 0))) > >> )) > >> "Test face.") > >> > >> (insert (propertize "XXXXX" 'face 'test-face)) > >> > >> However, it is not possible to apply any change to this face with > >> `customize-face', for example to change :line-width (3 . 0) to > >> :line-width (2 . 0), or to add a :foreground color, etc. This fails > >> with the message "set-face-attribute: Invalid face box: :line-width, > >> <value>, :style, <value>", and the :box attribute is discarded. > >> > >> AFAICS, :box attribute with any zero :line-width value works as > >> expected: the box line with zero width is not displayed. But zero is > >> not considered as a valid value by `set-face-attribute' (in fact, the > >> internal function `internal-set-lisp-face-attribute'). > >> > >> For consistency, a zero :line-width value either should be accepted by > >> `internal-set-lisp-face-attribute', or never accepted at all. > > > > Mauro, any comments about these issues? > > Hi Eli, > > Not really, no. The widget used for line-width just assumes 0 is a > valid value. I don't know if that's the case or not, it is not > explicitly documented in the ELisp manual and I don't really know the > xfaces.c code. > > For some reason, the code that rejects a value of 0 in > Finternal_set_lisp_face_attribute doesn't complain (maybe it does not > get to run?) when using defface, but it does complain when Customize > tries to set it, by calling face-spec-set. Thanks for the hints. This is a (very old) bug in Finternal_set_lisp_face_attribute. The value zero for line-width is not valid, but because of the bug it (and invalid values of other box attributes) could be accepted, but only if the invalid attribute is the last attribute of :box. Try this equivalent definition: (defface test-face '((t (:box (:line-width (3 . 0) :style released-button)) )) "Test face.") which just swaps the :box attributes, and you will see that defface signals an error. Customizing the :line-width attribute just happens to move that attribute to the front of the list, and then you get the error. I've now fixed this on the master branch, and now zero width values will always be rejected. ^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#67404: 30.0.50; Issue with :box face attribute 2023-11-30 15:20 ` Eli Zaretskii @ 2023-11-30 16:20 ` David Ponce 2023-11-30 17:53 ` Eli Zaretskii 0 siblings, 1 reply; 6+ messages in thread From: David Ponce @ 2023-11-30 16:20 UTC (permalink / raw) To: Eli Zaretskii, Mauro Aranda; +Cc: 67404 On 30/11/2023 16:20, Eli Zaretskii wrote: >> Date: Thu, 30 Nov 2023 07:46:02 -0300 >> Cc: 67404@debbugs.gnu.org >> From: Mauro Aranda <maurooaranda@gmail.com> >> >> On 26/11/23 07:27, Eli Zaretskii wrote: >> >> >> Date: Thu, 23 Nov 2023 16:23:15 +0100 >> >> From: David Ponce <da_vid@orange.fr> >> >> >> >> Hello, >> >> >> >> The below face with specified :box attribute can be defined and further >> >> used without problem: >> >> >> >> (defface test-face >> >> '((t >> >> (:box (:style released-button :line-width (3 . 0))) >> >> )) >> >> "Test face.") >> >> >> >> (insert (propertize "XXXXX" 'face 'test-face)) >> >> >> >> However, it is not possible to apply any change to this face with >> >> `customize-face', for example to change :line-width (3 . 0) to >> >> :line-width (2 . 0), or to add a :foreground color, etc. This fails >> >> with the message "set-face-attribute: Invalid face box: :line-width, >> >> <value>, :style, <value>", and the :box attribute is discarded. >> >> >> >> AFAICS, :box attribute with any zero :line-width value works as >> >> expected: the box line with zero width is not displayed. But zero is >> >> not considered as a valid value by `set-face-attribute' (in fact, the >> >> internal function `internal-set-lisp-face-attribute'). >> >> >> >> For consistency, a zero :line-width value either should be accepted by >> >> `internal-set-lisp-face-attribute', or never accepted at all. >> > >> > Mauro, any comments about these issues? >> >> Hi Eli, >> >> Not really, no. The widget used for line-width just assumes 0 is a >> valid value. I don't know if that's the case or not, it is not >> explicitly documented in the ELisp manual and I don't really know the >> xfaces.c code. >> >> For some reason, the code that rejects a value of 0 in >> Finternal_set_lisp_face_attribute doesn't complain (maybe it does not >> get to run?) when using defface, but it does complain when Customize >> tries to set it, by calling face-spec-set. > > Thanks for the hints. This is a (very old) bug in > Finternal_set_lisp_face_attribute. The value zero for line-width is > not valid, but because of the bug it (and invalid values of other box > attributes) could be accepted, but only if the invalid attribute is > the last attribute of :box. Try this equivalent definition: > > (defface test-face > '((t > (:box (:line-width (3 . 0) :style released-button)) > )) > "Test face.") > > which just swaps the :box attributes, and you will see that defface > signals an error. Customizing the :line-width attribute just happens > to move that attribute to the front of the list, and then you get the > error. > > I've now fixed this on the master branch, and now zero width values > will always be rejected. Hi Eli, I confirm that with your fix on master branch, the behavior is now consistent between defface and customize-face: both signal an error on value zero for line-width :-) However, please note that it remains possible to use a value zero for line-width on an anonymous face. For example, in the *scratch* buffer you still can eval the below expression without error: (insert (propertize "xxxx" 'font-lock-face '(:box (:style released-button :line-width (3 . 0))))) Thanks! ^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#67404: 30.0.50; Issue with :box face attribute 2023-11-30 16:20 ` David Ponce @ 2023-11-30 17:53 ` Eli Zaretskii 0 siblings, 0 replies; 6+ messages in thread From: Eli Zaretskii @ 2023-11-30 17:53 UTC (permalink / raw) To: David Ponce; +Cc: 67404-done, maurooaranda > Date: Thu, 30 Nov 2023 17:20:44 +0100 > Cc: 67404@debbugs.gnu.org > From: David Ponce <da_vid@orange.fr> > > I confirm that with your fix on master branch, the behavior is now > consistent between defface and customize-face: both signal an error > on value zero for line-width :-) Thanks for testing, I'm therefore closing this bug. > However, please note that it remains possible to use a value zero for > line-width on an anonymous face. For example, in the *scratch* buffer > you still can eval the below expression without error: > > (insert > (propertize "xxxx" 'font-lock-face > '(:box (:style released-button :line-width (3 . 0))))) This seems to be a general issue with anonymous faces: their attributes are validated much more leniently than those of defface. For example, try: (insert (propertize "xxxx" 'font-lock-face '(:box (:style foobar :line-width 'bogus)))) or (insert (propertize "xxxx" 'font-lock-face '(:underline (foo bar :style bogus :position)))) You get a face that effectively ignores the invalid attributes. I'm not sure we need to change that. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-11-30 17:53 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-11-23 15:23 bug#67404: 30.0.50; Issue with :box face attribute David Ponce 2023-11-26 10:27 ` Eli Zaretskii 2023-11-30 10:46 ` Mauro Aranda 2023-11-30 15:20 ` Eli Zaretskii 2023-11-30 16:20 ` David Ponce 2023-11-30 17:53 ` Eli Zaretskii
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).