[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Stefan Monnier
Subject: Re: master f51f963: Fix some side-effecting uses of make-text-button
Date: Fri, 05 Jun 2020 09:02:54 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux)

> As Paul said, not quite - it has historically modified its first
> argument by placing properties on it.  If you look at the line preceding
> this diff hunk you'll see a call to copy-sequence.
> My reasons for making this particular change are:
> 0. Since Emacs 24.4, make-text-button has returned its modified first
>    argument, so callers can use the value returned by make-text-button
>    instead of calling it only for its side effects.
> 1. This has now been reverted again, but for the last month,
>    make-text-button returned a modified _copy_ of its argument, which
>    meant that its side effects could no longer be relied upon.
> Either way, relying on its return value rather than its side effects
> seems like the best style to stick with for now.

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?

If we could detect when a string-button is "used" (i.e. displayed or
inserted into a buffer), then we could detect the use of the old
side-effecting style (by checking if the string passed as argument had
already been displayed/inserted elsewhere) and emit a good warning.

>>> @@ -202,7 +202,7 @@ The format has been repaired and the variable modified 
>>> accordingly.
>>>  You can save the current value through the customize system by
>>>  either clicking or hitting return "
>>>            (make-text-button
>>> -           "here" nil
>>> +           (copy-sequence "here") nil
>>>             'face '(:weight bold :inherit button)
>>>             'mouse-face '(:weight normal :background "gray50" :inherit 
>>> button)
>>>             'follow-link t
>> So, here why do we need to `copy-sequence`?
> To avoid destructively modifying a string literal by placing properties
> on it.

Makes sense,


reply via email to

[Prev in Thread] Current Thread [Next in Thread]