Re: master f51f963: Fix some side-effecting uses of make-text-button

[Paul Eggert <eggert@cs.ucla.edu>]

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

On 6/4/20 3:44 PM, Stefan Monnier wrote:
> So, IIUC we consider `make-text-button` to be functional rather than
> side-effecting, right?

It *ought* to be functional but it's currently not, because make-text-button has
a side effect on its argument string.

How about the attached patch? This would mean SLY would need the patch I sent in
earlier[1] since the attached patch is not 100% compatible with existing Emacs;
however, it does make make-text-button more functional and that's a good thing.

[1] https://lists.gnu.org/r/emacs-devel/2020-06/msg00152.html

[-- Attachment #2: make-text-button.diff --]
[-- Type: text/x-patch, Size: 2549 bytes --]

diff --git a/etc/NEWS b/etc/NEWS
index ed4722b27f..5479831448 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -471,6 +471,11 @@ are 'eq'.  To compare contents, use 'compare-window-configurations'
 instead.  This change helps fix a bug in 'sxhash-equal', which returned
 incorrect hashes for window configurations and some other objects.
 
+** When its first argument is a string, 'make-text-button' no longer
+modifies the string's text properties; instead, it uses and returns a
+copy of the string.  This helps avoid trouble when strings are shared
+or constants.
+
 ---
 ** The obsolete function 'thread-alive-p' has been removed.
 
diff --git a/lisp/apropos.el b/lisp/apropos.el
index 22866cd2cc..2566d44dfc 100644
--- a/lisp/apropos.el
+++ b/lisp/apropos.el
@@ -661,7 +661,7 @@ apropos
 (defun apropos-library-button (sym)
   (if (null sym)
       "<nothing>"
-    (let ((name (copy-sequence (symbol-name sym))))
+    (let ((name (symbol-name sym)))
       (make-text-button name nil
                         'type 'apropos-library
                         'face 'apropos-symbol
diff --git a/lisp/button.el b/lisp/button.el
index 3a6a6de774..76b0e9102f 100644
--- a/lisp/button.el
+++ b/lisp/button.el
@@ -341,7 +341,7 @@ make-text-button
 as the argument for the `action' callback function instead of the
 default argument, which is the button itself.
 
-BEG can also be a string, in which case it is made into a button.
+BEG can also be a string, in which case a copy of it is made into a button.
 
 Also see `insert-text-button'."
   (let ((object nil)
@@ -349,7 +349,9 @@ make-text-button
 	 (or (plist-member properties 'type)
 	     (plist-member properties :type))))
     (when (stringp beg)
-      (setq object beg beg 0 end (length object)))
+      (setq object (copy-sequence beg))
+      (setq beg 0)
+      (setq end (length object)))
     ;; Disallow setting the `category' property directly.
     (when (plist-get properties 'category)
       (error "Button `category' property may not be set directly"))
diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el
index c39000b488..bfb9787a96 100644
--- a/lisp/ibuf-ext.el
+++ b/lisp/ibuf-ext.el
@@ -202,7 +202,7 @@ ibuffer-old-saved-filters-warning
 You can save the current value through the customize system by
 either clicking or hitting return "
           (make-text-button
-           (copy-sequence "here") nil
+           "here" nil
            'face '(:weight bold :inherit button)
            'mouse-face '(:weight normal :background "gray50" :inherit button)
            'follow-link t