bug#69942: 30.0.50; Fontification of radio-button widget labels

bug#69942: 30.0.50; Fontification of radio-button widget labels

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

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

In bug#69941 I reported a faulty fontification of radio-button widgets
and noted in passing that the labels associated with the radio buttons
also have unexpected faces, namely, the widget-inactive face regardless
of whether the associated radio buttons are inactive or active (except
for the label of a radio button that has been pressed, which has the
default face).  While the faulty fontification discussed in bug#69941
appears to be a real bug, the widget-inactive face assigned to
radio-button labels is apparently by design -- it was present in the
initial commit of the widget library.  But this seems to me to have been
a UX mistake, since it effectively ignores the semantics implied by the
name widget-inactive.  I think a less surprising UI would be for the
labels to be fontified according to the widget's activation state:
default face when the widget is active and widget-inactive face when
it's inactive.  The attached patches provide two possible
implementations of this UI.

The first patch makes the change unconditionally, treating the current
fontification as a UI/UX bug.  But it may be argued that this aspect of
the widget UI should not be unconditionally changed, since it was
apparently a deliberate design choice and there have been (AFAIK) no
complaints about the semantic discrepancy till now.  The lack of
complaint could be because the widget-inactive face inherits the shadow
face, so it is not sharply different from the default face.  But if one
uses a very different face (as I did for illustrative purposes in
bug#69941), the inconsistency is very obvious and (IMO) jarring.
Nevertheless, to allow keeping the current fontification, the second
patch conditionalizes the change from the current fontification by means
of a user option (with the default being the current fontification).

Is either of these changes acceptable?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: widget-radio-nocust.diff --]
[-- Type: text/x-patch, Size: 1558 bytes --]

diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 172da3db1e0..21848849ba5 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -2652,9 +2652,7 @@ widget-radio-add-item
 		(setq child (if chosen
 				(widget-create-child-value
 				 widget type value)
-			      (widget-create-child widget type)))
-		(unless chosen
-		  (widget-apply child :deactivate)))
+			      (widget-create-child widget type))))
 	       (t
 		(error "Unknown escape `%c'" escape)))))
      ;; Update properties.
@@ -2706,12 +2704,8 @@ widget-radio-value-set
 	     (match (and (not found)
 			 (widget-apply current :match value))))
 	(widget-value-set button match)
-	(if match
-	    (progn
-	      (widget-value-set current value)
-	      (widget-apply current :activate))
-	  (widget-apply current :deactivate))
-	(setq found (or found match))))))
+	(when match (widget-value-set current value))
+        (setq found (or found match))))))

 (defun widget-radio-validate (widget)
   ;; Valid if we have made a valid choice.
@@ -2733,11 +2727,9 @@ widget-radio-action
       (dolist (current (widget-get widget :children))
 	(let* ((button (widget-get current :button)))
 	  (cond ((eq child button)
-		 (widget-value-set button t)
-		 (widget-apply current :activate))
+		 (widget-value-set button t))
 		((widget-value button)
-		 (widget-value-set button nil)
-		 (widget-apply current :deactivate)))))))
+		 (widget-value-set button nil)))))))
   ;; Pass notification to parent.
   (widget-apply widget :notify child event))


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: widget-radio-cust.diff --]
[-- Type: text/x-patch, Size: 2585 bytes --]

diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 172da3db1e0..616286a817d 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -2591,6 +2591,16 @@ 'radio-button
   :off "( )"
   :off-glyph "radio")

+(defcustom widget-radio-face-from-state nil
+  "How to fontify the label of a radio button.
+If non-nil, use `widget-inactive' face for the label if the
+radio-button-choice widget is inactive and default face if it is active.
+If nil, use the default face for the label only if the associated radio
+button is pressed, otherwise use `widget-inactive' face."
+  :type 'boolean
+  :group 'widget-faces
+  :version "30.1")
+
 (defun widget-radio-button-notify (widget _child &optional event)
   ;; Tell daddy.
   (widget-apply (widget-get widget :parent) :action widget event))
@@ -2653,8 +2663,9 @@ widget-radio-add-item
 				(widget-create-child-value
 				 widget type value)
 			      (widget-create-child widget type)))
-		(unless chosen
-		  (widget-apply child :deactivate)))
+		(unless widget-radio-face-from-state
+                  (unless chosen
+		    (widget-apply child :deactivate))))
 	       (t
 		(error "Unknown escape `%c'" escape)))))
      ;; Update properties.
@@ -2706,12 +2717,14 @@ widget-radio-value-set
 	     (match (and (not found)
 			 (widget-apply current :match value))))
 	(widget-value-set button match)
-	(if match
-	    (progn
-	      (widget-value-set current value)
-	      (widget-apply current :activate))
-	  (widget-apply current :deactivate))
-	(setq found (or found match))))))
+	(if widget-radio-face-from-state
+            (when match (widget-value-set current value))
+	  (if match
+              (progn
+                (widget-value-set current value)
+                (widget-apply current :activate))
+            (widget-apply current :deactivate)))
+        (setq found (or found match))))))

 (defun widget-radio-validate (widget)
   ;; Valid if we have made a valid choice.
@@ -2734,10 +2747,12 @@ widget-radio-action
 	(let* ((button (widget-get current :button)))
 	  (cond ((eq child button)
 		 (widget-value-set button t)
-		 (widget-apply current :activate))
+                 (unless widget-radio-face-from-state
+		   (widget-apply current :activate)))
 		((widget-value button)
 		 (widget-value-set button nil)
-		 (widget-apply current :deactivate)))))))
+		 (unless widget-radio-face-from-state
+                   (widget-apply current :deactivate))))))))
   ;; Pass notification to parent.
   (widget-apply widget :notify child event))


[-- Attachment #4: Type: text/plain, Size: 745 bytes --]



In GNU Emacs 30.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version
 3.24.38, cairo version 1.18.0) of 2024-03-22 built on strobelfs2
Repository revision: c1530a2e4973005633ebe00d447f1f3aa1200301
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101009
System Description: Linux From Scratch r12.0-112

Configured using:
 'configure -C --with-xwidgets 'CFLAGS=-Og -g3'
 PKG_CONFIG_PATH=/opt/qt5/lib/pkgconfig'

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

bug#69942: 30.0.50; Fontification of radio-button widget labels

[Eli Zaretskii]

