From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.ciao.gmane.io!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.devel Subject: Re: master f51f963: Fix some side-effecting uses of make-text-button Date: Sat, 6 Jun 2020 12:09:04 -0700 Organization: UCLA Computer Science Department Message-ID: <3325df83-4840-c15d-3f38-372628a54536@cs.ucla.edu> References: <20200604223056.17078.81265@vcs0.savannah.gnu.org> <20200604223058.1850020A26@vcs0.savannah.gnu.org> <87eeqtiy4x.fsf@tcd.ie> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------844231CD102D2FF8DE3B9BA2" Injection-Info: ciao.gmane.io; posting-host="ciao.gmane.io:159.69.161.202"; logging-data="86160"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 Cc: emacs-devel@gnu.org To: Stefan Monnier , "Basil L. Contovounesios" Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sat Jun 06 21:12:08 2020 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jheEi-000MOH-9z for ged-emacs-devel@m.gmane-mx.org; Sat, 06 Jun 2020 21:12:08 +0200 Original-Received: from localhost ([::1]:48880 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jheEh-000770-Cw for ged-emacs-devel@m.gmane-mx.org; Sat, 06 Jun 2020 15:12:07 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:37016) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jheBr-0004y9-2T for emacs-devel@gnu.org; Sat, 06 Jun 2020 15:09:11 -0400 Original-Received: from zimbra.cs.ucla.edu ([131.179.128.68]:42214) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jheBp-0002oV-QJ for emacs-devel@gnu.org; Sat, 06 Jun 2020 15:09:10 -0400 Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id C45F71600E0; Sat, 6 Jun 2020 12:09:07 -0700 (PDT) Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id KLFQSNSgs21n; Sat, 6 Jun 2020 12:09:06 -0700 (PDT) Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id A37181600E1; Sat, 6 Jun 2020 12:09:06 -0700 (PDT) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id XMPl-7Dv-kEp; Sat, 6 Jun 2020 12:09:06 -0700 (PDT) Original-Received: from [192.168.1.9] (cpe-23-242-74-103.socal.res.rr.com [23.242.74.103]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id 5055D1600E0; Sat, 6 Jun 2020 12:09:06 -0700 (PDT) Autocrypt: addr=eggert@cs.ucla.edu; prefer-encrypt=mutual; keydata= LS0tLS1CRUdJTiBQR1AgUFVCTElDIEtFWSBCTE9DSy0tLS0tCgptUUlOQkV5QWNtUUJFQURB QXlIMnhvVHU3cHBHNUQzYThGTVpFb243NGRDdmM0K3ExWEEySjJ0QnkycHdhVHFmCmhweHhk R0E5Smo1MFVKM1BENGJTVUVnTjh0TFowc2FuNDdsNVhUQUZMaTI0NTZjaVNsNW04c0thSGxH ZHQ5WG0KQUF0bVhxZVpWSVlYL1VGUzk2ZkR6ZjR4aEVtbS95N0xiWUVQUWRVZHh1NDd4QTVL aFRZcDVibHRGM1dZRHoxWQpnZDdneDA3QXV3cDdpdzdlTnZub0RUQWxLQWw4S1lEWnpiRE5D UUdFYnBZM2VmWkl2UGRlSStGV1FONFcra2doCnkrUDZhdTZQcklJaFlyYWV1YTdYRGRiMkxT MWVuM1NzbUUzUWpxZlJxSS9BMnVlOEpNd3N2WGUvV0szOEV6czYKeDc0aVRhcUkzQUZINmls QWhEcXBNbmQvbXNTRVNORnQ3NkRpTzFaS1FNcjlhbVZQa25qZlBtSklTcWRoZ0IxRApsRWR3 MzRzUk9mNlY4bVp3MHhmcVQ2UEtFNDZMY0ZlZnpzMGtiZzRHT1JmOHZqRzJTZjF0azVlVThN Qml5Ti9iClowM2JLTmpOWU1wT0REUVF3dVA4NGtZTGtYMndCeHhNQWhCeHdiRFZadWR6eERa SjFDMlZYdWpDT0pWeHEya2wKakJNOUVUWXVVR3FkNzVBVzJMWHJMdzYrTXVJc0hGQVlBZ1Jy NytLY3dEZ0JBZndoU In-Reply-To: Content-Language: en-US Received-SPF: pass client-ip=131.179.128.68; envelope-from=eggert@cs.ucla.edu; helo=zimbra.cs.ucla.edu X-detected-operating-system: by eggs.gnu.org: First seen = 2020/06/06 13:54:51 X-ACL-Warn: Detected OS = Linux 3.1-3.10 X-Spam_score_int: -41 X-Spam_score: -4.2 X-Spam_bar: ---- X-Spam_report: (-4.2 / 5.0 requ) BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3, SPF_PASS=-0.001, URIBL_BLOCKED=0.001 autolearn=_AUTOLEARN X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:251961 Archived-At: This is a multi-part message in MIME format. --------------844231CD102D2FF8DE3B9BA2 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 6/5/20 6:02 AM, Stefan Monnier wrote: > So, IIUC `make-text-button` should ideally work functionally, but for > historical reasons it works by side-effect. What's the long term plan? > Do we plan to live with the current side-effecting behavior, or do we > plan to move to the "pure" functional behavior? Given the comments in this thread it seems that there's consensus that it should move to the "pure" functional behavior, as the side-effecting behavior is confusing (and this is independent of whether string literals are constant). I installed the attached patch, which is along the lines that I proposed a couple of days ago; it has the doc fix that Pip Cet suggested. --------------844231CD102D2FF8DE3B9BA2 Content-Type: text/x-patch; charset=UTF-8; name="0001-make-text-button-no-longer-modifies-its-string-arg.patch" Content-Disposition: attachment; filename*0="0001-make-text-button-no-longer-modifies-its-string-arg.patc"; filename*1="h" Content-Transfer-Encoding: quoted-printable >From 481e57cd310177452fe5d4e733370337eb35ba96 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sat, 6 Jun 2020 12:05:10 -0700 Subject: [PATCH] make-text-button no longer modifies its string arg MIME-Version: 1.0 Content-Type: text/plain; charset=3DUTF-8 Content-Transfer-Encoding: 8bit * etc/NEWS: Mention this. * lisp/apropos.el (apropos-library-button): * lisp/ibuf-ext.el (ibuffer-old-saved-filters-warning): There=E2=80=99s no longer a need copy make-text-button=E2=80=99s string a= rg. * lisp/button.el (make-text-button): Return a copy of a string arg. Delay making the copy until after error-checking. --- etc/NEWS | 5 +++++ lisp/apropos.el | 2 +- lisp/button.el | 9 ++++++--- lisp/ibuf-ext.el | 2 +- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/etc/NEWS b/etc/NEWS index 27e511047e..edad5b37d6 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -476,6 +476,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. =20 +** 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. =20 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) "" - (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..d9c36a0375 100644 --- a/lisp/button.el +++ b/lisp/button.el @@ -341,15 +341,14 @@ make-text-button as the argument for the `action' callback function instead of the default argument, which is the button itself. =20 -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 and returned. =20 Also see `insert-text-button'." (let ((object nil) (type-entry (or (plist-member properties 'type) (plist-member properties :type)))) - (when (stringp beg) - (setq object beg beg 0 end (length object))) ;; Disallow setting the `category' property directly. (when (plist-get properties 'category) (error "Button `category' property may not be set directly")) @@ -362,6 +361,10 @@ make-text-button (setcar type-entry 'category) (setcar (cdr type-entry) (button-category-symbol (cadr type-entry)))) + (when (stringp beg) + (setq object (copy-sequence beg)) + (setq beg 0) + (setq end (length object))) ;; Now add all the text properties at once. (add-text-properties beg end ;; Each button should have a non-eq `button' 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 bu= tton) 'follow-link t --=20 2.17.1 --------------844231CD102D2FF8DE3B9BA2--