Subject: bug#61413: [PATCH] Make warnings show a "warning" emoji instead of a stop-sign

From: Kévin Le Gouguec <[email-masked]>
Date: Sat, 22 Feb 2025 19:34:12 +0100

Mauro Aranda <[email-masked]> writes:

> Kévin Le Gouguec <[email-masked]> writes:
>
>> Mauro Aranda <[email-masked]> writes:
>>
>>> But, now we don't know if the help-echo property was effectively added
>>> by the library or it was already there :-(.  Sadness.
>>
>> Would the attached address that problem?  (To apply on top of the
>> previous patch; I'll squash it all if we are happy with it)
>
> Looks good to me, thanks.

Alright, here's the squashed patch then - with amended ChangeLog.  Let
me know whether it looks OK to install.

Back to the topic of the warnings-suppress icon representations: FWIW, I
toyed with status/dialog-error-symbolic.svg from adwaita-icon-theme, but
could not land on a result I found perfectly harmonious.

* Edited the SVG to remove the foreground color, as we usually do, so
that faces apply to the inserted image,
* but warnings-suppress inherits from the 'button icon, which means the
image gets the 'icon-face; the result looks messy IMO,
* inheriting from nil instead, buttonize's 'button face takes over,
which is somewhat confusing too,
* adding ':face default' to the image spec makes the result less jarring
(to me).

Attaching that experiment as well FTR; not wholly pleased with it
though.

(I wonder whether this part of the 'button icon…

  (define-icon button nil
    '((image :face icon-button) ; 👈 that one
      ; …
      )
    "Base icon for buttons."
    :version "29.1")

… is desirable.  AFAICT _all_ our in-tree icons inherit from nil instead
of 'button' despite their "button-like" function; wondering to what
extent they are opting out of this ':face icon-button'?)

>From 5668adb52c0f97b38cf47cdab77b8ffb4e923706 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <[email-masked]>
Date: Thu, 20 Feb 2025 22:37:13 +0100
Subject: [PATCH] Prevent button.el from clearing help-echo strings

In order to fix one of the issues discussed in bug#61413, i.e.
'buttonize' clobbering the help-echo property set by
'icon-string'.

This is a reasonable interpretation of the button.el
docstrings - "if HELP-ECHO, use that as the `help-echo'
property"; conversely, if not HELP-ECHO, then do not do
anything, preserving existing values for that property.

* lisp/button.el (button--properties): Only add a help-echo
property if HELP-ECHO is non-nil.  Add an additional property
for bookkeeping.
(unbuttonize-region): Check for that bookkeeping property
before clearing help-echo.
* test/lisp/button-tests.el (button--preserve-help-echo):
Validate these changes.
---
 lisp/button.el            | 32 +++++++++++++++++++++-----------
 test/lisp/button-tests.el | 28 ++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/lisp/button.el b/lisp/button.el
index 674de1bb4fa..58f00c4c2ad 100644
--- a/lisp/button.el
+++ b/lisp/button.el
@@ -652,15 +652,19 @@ buttonize
     string))
 
 (defun button--properties (callback data help-echo)
-  (list 'font-lock-face 'button
-        'mouse-face 'highlight
-        'help-echo help-echo
-        'button t
-        'follow-link t
-        'category t
-        'button-data data
-        'keymap button-map
-        'action callback))
+  (append
+   (list 'font-lock-face 'button
+         'mouse-face 'highlight
+         'button t
+         'follow-link t
+         'category t
+         'button-data data
+         'keymap button-map
+         'action callback)
+   (and help-echo
+        (list 'help-echo help-echo
+              ;; Record that button.el is responsible for this property.
+              'help-echo-button t))))
 
 (defun buttonize-region (start end callback &optional data help-echo)
   "Make the region between START and END into a button.
@@ -681,8 +685,14 @@ unbuttonize-region
     (when (overlay-get o 'button)
       (delete-overlay o)))
   (with-silent-modifications
-    (remove-text-properties start end
-                            (button--properties nil nil nil))
+    (remove-text-properties
+     start end
+     (append
+      (button--properties nil nil nil)
+      ;; Only remove help-echo if it was added by button.el.
+      (and (get-text-property start 'help-echo-button)
+           (list 'help-echo nil
+                 'help-echo-button nil))))
     (add-face-text-property start end
                             'button nil)))
 
diff --git a/test/lisp/button-tests.el b/test/lisp/button-tests.el
index 7f6a5bd52cd..b784cf02e28 100644
--- a/test/lisp/button-tests.el
+++ b/test/lisp/button-tests.el
@@ -101,4 +101,32 @@ button--help-echo-function
       (setq button (insert-button "overlay" 'help-echo help))
       (should (equal (button--help-echo button) "overlay: x")))))
 
+(ert-deftest button--preserve-help-echo ()
+  "Ensure buttonizing functions preserve existing `help-echo' properties."
+  ;; buttonize.
+  (let* ((string (propertize "button text" 'help-echo "help text"))
+         (button (buttonize string #'ignore)))
+    (should (equal (get-text-property 0 'help-echo button)
+                   "help text")))
+  ;; buttonize-region.
+  (with-temp-buffer
+    (insert (propertize "button text" 'help-echo "help text"))
+    (buttonize-region (point-min) (point) #'ignore)
+    (should (equal (get-text-property (point-min) 'help-echo)
+                   "help text"))
+    ;; unbuttonize-region should not clear the property either.
+    (unbuttonize-region (point-min) (point))
+    (should (equal (get-text-property (point-min) 'help-echo)
+                   "help text")))
+  ;; unbuttonize-region should still clear properties applied with
+  ;; buttonize.
+  (with-temp-buffer
+    (insert "button text")
+    (buttonize-region (point-min) (point) #'ignore nil "help text")
+    (should (equal (get-text-property (point-min) 'help-echo)
+                   "help text"))
+    (unbuttonize-region (point-min) (point))
+    (should (equal (get-text-property (point-min) 'help-echo)
+                   nil))))
+
 ;;; button-tests.el ends here
-- 
2.48.1

diff --git a/etc/images/README b/etc/images/README
index d3f442a3b24..4c85ee182ca 100644
--- a/etc/images/README
+++ b/etc/images/README
@@ -137,8 +137,9 @@ Emacs images and their source in the Adwaita/symbolic directory:
   left.svg                  ui/pan-start-symbolic.svg
   right.svg                 ui/pan-end-symbolic.svg
   up.svg                    ui/pan-up-symbolic.svg
-  conceal.svg               actions/view-conceal-symbolic.svg
-  reveal.svg                actions/view-reveal-symbolic.svg
+  conceal.svg(*)            actions/view-conceal-symbolic.svg
+  reveal.svg(*)             actions/view-reveal-symbolic.svg
+  no-entry.svg(*)           status/dialog-error-symbolic.svg
 
-conceal.pbm and reveal.pbm are generated from the respective *.svg
-files, using the ImageMagick converter tool.
+  (*) *.pbm files were generated from the respective *.svg files, using
+  the ImageMagick converter tool.
diff --git a/etc/images/no-entry.pbm b/etc/images/no-entry.pbm
new file mode 100644
index 0000000000000000000000000000000000000000..830d2d3e5c63ad788586dc4dd26e299395673625
GIT binary patch
literal 41
vcmWGA;W9K+Ff`+0U|@g1|3UtT{h#`O_5TX^3iv>@{U7-s{2$mKFfafBGcOS5

literal 0
HcmV?d00001

diff --git a/etc/images/no-entry.svg b/etc/images/no-entry.svg
new file mode 100644
index 00000000000..8ec31601a8f
--- /dev/null
+++ b/etc/images/no-entry.svg
@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<svg height="16px" viewBox="0 0 16 16" width="16px" xmlns="http://www.w3.org/2000/svg">
+    <path d="m 8 1 c -3.871094 0 -7 3.128906 -7 7 s 3.128906 7 7 7 s 7 -3.128906 7 -7 s -3.128906 -7 -7 -7 z m -4 6 h 8 v 2 h -8 z m 0 0"/>
+</svg>
diff --git a/lisp/emacs-lisp/warnings.el b/lisp/emacs-lisp/warnings.el
index 8a5c73ebd3a..64f7422c928 100644
--- a/lisp/emacs-lisp/warnings.el
+++ b/lisp/emacs-lisp/warnings.el
@@ -213,7 +213,12 @@ warning-suppress-p
     some-match))
 
 (define-icon warnings-suppress button
-  `((emoji "⛔")
+  `(
+    ;; The icon-button face looks confusing on images; set a more
+    ;; neutral face to get an effect similar to other button-like icons
+    ;; (Custom, outline).
+    (image "no-entry.svg" "no-entry.pbm" :face default)
+    (emoji "⛔")
     ;; Many MS-Windows console fonts don't have good glyphs for U+25A0.
     (symbol ,(if (and (eq system-type 'windows-nt)
                       (null window-system))