> Date: Fri, 22 Mar 2024 16:04:15 +0100
> From:  Stephen Berman via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> In bug#69941 I reported a faulty fontification of radio-button widgets
> and noted in passing that the labels associated with the radio buttons
> also have unexpected faces, namely, the widget-inactive face regardless
> of whether the associated radio buttons are inactive or active (except
> for the label of a radio button that has been pressed, which has the
> default face).  While the faulty fontification discussed in bug#69941
> appears to be a real bug, the widget-inactive face assigned to
> radio-button labels is apparently by design -- it was present in the
> initial commit of the widget library.  But this seems to me to have been
> a UX mistake, since it effectively ignores the semantics implied by the
> name widget-inactive.  I think a less surprising UI would be for the
> labels to be fontified according to the widget's activation state:
> default face when the widget is active and widget-inactive face when
> it's inactive.  The attached patches provide two possible
> implementations of this UI.
> 
> The first patch makes the change unconditionally, treating the current
> fontification as a UI/UX bug.  But it may be argued that this aspect of
> the widget UI should not be unconditionally changed, since it was
> apparently a deliberate design choice and there have been (AFAIK) no
> complaints about the semantic discrepancy till now.  The lack of
> complaint could be because the widget-inactive face inherits the shadow
> face, so it is not sharply different from the default face.  But if one
> uses a very different face (as I did for illustrative purposes in
> bug#69941), the inconsistency is very obvious and (IMO) jarring.
> Nevertheless, to allow keeping the current fontification, the second
> patch conditionalizes the change from the current fontification by means
> of a user option (with the default being the current fontification).
> 
> Is either of these changes acceptable?

Adding Mauro to the discussion.

bug#69942: 30.0.50; Fontification of radio-button widget labels

[Mauro Aranda]

Stephen Berman <stephen.berman@gmx.net> writes:

 > In bug#69941 I reported a faulty fontification of radio-button widgets
 > and noted in passing that the labels associated with the radio buttons
 > also have unexpected faces, namely, the widget-inactive face regardless
 > of whether the associated radio buttons are inactive or active (except
 > for the label of a radio button that has been pressed, which has the
 > default face).  While the faulty fontification discussed in bug#69941
 > appears to be a real bug, the widget-inactive face assigned to
 > radio-button labels is apparently by design -- it was present in the
 > initial commit of the widget library.  But this seems to me to have been
 > a UX mistake, since it effectively ignores the semantics implied by the
 > name widget-inactive.  I think a less surprising UI would be for the
 > labels to be fontified according to the widget's activation state:
 > default face when the widget is active and widget-inactive face when
 > it's inactive.  The attached patches provide two possible
 > implementations of this UI.
 >
 > The first patch makes the change unconditionally, treating the current
 > fontification as a UI/UX bug.  But it may be argued that this aspect of
 > the widget UI should not be unconditionally changed, since it was
 > apparently a deliberate design choice and there have been (AFAIK) no
 > complaints about the semantic discrepancy till now.  The lack of
 > complaint could be because the widget-inactive face inherits the shadow
 > face, so it is not sharply different from the default face. But if one
 > uses a very different face (as I did for illustrative purposes in
 > bug#69941), the inconsistency is very obvious and (IMO) jarring.
 > Nevertheless, to allow keeping the current fontification, the second
 > patch conditionalizes the change from the current fontification by means
 > of a user option (with the default being the current fontification).
 >
 > Is either of these changes acceptable?

Thanks for working on this.  What about adding a widget-unselected face?
I think that might be the intention with using the widget-inactive face
for unselected radio items.

bug#69942: 30.0.50; Fontification of radio-button widget labels

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

On Sat, 23 Mar 2024 18:05:30 -0300 Mauro Aranda <maurooaranda@gmail.com> wrote:

> Stephen Berman <stephen.berman@gmx.net> writes:
>
>> In bug#69941 I reported a faulty fontification of radio-button widgets
>> and noted in passing that the labels associated with the radio buttons
>> also have unexpected faces, namely, the widget-inactive face regardless
>> of whether the associated radio buttons are inactive or active (except
>> for the label of a radio button that has been pressed, which has the
>> default face).  While the faulty fontification discussed in bug#69941
>> appears to be a real bug, the widget-inactive face assigned to
>> radio-button labels is apparently by design -- it was present in the
>> initial commit of the widget library.  But this seems to me to have been
>> a UX mistake, since it effectively ignores the semantics implied by the
>> name widget-inactive.  I think a less surprising UI would be for the
>> labels to be fontified according to the widget's activation state:
>> default face when the widget is active and widget-inactive face when
>> it's inactive.  The attached patches provide two possible
>> implementations of this UI.
>>
>> The first patch makes the change unconditionally, treating the current
>> fontification as a UI/UX bug.  But it may be argued that this aspect of
>> the widget UI should not be unconditionally changed, since it was
>> apparently a deliberate design choice and there have been (AFAIK) no
>> complaints about the semantic discrepancy till now.  The lack of
>> complaint could be because the widget-inactive face inherits the shadow
>> face, so it is not sharply different from the default face. But if one
>> uses a very different face (as I did for illustrative purposes in
>> bug#69941), the inconsistency is very obvious and (IMO) jarring.
>> Nevertheless, to allow keeping the current fontification, the second
>> patch conditionalizes the change from the current fontification by means
>> of a user option (with the default being the current fontification).
>>
>> Is either of these changes acceptable?
>
> Thanks for working on this.  What about adding a widget-unselected face?
> I think that might be the intention with using the widget-inactive face
> for unselected radio items.

Yes, I agree that was likely the intention.  But I think it's
superfluous: after all, the distinction between selected (or chosen) and
unselected items is already clear from the appearance of the radio
buttons or, with checklist widgets, the check boxes (my patch neglected
checklists, but it's straightforward to account for them: in
widget-checklist-add-item the (widget-apply child :deactivate) sexp
should be wrapped in an (unless widget-radio-face-from-state ...)).

On the other hand, with an unselected face for the labels of the radio
button or check boxes, if it defaults to inheriting the shadow face for
unselected items, that corresponds to the current appearance with the
widget-inactive face, and by setting the widget-unselected face to the
default face, all labels would appear the same, which is what I want.
So for me that's an acceptable alternative to my proposed defcustom.  I
tried to implement it, but I'm not very conversant with the workings of
widget properties and how to apply faces depending on the widget's
state, and I haven't managed to come up with a working implementation
yet.  I'll keep trying, but you or someone else might be able to do it
sooner.

(There is another argument, besides superfluousness, against using a
separate face for unselected items: using multiple check boxes instead
of a checklist, as e.g. recentf-edit-list does.  With these the label of
each check box is supplied by the :tag property, so it is not touched by
the current handling in terms of the child widget's activation state.
I'm not sure if using an unselected face here would be unproblematic or
not.)

Steve Berman

bug#69942: 30.0.50; Fontification of radio-button widget labels

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

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

On Sun, 24 Mar 2024 19:47:16 +0100 Stephen Berman <stephen.berman@gmx.net> wrote:

> On Sat, 23 Mar 2024 18:05:30 -0300 Mauro Aranda <maurooaranda@gmail.com> wrote:
>
>> Stephen Berman <stephen.berman@gmx.net> writes:
>>
>>> In bug#69941 I reported a faulty fontification of radio-button widgets
>>> and noted in passing that the labels associated with the radio buttons
>>> also have unexpected faces, namely, the widget-inactive face regardless
>>> of whether the associated radio buttons are inactive or active (except
>>> for the label of a radio button that has been pressed, which has the
>>> default face).  While the faulty fontification discussed in bug#69941
>>> appears to be a real bug, the widget-inactive face assigned to
>>> radio-button labels is apparently by design -- it was present in the
>>> initial commit of the widget library.  But this seems to me to have been
>>> a UX mistake, since it effectively ignores the semantics implied by the
>>> name widget-inactive.  I think a less surprising UI would be for the
>>> labels to be fontified according to the widget's activation state:
>>> default face when the widget is active and widget-inactive face when
>>> it's inactive.  The attached patches provide two possible
>>> implementations of this UI.
>>>
>>> The first patch makes the change unconditionally, treating the current
>>> fontification as a UI/UX bug.  But it may be argued that this aspect of
>>> the widget UI should not be unconditionally changed, since it was
>>> apparently a deliberate design choice and there have been (AFAIK) no
>>> complaints about the semantic discrepancy till now.  The lack of
>>> complaint could be because the widget-inactive face inherits the shadow
>>> face, so it is not sharply different from the default face. But if one
>>> uses a very different face (as I did for illustrative purposes in
>>> bug#69941), the inconsistency is very obvious and (IMO) jarring.
>>> Nevertheless, to allow keeping the current fontification, the second
>>> patch conditionalizes the change from the current fontification by means
>>> of a user option (with the default being the current fontification).
>>>
>>> Is either of these changes acceptable?
>>
>> Thanks for working on this.  What about adding a widget-unselected face?
>> I think that might be the intention with using the widget-inactive face
>> for unselected radio items.
>
> Yes, I agree that was likely the intention.  But I think it's
> superfluous: after all, the distinction between selected (or chosen) and
> unselected items is already clear from the appearance of the radio
> buttons or, with checklist widgets, the check boxes (my patch neglected
> checklists, but it's straightforward to account for them: in
> widget-checklist-add-item the (widget-apply child :deactivate) sexp
> should be wrapped in an (unless widget-radio-face-from-state ...)).
>
> On the other hand, with an unselected face for the labels of the radio
> button or check boxes, if it defaults to inheriting the shadow face for
> unselected items, that corresponds to the current appearance with the
> widget-inactive face, and by setting the widget-unselected face to the
> default face, all labels would appear the same, which is what I want.
> So for me that's an acceptable alternative to my proposed defcustom.  I
> tried to implement it, but I'm not very conversant with the workings of
> widget properties and how to apply faces depending on the widget's
> state, and I haven't managed to come up with a working implementation
> yet.  I'll keep trying, but you or someone else might be able to do it
> sooner.
>
> (There is another argument, besides superfluousness, against using a
> separate face for unselected items: using multiple check boxes instead
> of a checklist, as e.g. recentf-edit-list does.  With these the label of
> each check box is supplied by the :tag property, so it is not touched by
> the current handling in terms of the child widget's activation state.
> I'm not sure if using an unselected face here would be unproblematic or
> not.)

Ok, I've gotten further with implementing disinguishing by faces
selected (chosen) and unselected radio buttons in radio-button-choice
widgets and check boxes in checklist widgets, see the attached patch.
Initial tests seem ok, but it definitely needs more testing.

Steve Berman


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: selected and unselected widgets --]
[-- Type: text/x-patch, Size: 5205 bytes --]

diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 172da3db1e0..005aa918087 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -555,6 +555,27 @@ widget-specify-active
       (delete-overlay inactive)
       (widget-put widget :inactive nil))))

