Re: [Emacs-diffs] master b75fb81 1/4: Extend button.el to take callback data

[Basil L. Contovounesios]

Lars Ingebrigtsen <address@hidden> writes:

> "Basil L. Contovounesios" <address@hidden> writes:
>
>> I don't understand what you mean by "recreate the data" or "looking at
>> the extent of the buttons".
>>
>> If an action function depends on some data associated with its button,
>> then it is up to the creator or modifier of the button to tag it with
>> that data.  The action function then need only do a property lookup via
>> button-get. 
>
> Here's a typical usage:
>
>       (make-text-button start (point)
>                       'face 'rcirc-url
>                       'follow-link t
>                       'rcirc-url url
>                       'action (lambda (button)
>                                 (browse-url (button-get button 'rcirc-url))))
>
> with the "button knows the data" change, it's:
>
>       (make-text-button start (point)
>                       'face 'rcirc-url
>                       'follow-link t
>                       'burron-data url
>                       'action #'browse-url)
>
> That looks like an interface improvement to me.

I'm arguing that something like the following is a better interface
improvement, because it doesn't require changing the interface:

  (defun button-callback-action (button)
    "Call BUTTON's `button-callback' property."
    (apply #'funcall (button-get button 'button-callback)))

  (define-button-type 'rcirc-url
    'face 'rcirc-url
    'follow-link t
    'action #'button-callback-action)

  (make-text-button start (point)
                    'type 'rcirc-url
                    'button-callback (list #'browse-url url))

(Using define-button-type is optional, but encouraged.)

Providing such a convenience function in button.el (or one of its client
libraries) affords the same brevity as the button-data approach, without
changing the button API, making existing buttons more brittle, or
sacrificing generality.

Button action functions have until now expected a button as an argument,
but now they may or may not be given a button, and this is determined by
the presence or not of a button property.  Why introduce this brittle
backward incompatibility?

>> Alternatively, action functions can also be closures.
>
> They can be, in lexical packages.

Sure; I only mentioned closures for completeness, not as a total
solution.

>> I don't mind the name 'button-data', and I wasn't worried about naming
>> collisions.
>>
>> What I'm worried about is the existence of buttons in the wild, whose
>> existing action functions will break if said buttons happen to be given
>> a button-data property.  This seems unnecessarily brittle and
>> backward-incompatible, in exchange for what seems like an insufficiently
>> useful convenience.  Unless I'm missing something, that is.
>
> But you said you're not worried about naming collisions?   How would
> these buttons then "happen to be given" that property?

Oh, we seem to be interpreting "naming collision" slightly differently.

I took it to mean that a client of the button library has used a
property name for one purpose, which is used for another purpose
elsewhere.

But what is happening here is that existing code is written with the
assumption that button action functions receive a button as argument.
This assumption now breaks down on buttons with a non-nil button-data
property.  Why make this unnecessary and backward-incompatible change?

Thanks,

-- 
Basil