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

[Basil L. Contovounesios]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>        (make-text-button name nil
>>                          'type 'apropos-library
>>                          'face 'apropos-symbol
>> -                        'apropos-symbol name)
>> -      name)))
>> +                        'apropos-symbol name))))
>
> So, IIUC we consider `make-text-button` to be functional rather than
> side-effecting, right?

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.

>> @@ -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.

-- 
Basil