+(defface widget-unselected
+  '((t :inherit shadow))
+  "Face used for unselected widgets."
+  :group 'widget-faces
+  :version "30.1")
+
+(defun widget-specify-unselected (widget from to)
+  "Fontify WIDGET as unselected (not chosen)."
+  (let ((overlay (make-overlay from to nil t nil)))
+    (overlay-put overlay 'face 'widget-unselected)
+    (overlay-put overlay 'evaporate t)
+    (overlay-put overlay 'priority 100)
+    (widget-put widget :unselected overlay)))
+
+(defun widget-specify-selected (widget)
+  "Remove fontification of WIDGET as unselected (not chosen)."
+  (let ((unselected (widget-get widget :unselected)))
+    (when unselected
+      (delete-overlay unselected)
+      (widget-put widget :unselected nil))))
+
 ;;; Widget Properties.

 (defsubst widget-type (widget)
@@ -2415,10 +2436,16 @@ 'checkbox
 (defun widget-checkbox-action (widget &optional event)
   "Toggle checkbox, notify parent, and set active state of sibling."
   (widget-toggle-action widget event)
-  (let ((sibling (widget-get-sibling widget)))
+  (let* ((sibling (widget-get-sibling widget))
+         (from (widget-get sibling :from))
+	 (to (widget-get sibling :to)))
     (when sibling
-      (widget-apply sibling
-	            (if (widget-value widget) :activate :deactivate))
+      (if (widget-value widget)
+          (progn
+            (widget-apply sibling :activate)
+            (widget-specify-selected sibling))
+        :deactivate
+        (widget-specify-unselected sibling from to))
       (widget-clear-undo))))

 ;;; The `checklist' Widget.
@@ -2474,15 +2501,19 @@ widget-checklist-add-item
 	       ((eq escape ?v)
 		(setq child
 		      (cond ((not chosen)
-			     (let ((child (widget-create-child widget type)))
-			       (widget-apply child :deactivate)
+			     (let* ((child (widget-create-child widget type))
+                                    (from (widget-get child :from))
+			            (to (widget-get child :to)))
+                               (widget-specify-unselected child from to)
 			       child))
                             ((widget-inline-p type t)
 			     (widget-create-child-value
-			      widget type (cdr chosen)))
+			      widget type (cdr chosen))
+                             (widget-specify-selected child))
 			    (t
 			     (widget-create-child-value
-			      widget type (car (cdr chosen)))))))
+			      widget type (car (cdr chosen)))
+                             (widget-specify-selected child)))))
 	       (t
 		(error "Unknown escape `%c'" escape)))))
      ;; Update properties.
@@ -2653,8 +2684,11 @@ widget-radio-add-item
 				(widget-create-child-value
 				 widget type value)
 			      (widget-create-child widget type)))
-		(unless chosen
-		  (widget-apply child :deactivate)))
+                (if chosen
+                    (widget-specify-selected child)
+                  (let ((from (widget-get child :from))
+			(to (widget-get child :to)))
+                    (widget-specify-unselected child from to))))
 	       (t
 		(error "Unknown escape `%c'" escape)))))
      ;; Update properties.
@@ -2704,14 +2738,17 @@ widget-radio-value-set
     (dolist (current (widget-get widget :children))
       (let* ((button (widget-get current :button))
 	     (match (and (not found)
-			 (widget-apply current :match value))))
+			 (widget-apply current :match value)))
+             (from (widget-get current :from))
+	     (to (widget-get current :to)))
 	(widget-value-set button match)
 	(if match
-	    (progn
-	      (widget-value-set current value)
-	      (widget-apply current :activate))
-	  (widget-apply current :deactivate))
-	(setq found (or found match))))))
+            (progn
+              (widget-value-set current value)
+              (widget-apply current :activate)
+              (widget-specify-selected current))
+          (widget-specify-unselected current from to))
+        (setq found (or found match))))))

 (defun widget-radio-validate (widget)
   ;; Valid if we have made a valid choice.
@@ -2731,13 +2768,16 @@ widget-radio-action
   (let ((buttons (widget-get widget :buttons)))
     (when (memq child buttons)
       (dolist (current (widget-get widget :children))
-	(let* ((button (widget-get current :button)))
+	(let* ((button (widget-get current :button))
+               (from (widget-get current :from))
+	       (to (widget-get current :to)))
 	  (cond ((eq child button)
 		 (widget-value-set button t)
-		 (widget-apply current :activate))
+		 (widget-apply current :activate)
+                 (widget-specify-selected current))
 		((widget-value button)
 		 (widget-value-set button nil)
-		 (widget-apply current :deactivate)))))))
+                 (widget-specify-unselected current from to)))))))
   ;; Pass notification to parent.
   (widget-apply widget :notify child event))

bug#69942: 30.0.50; Fontification of radio-button widget labels

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

On Mon, 25 Mar 2024 01:40:36 +0100 Stephen Berman <stephen.berman@gmx.net> wrote:

> On Sun, 24 Mar 2024 19:47:16 +0100 Stephen Berman <stephen.berman@gmx.net> wrote:
>
>> On Sat, 23 Mar 2024 18:05:30 -0300 Mauro Aranda <maurooaranda@gmail.com> wrote:
>>
>>> Stephen Berman <stephen.berman@gmx.net> writes:
>>>
>>>> In bug#69941 I reported a faulty fontification of radio-button widgets
>>>> and noted in passing that the labels associated with the radio buttons
>>>> also have unexpected faces, namely, the widget-inactive face regardless
>>>> of whether the associated radio buttons are inactive or active (except
>>>> for the label of a radio button that has been pressed, which has the
>>>> default face).  While the faulty fontification discussed in bug#69941
>>>> appears to be a real bug, the widget-inactive face assigned to
>>>> radio-button labels is apparently by design -- it was present in the
>>>> initial commit of the widget library.  But this seems to me to have been
>>>> a UX mistake, since it effectively ignores the semantics implied by the
>>>> name widget-inactive.  I think a less surprising UI would be for the
>>>> labels to be fontified according to the widget's activation state:
>>>> default face when the widget is active and widget-inactive face when
>>>> it's inactive.  The attached patches provide two possible
>>>> implementations of this UI.
>>>>
>>>> The first patch makes the change unconditionally, treating the current
>>>> fontification as a UI/UX bug.  But it may be argued that this aspect of
>>>> the widget UI should not be unconditionally changed, since it was
>>>> apparently a deliberate design choice and there have been (AFAIK) no
>>>> complaints about the semantic discrepancy till now.  The lack of
>>>> complaint could be because the widget-inactive face inherits the shadow
>>>> face, so it is not sharply different from the default face. But if one
>>>> uses a very different face (as I did for illustrative purposes in
>>>> bug#69941), the inconsistency is very obvious and (IMO) jarring.
>>>> Nevertheless, to allow keeping the current fontification, the second
>>>> patch conditionalizes the change from the current fontification by means
>>>> of a user option (with the default being the current fontification).
>>>>
>>>> Is either of these changes acceptable?
>>>
>>> Thanks for working on this.  What about adding a widget-unselected face?
>>> I think that might be the intention with using the widget-inactive face
>>> for unselected radio items.
>>
>> Yes, I agree that was likely the intention.  But I think it's
>> superfluous: after all, the distinction between selected (or chosen) and
>> unselected items is already clear from the appearance of the radio
>> buttons or, with checklist widgets, the check boxes (my patch neglected
>> checklists, but it's straightforward to account for them: in
>> widget-checklist-add-item the (widget-apply child :deactivate) sexp
>> should be wrapped in an (unless widget-radio-face-from-state ...)).
>>
>> On the other hand, with an unselected face for the labels of the radio
>> button or check boxes, if it defaults to inheriting the shadow face for
>> unselected items, that corresponds to the current appearance with the
>> widget-inactive face, and by setting the widget-unselected face to the
>> default face, all labels would appear the same, which is what I want.
>> So for me that's an acceptable alternative to my proposed defcustom.  I
>> tried to implement it, but I'm not very conversant with the workings of
>> widget properties and how to apply faces depending on the widget's
>> state, and I haven't managed to come up with a working implementation
>> yet.  I'll keep trying, but you or someone else might be able to do it
>> sooner.
>>
>> (There is another argument, besides superfluousness, against using a
>> separate face for unselected items: using multiple check boxes instead
>> of a checklist, as e.g. recentf-edit-list does.  With these the label of
>> each check box is supplied by the :tag property, so it is not touched by
>> the current handling in terms of the child widget's activation state.
>> I'm not sure if using an unselected face here would be unproblematic or
>> not.)
>
> Ok, I've gotten further with implementing disinguishing by faces
> selected (chosen) and unselected radio buttons in radio-button-choice
> widgets and check boxes in checklist widgets, see the attached patch.
> Initial tests seem ok, but it definitely needs more testing.

Any comments on this patch for using a widget-unselected face?  I have
been detained from further testing this past week, but can now resume.

Steve Berman

bug#69942: 30.0.50; Fontification of radio-button widget labels

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  69942@debbugs.gnu.org
> Date: Mon, 01 Apr 2024 17:21:27 +0200
> 
> On Mon, 25 Mar 2024 01:40:36 +0100 Stephen Berman <stephen.berman@gmx.net> wrote:
> 
> > Ok, I've gotten further with implementing disinguishing by faces
> > selected (chosen) and unselected radio buttons in radio-button-choice
> > widgets and check boxes in checklist widgets, see the attached patch.
> > Initial tests seem ok, but it definitely needs more testing.
> 
> Any comments on this patch for using a widget-unselected face?  I have
> been detained from further testing this past week, but can now resume.

Mauro, any further comments?

bug#69942: 30.0.50; Fontification of radio-button widget labels

[Mauro Aranda]

On 6/4/24 06:02, Eli Zaretskii wrote:
 >> From: Stephen Berman <stephen.berman@gmx.net>
 >> Cc: Eli Zaretskii <eliz@gnu.org>, 69942@debbugs.gnu.org
 >> Date: Mon, 01 Apr 2024 17:21:27 +0200
 >>
 >> On Mon, 25 Mar 2024 01:40:36 +0100 Stephen Berman 
<stephen.berman@gmx.net> wrote:
 >>
 >>> Ok, I've gotten further with implementing disinguishing by faces
 >>> selected (chosen) and unselected radio buttons in radio-button-choice
 >>> widgets and check boxes in checklist widgets, see the attached patch.
 >>> Initial tests seem ok, but it definitely needs more testing.
 >>
 >> Any comments on this patch for using a widget-unselected face?  I have
 >> been detained from further testing this past week, but can now resume.
 >
 > Mauro, any further comments?

Hi Eli and Stephen,

Please forgive me, for the past 2 weeks I haven't been able to do any
computer stuff.  If it's OK, please give me until the weekend so I
can catch up with this and the other 2 bug reports by Stephen.

bug#69942: 30.0.50; Fontification of radio-button widget labels

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

On Mon, 8 Apr 2024 07:58:44 -0300 Mauro Aranda <maurooaranda@gmail.com> wrote:

> On 6/4/24 06:02, Eli Zaretskii wrote:
>>> From: Stephen Berman <stephen.berman@gmx.net>
>>> Cc: Eli Zaretskii <eliz@gnu.org>, 69942@debbugs.gnu.org
>>> Date: Mon, 01 Apr 2024 17:21:27 +0200
>>>
>>> On Mon, 25 Mar 2024 01:40:36 +0100 Stephen Berman <stephen.berman@gmx.net>
>   wrote:
>>>
>>>> Ok, I've gotten further with implementing disinguishing by faces
>>>> selected (chosen) and unselected radio buttons in radio-button-choice
>>>> widgets and check boxes in checklist widgets, see the attached patch.
>>>> Initial tests seem ok, but it definitely needs more testing.
>>>
>>> Any comments on this patch for using a widget-unselected face?  I have
>>> been detained from further testing this past week, but can now resume.
>>
>> Mauro, any further comments?
>
> Hi Eli and Stephen,
>
> Please forgive me, for the past 2 weeks I haven't been able to do any
> computer stuff.  If it's OK, please give me until the weekend so I
> can catch up with this and the other 2 bug reports by Stephen.

Please take your time; I'm in no rush.

Steve Berman

bug#69942: 30.0.50; Fontification of radio-button widget labels

[Eli Zaretskii]

> Date: Mon, 8 Apr 2024 07:58:44 -0300
> Cc: 69942@debbugs.gnu.org
> From: Mauro Aranda <maurooaranda@gmail.com>
> 
>  > Mauro, any further comments?
> 
> Hi Eli and Stephen,
> 
> Please forgive me, for the past 2 weeks I haven't been able to do any
> computer stuff.  If it's OK, please give me until the weekend so I
> can catch up with this and the other 2 bug reports by Stephen.

No need to apologize, we all have our Real Lives.

Take your time, there's no urgency.  Thanks in advance.

bug#69942: 30.0.50; Fontification of radio-button widget labels

[Eli Zaretskii]

> Date: Mon, 8 Apr 2024 07:58:44 -0300
> Cc: 69942@debbugs.gnu.org
> From: Mauro Aranda <maurooaranda@gmail.com>
> 
> On 6/4/24 06:02, Eli Zaretskii wrote:
>  >> From: Stephen Berman <stephen.berman@gmx.net>
>  >> Cc: Eli Zaretskii <eliz@gnu.org>, 69942@debbugs.gnu.org
>  >> Date: Mon, 01 Apr 2024 17:21:27 +0200
>  >>
>  >> On Mon, 25 Mar 2024 01:40:36 +0100 Stephen Berman 
> <stephen.berman@gmx.net> wrote:
>  >>
>  >>> Ok, I've gotten further with implementing disinguishing by faces
>  >>> selected (chosen) and unselected radio buttons in radio-button-choice
>  >>> widgets and check boxes in checklist widgets, see the attached patch.
>  >>> Initial tests seem ok, but it definitely needs more testing.
>  >>
>  >> Any comments on this patch for using a widget-unselected face?  I have
>  >> been detained from further testing this past week, but can now resume.
>  >
>  > Mauro, any further comments?
> 
> Hi Eli and Stephen,
> 
> Please forgive me, for the past 2 weeks I haven't been able to do any
> computer stuff.  If it's OK, please give me until the weekend so I
> can catch up with this and the other 2 bug reports by Stephen.

Mauro, were you able to find time to look into this and the other 2
bugs?

bug#69942: 30.0.50; Fontification of radio-button widget labels

[Mauro Aranda]

Eli Zaretskii <eliz@gnu.org> writes:

 >> Date: Mon, 8 Apr 2024 07:58:44 -0300
 >> Cc: 69942@debbugs.gnu.org
 >> From: Mauro Aranda <maurooaranda@gmail.com>
 >>
 >> On 6/4/24 06:02, Eli Zaretskii wrote:
 >>  >> From: Stephen Berman <stephen.berman@gmx.net>
 >>  >> Cc: Eli Zaretskii <eliz@gnu.org>, 69942@debbugs.gnu.org
 >>  >> Date: Mon, 01 Apr 2024 17:21:27 +0200
 >>  >>
 >>  >> On Mon, 25 Mar 2024 01:40:36 +0100 Stephen Berman
 >> <stephen.berman@gmx.net> wrote:
 >>  >>
 >>  >>> Ok, I've gotten further with implementing disinguishing by faces
 >>  >>> selected (chosen) and unselected radio buttons in 
radio-button-choice
 >>  >>> widgets and check boxes in checklist widgets, see the attached 
patch.
 >>  >>> Initial tests seem ok, but it definitely needs more testing.
 >>  >>
 >>  >> Any comments on this patch for using a widget-unselected face?  
I have
 >>  >> been detained from further testing this past week, but can now 
resume.
 >>  >
 >>  > Mauro, any further comments?
 >>
 >> Hi Eli and Stephen,
 >>
 >> Please forgive me, for the past 2 weeks I haven't been able to do any
 >> computer stuff.  If it's OK, please give me until the weekend so I
 >> can catch up with this and the other 2 bug reports by Stephen.
 >
 > Mauro, were you able to find time to look into this and the other 2
 > bugs?

I have, just now.  The patch looks good to me.  It'll be great if
Stephen can add some documentation to the manual, so it stays updated.
If not, I can do that in a few days.

bug#69942: 30.0.50; Fontification of radio-button widget labels

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

On Thu, 18 Apr 2024 07:07:43 -0300 Mauro Aranda <maurooaranda@gmail.com> wrote:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> Date: Mon, 8 Apr 2024 07:58:44 -0300
>>> Cc: 69942@debbugs.gnu.org
>>> From: Mauro Aranda <maurooaranda@gmail.com>
>>>
>>> On 6/4/24 06:02, Eli Zaretskii wrote:
>>>  >> From: Stephen Berman <stephen.berman@gmx.net>
>>>  >> Cc: Eli Zaretskii <eliz@gnu.org>, 69942@debbugs.gnu.org
>>>  >> Date: Mon, 01 Apr 2024 17:21:27 +0200
>>>  >>
>>>  >> On Mon, 25 Mar 2024 01:40:36 +0100 Stephen Berman
>>> <stephen.berman@gmx.net> wrote:
>>>  >>
>>>  >>> Ok, I've gotten further with implementing disinguishing by faces
>>>  >>> selected (chosen) and unselected radio buttons in radio-button-choice
>>>  >>> widgets and check boxes in checklist widgets, see the attached patch.
>>>  >>> Initial tests seem ok, but it definitely needs more testing.
>>>  >>
>>>  >> Any comments on this patch for using a widget-unselected face?  I have
>>>  >> been detained from further testing this past week, but can now resume.
>>>  >
>>>  > Mauro, any further comments?
>>>
>>> Hi Eli and Stephen,
>>>
>>> Please forgive me, for the past 2 weeks I haven't been able to do any
>>> computer stuff.  If it's OK, please give me until the weekend so I
>>> can catch up with this and the other 2 bug reports by Stephen.
>>
>> Mauro, were you able to find time to look into this and the other 2
>> bugs?
>
> I have, just now.  The patch looks good to me.  It'll be great if
> Stephen can add some documentation to the manual, so it stays updated.
> If not, I can do that in a few days.

Sure, I'll add documentation and post it here for approval before
pushing the changes.

Steve Berman

bug#69942: 30.0.50; Fontification of radio-button widget labels

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

On Thu, 18 Apr 2024 15:37:58 +0200 Stephen Berman <stephen.berman@gmx.net> wrote:

> On Thu, 18 Apr 2024 07:07:43 -0300 Mauro Aranda <maurooaranda@gmail.com> wrote:
>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>>>> Date: Mon, 8 Apr 2024 07:58:44 -0300
>>>> Cc: 69942@debbugs.gnu.org
>>>> From: Mauro Aranda <maurooaranda@gmail.com>
>>>>
>>>> On 6/4/24 06:02, Eli Zaretskii wrote:
>>>>  >> From: Stephen Berman <stephen.berman@gmx.net>
>>>>  >> Cc: Eli Zaretskii <eliz@gnu.org>, 69942@debbugs.gnu.org
>>>>  >> Date: Mon, 01 Apr 2024 17:21:27 +0200
>>>>  >>
>>>>  >> On Mon, 25 Mar 2024 01:40:36 +0100 Stephen Berman
>>>> <stephen.berman@gmx.net> wrote:
>>>>  >>
>>>>  >>> Ok, I've gotten further with implementing disinguishing by faces
>>>>  >>> selected (chosen) and unselected radio buttons in radio-button-choice
>>>>  >>> widgets and check boxes in checklist widgets, see the attached patch.
>>>>  >>> Initial tests seem ok, but it definitely needs more testing.
>>>>  >>
>>>>  >> Any comments on this patch for using a widget-unselected face?  I have
>>>>  >> been detained from further testing this past week, but can now resume.
>>>>  >
>>>>  > Mauro, any further comments?
>>>>
>>>> Hi Eli and Stephen,
>>>>
>>>> Please forgive me, for the past 2 weeks I haven't been able to do any
>>>> computer stuff.  If it's OK, please give me until the weekend so I
>>>> can catch up with this and the other 2 bug reports by Stephen.
>>>
>>> Mauro, were you able to find time to look into this and the other 2
>>> bugs?
>>
>> I have, just now.  The patch looks good to me.  It'll be great if
>> Stephen can add some documentation to the manual, so it stays updated.
>> If not, I can do that in a few days.
>
> Sure, I'll add documentation and post it here for approval before
> pushing the changes.

I've encountered some problems with the patch.  One is that it breaks
the display of all face attributes in the customize-face buffer.  I've
determined the part of the patch that triggers this, though I haven't
yet figured out just why this bit of code breaks the display.  Also, it
appears that the widget-unselected face does not completely replace
widget-inactive where it's intended to do so, but I need to do more
testing and debugging here to find out why.  Until I've fixed these
issues the patch is not suitable for installing, so I'm also holding off
with the accompanying documentation.  (But in preparation for the
documentation I looked more closely at the Widget manual and found
several typos and other issues, for which I opened bug#70502.)

Steve Berman

bug#69942: 30.0.50; Fontification of radio-button widget labels

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

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

On Sun, 21 Apr 2024 21:45:21 +0200 Stephen Berman <stephen.berman@gmx.net> wrote:

> On Thu, 18 Apr 2024 15:37:58 +0200 Stephen Berman <stephen.berman@gmx.net> wrote:
>
>> On Thu, 18 Apr 2024 07:07:43 -0300 Mauro Aranda <maurooaranda@gmail.com> wrote:
>>
>>> Eli Zaretskii <eliz@gnu.org> writes:
>>>
>>>>> Date: Mon, 8 Apr 2024 07:58:44 -0300
>>>>> Cc: 69942@debbugs.gnu.org
>>>>> From: Mauro Aranda <maurooaranda@gmail.com>
>>>>>
>>>>> On 6/4/24 06:02, Eli Zaretskii wrote:
>>>>>  >> From: Stephen Berman <stephen.berman@gmx.net>
>>>>>  >> Cc: Eli Zaretskii <eliz@gnu.org>, 69942@debbugs.gnu.org
>>>>>  >> Date: Mon, 01 Apr 2024 17:21:27 +0200
>>>>>  >>
>>>>>  >> On Mon, 25 Mar 2024 01:40:36 +0100 Stephen Berman
>>>>> <stephen.berman@gmx.net> wrote:
>>>>>  >>
>>>>>  >>> Ok, I've gotten further with implementing disinguishing by faces
>>>>>  >>> selected (chosen) and unselected radio buttons in radio-button-choice
>>>>>  >>> widgets and check boxes in checklist widgets, see the attached patch.
>>>>>  >>> Initial tests seem ok, but it definitely needs more testing.
>>>>>  >>
>>>>>  >> Any comments on this patch for using a widget-unselected face?  I have
>>>>>  >> been detained from further testing this past week, but can now resume.
>>>>>  >
>>>>>  > Mauro, any further comments?
>>>>>
>>>>> Hi Eli and Stephen,
>>>>>
>>>>> Please forgive me, for the past 2 weeks I haven't been able to do any
>>>>> computer stuff.  If it's OK, please give me until the weekend so I
>>>>> can catch up with this and the other 2 bug reports by Stephen.
>>>>
>>>> Mauro, were you able to find time to look into this and the other 2
>>>> bugs?
>>>
>>> I have, just now.  The patch looks good to me.  It'll be great if
>>> Stephen can add some documentation to the manual, so it stays updated.
>>> If not, I can do that in a few days.
>>
>> Sure, I'll add documentation and post it here for approval before
>> pushing the changes.
>
> I've encountered some problems with the patch.  One is that it breaks
> the display of all face attributes in the customize-face buffer.  I've
> determined the part of the patch that triggers this, though I haven't
> yet figured out just why this bit of code breaks the display.  Also, it
> appears that the widget-unselected face does not completely replace
> widget-inactive where it's intended to do so, but I need to do more
> testing and debugging here to find out why.  Until I've fixed these
> issues the patch is not suitable for installing, so I'm also holding off
> with the accompanying documentation.  (But in preparation for the
> documentation I looked more closely at the Widget manual and found
> several typos and other issues, for which I opened bug#70502.)

The breakage in displaying all face attributes in the customize-face
buffer was caused by the invocation of `(widget-specify-selected child)'
in widget-checklist-add-item in the cond-clause satisfying
`(widget-inline-p type t)'.  I still don't understand why it has this
effect, and I have to admit that I don't understand what an inline
widget is.  But simply omitting the invocation of
`(widget-specify-selected child)' at this point in the patch does avoid
the customize-face breakage and I have not noticed any problems due to
this omission.

As for the problematic interaction between the widget-unselected and
widget-inactive faces, this seems to have been due to my having copied
most of the definition of widget-specify-unselected from that of
widget-specify-inactive, and specifically, copying the overlay priority.
In the attached patch, widget-specify-unselected now uses a lower
overlay priority than the one used in widget-specify-inactive, and in my
tests this yields the desired results: the labels of active checkboxes
and radio-buttons have widget-unselected face but when these widgets are
deactivated, the labels have widget-inactive face.

Another change I've made in the attached patch is to have the default
value of widget-unselected face inherit from widget-inactive instead of
inheriting from shadow face, like widget-inactive does by default.  This
way, if a user customizes widget-inactive, that will also apply by
default to widget-unselected, thus retaining the current default widget
UI where the labels of checkboxes and radio-buttons have widget-inactive
face.  (Thus, the "desired results" in the preceding paragraph are only
visible when widget-unselected face is customized to differ from
widget-unselected face.)

Finally, regarding documentation of widget-unselected face in the Widget
manual, I think it would be helpful for the documentation to mention the
use case that motivated introducing it (following Mauro's suggestion to
use a face instead of a defcustom), namely, to visually distinguish the
labels of unselected and inactive widgets.  Here is what I suggest:

@deffn Face widget-unselected
Face used for unselected widgets.  This face is also used on the text
labels of radio-button and checkbox widgets.

The default value inherits @code{widget-inactive} face.  If you want to
visually distinguish the labels of unselected active radio-button or
checkbox widgets from the labels of unselected inactive widgets,
customize this face to a non-default value.
@end deffn

Since the recent widget.texi changes that include documenting
widget-inactive face have not yet been merged to master, I haven't
included the propopsed documentaton of widget-unselected in the attached
patch, but if approved, will of course add it after the merge.

Steve Berman


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: widget-unselected patch --]
[-- Type: text/x-patch, Size: 5230 bytes --]

diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index dc481d4d0a5..b7673b01c73 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -555,6 +555,29 @@ widget-specify-active
       (delete-overlay inactive)
       (widget-put widget :inactive nil))))

+(defface widget-unselected
+  '((t :inherit widget-inactive))
+  "Face used for unselected widgets."
+  :group 'widget-faces
+  :version "30.1")
+
+(defun widget-specify-unselected (widget from to)
+  "Fontify WIDGET as unselected (not chosen)."
+  (let ((overlay (make-overlay from to nil t nil)))
+    (overlay-put overlay 'face 'widget-unselected)
+    (overlay-put overlay 'evaporate t)
+    ;; The overlay priority here should be lower than the priority in
+    ;; `widget-specify-active' (bug#69942).
+    (overlay-put overlay 'priority 90)
+    (widget-put widget :unselected overlay)))
+
+(defun widget-specify-selected (widget)
+  "Remove fontification of WIDGET as unselected (not chosen)."
+  (let ((unselected (widget-get widget :unselected)))
+    (when unselected
+      (delete-overlay unselected)
+      (widget-put widget :unselected nil))))
+
 ;;; Widget Properties.

 (defsubst widget-type (widget)
@@ -2439,10 +2462,16 @@ 'checkbox
 (defun widget-checkbox-action (widget &optional event)
   "Toggle checkbox, notify parent, and set active state of sibling."
   (widget-toggle-action widget event)
-  (let ((sibling (widget-get-sibling widget)))
+  (let* ((sibling (widget-get-sibling widget))
+         (from (widget-get sibling :from))
+	 (to (widget-get sibling :to)))
     (when sibling
-      (widget-apply sibling
-	            (if (widget-value widget) :activate :deactivate))
+      (if (widget-value widget)
+          (progn
+            (widget-apply sibling :activate)
+            (widget-specify-selected sibling))
+        :deactivate
+        (widget-specify-unselected sibling from to))
       (widget-clear-undo))))

 ;;; The `checklist' Widget.
@@ -2498,15 +2527,18 @@ widget-checklist-add-item
 	       ((eq escape ?v)
 		(setq child
 		      (cond ((not chosen)
-			     (let ((child (widget-create-child widget type)))
-			       (widget-apply child :deactivate)
+			     (let* ((child (widget-create-child widget type))
+                                    (from (widget-get child :from))
+			            (to (widget-get child :to)))
+                               (widget-specify-unselected child from to)
 			       child))
                             ((widget-inline-p type t)
 			     (widget-create-child-value
 			      widget type (cdr chosen)))
 			    (t
 			     (widget-create-child-value
-			      widget type (car (cdr chosen)))))))
+			      widget type (car (cdr chosen)))
+                             (widget-specify-selected child)))))
 	       (t
 		(error "Unknown escape `%c'" escape)))))
      ;; Update properties.
@@ -2677,8 +2709,11 @@ widget-radio-add-item
 				(widget-create-child-value
 				 widget type value)
 			      (widget-create-child widget type)))
-		(unless chosen
-		  (widget-apply child :deactivate)))
+                (if chosen
+                    (widget-specify-selected child)
+                  (let ((from (widget-get child :from))
+			(to (widget-get child :to)))
+                    (widget-specify-unselected child from to))))
 	       (t
 		(error "Unknown escape `%c'" escape)))))
      ;; Update properties.
@@ -2728,14 +2763,17 @@ widget-radio-value-set
     (dolist (current (widget-get widget :children))
       (let* ((button (widget-get current :button))
 	     (match (and (not found)
-			 (widget-apply current :match value))))
+			 (widget-apply current :match value)))
+             (from (widget-get current :from))
+	     (to (widget-get current :to)))
 	(widget-value-set button match)
 	(if match
-	    (progn
-	      (widget-value-set current value)
-	      (widget-apply current :activate))
-	  (widget-apply current :deactivate))
-	(setq found (or found match))))))
+            (progn
+              (widget-value-set current value)
+              (widget-apply current :activate)
+              (widget-specify-selected current))
+          (widget-specify-unselected current from to))
+        (setq found (or found match))))))

 (defun widget-radio-validate (widget)
   ;; Valid if we have made a valid choice.
@@ -2755,13 +2793,16 @@ widget-radio-action
   (let ((buttons (widget-get widget :buttons)))
     (when (memq child buttons)
       (dolist (current (widget-get widget :children))
-	(let* ((button (widget-get current :button)))
+	(let* ((button (widget-get current :button))
+               (from (widget-get current :from))
+	       (to (widget-get current :to)))
 	  (cond ((eq child button)
 		 (widget-value-set button t)
-		 (widget-apply current :activate))
+		 (widget-apply current :activate)
+                 (widget-specify-selected current))
 		((widget-value button)
 		 (widget-value-set button nil)
-		 (widget-apply current :deactivate)))))))
+                 (widget-specify-unselected current from to)))))))
   ;; Pass notification to parent.
   (widget-apply widget :notify child event))

bug#69942: 30.0.50; Fontification of radio-button widget labels

[Eli Zaretskii]

Ping!  Any further comments, or is there agreement to install the
proposed patch?

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  69942@debbugs.gnu.org
> Date: Fri, 26 Apr 2024 14:47:26 +0200
> 
> On Sun, 21 Apr 2024 21:45:21 +0200 Stephen Berman <stephen.berman@gmx.net> wrote:
> 
> > On Thu, 18 Apr 2024 15:37:58 +0200 Stephen Berman <stephen.berman@gmx.net> wrote:
> >
> >> On Thu, 18 Apr 2024 07:07:43 -0300 Mauro Aranda <maurooaranda@gmail.com> wrote:
> >>
> >>> Eli Zaretskii <eliz@gnu.org> writes:
> >>>
> >>>>> Date: Mon, 8 Apr 2024 07:58:44 -0300
> >>>>> Cc: 69942@debbugs.gnu.org
> >>>>> From: Mauro Aranda <maurooaranda@gmail.com>
> >>>>>
> >>>>> On 6/4/24 06:02, Eli Zaretskii wrote:
> >>>>>  >> From: Stephen Berman <stephen.berman@gmx.net>
> >>>>>  >> Cc: Eli Zaretskii <eliz@gnu.org>, 69942@debbugs.gnu.org
> >>>>>  >> Date: Mon, 01 Apr 2024 17:21:27 +0200
> >>>>>  >>
> >>>>>  >> On Mon, 25 Mar 2024 01:40:36 +0100 Stephen Berman
> >>>>> <stephen.berman@gmx.net> wrote:
> >>>>>  >>
> >>>>>  >>> Ok, I've gotten further with implementing disinguishing by faces
> >>>>>  >>> selected (chosen) and unselected radio buttons in radio-button-choice
> >>>>>  >>> widgets and check boxes in checklist widgets, see the attached patch.
> >>>>>  >>> Initial tests seem ok, but it definitely needs more testing.
> >>>>>  >>
> >>>>>  >> Any comments on this patch for using a widget-unselected face?  I have
> >>>>>  >> been detained from further testing this past week, but can now resume.
> >>>>>  >
> >>>>>  > Mauro, any further comments?
> >>>>>
> >>>>> Hi Eli and Stephen,
> >>>>>
> >>>>> Please forgive me, for the past 2 weeks I haven't been able to do any
> >>>>> computer stuff.  If it's OK, please give me until the weekend so I
> >>>>> can catch up with this and the other 2 bug reports by Stephen.
> >>>>
> >>>> Mauro, were you able to find time to look into this and the other 2
> >>>> bugs?
> >>>
> >>> I have, just now.  The patch looks good to me.  It'll be great if
> >>> Stephen can add some documentation to the manual, so it stays updated.
> >>> If not, I can do that in a few days.
> >>
> >> Sure, I'll add documentation and post it here for approval before
> >> pushing the changes.
> >
> > I've encountered some problems with the patch.  One is that it breaks
> > the display of all face attributes in the customize-face buffer.  I've
> > determined the part of the patch that triggers this, though I haven't
> > yet figured out just why this bit of code breaks the display.  Also, it
> > appears that the widget-unselected face does not completely replace
> > widget-inactive where it's intended to do so, but I need to do more
> > testing and debugging here to find out why.  Until I've fixed these
> > issues the patch is not suitable for installing, so I'm also holding off
> > with the accompanying documentation.  (But in preparation for the
> > documentation I looked more closely at the Widget manual and found
> > several typos and other issues, for which I opened bug#70502.)
> 
> The breakage in displaying all face attributes in the customize-face
> buffer was caused by the invocation of `(widget-specify-selected child)'
> in widget-checklist-add-item in the cond-clause satisfying
> `(widget-inline-p type t)'.  I still don't understand why it has this
> effect, and I have to admit that I don't understand what an inline
> widget is.  But simply omitting the invocation of
> `(widget-specify-selected child)' at this point in the patch does avoid
> the customize-face breakage and I have not noticed any problems due to
> this omission.
> 
> As for the problematic interaction between the widget-unselected and
> widget-inactive faces, this seems to have been due to my having copied
> most of the definition of widget-specify-unselected from that of
> widget-specify-inactive, and specifically, copying the overlay priority.
> In the attached patch, widget-specify-unselected now uses a lower
> overlay priority than the one used in widget-specify-inactive, and in my
> tests this yields the desired results: the labels of active checkboxes
> and radio-buttons have widget-unselected face but when these widgets are
> deactivated, the labels have widget-inactive face.
> 
> Another change I've made in the attached patch is to have the default
> value of widget-unselected face inherit from widget-inactive instead of
> inheriting from shadow face, like widget-inactive does by default.  This
> way, if a user customizes widget-inactive, that will also apply by
> default to widget-unselected, thus retaining the current default widget
> UI where the labels of checkboxes and radio-buttons have widget-inactive
> face.  (Thus, the "desired results" in the preceding paragraph are only
> visible when widget-unselected face is customized to differ from
> widget-unselected face.)
> 
> Finally, regarding documentation of widget-unselected face in the Widget
> manual, I think it would be helpful for the documentation to mention the
> use case that motivated introducing it (following Mauro's suggestion to
> use a face instead of a defcustom), namely, to visually distinguish the
> labels of unselected and inactive widgets.  Here is what I suggest:
> 
> @deffn Face widget-unselected
> Face used for unselected widgets.  This face is also used on the text
> labels of radio-button and checkbox widgets.
> 
> The default value inherits @code{widget-inactive} face.  If you want to
> visually distinguish the labels of unselected active radio-button or
> checkbox widgets from the labels of unselected inactive widgets,
> customize this face to a non-default value.
> @end deffn
> 
> Since the recent widget.texi changes that include documenting
> widget-inactive face have not yet been merged to master, I haven't
> included the propopsed documentaton of widget-unselected in the attached
> patch, but if approved, will of course add it after the merge.
> 
> Steve Berman