all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: "Basil L. Contovounesios" <contovob@tcd.ie>
To: Lars Ingebrigtsen <larsi@gnus.org>
Cc: emacs-devel@gnu.org
Subject: Re: [Emacs-diffs] master b75fb81 1/4: Extend button.el to take callback data
Date: Fri, 02 Aug 2019 03:40:09 +0300	[thread overview]
Message-ID: <878ssciemu.fsf@tcd.ie> (raw)
In-Reply-To: <877e7wlv98.fsf@mouse.gnus.org> (Lars Ingebrigtsen's message of "Thu, 01 Aug 2019 18:12:51 +0200")

Lars Ingebrigtsen <larsi@gnus.org> writes:

> "Basil L. Contovounesios" <contovob@tcd.ie> 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



  parent reply	other threads:[~2019-08-02  0:40 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20190730132507.32385.70681@vcs0.savannah.gnu.org>
     [not found] ` <20190730132509.77D2820C0A@vcs0.savannah.gnu.org>
2019-08-01 12:11   ` [Emacs-diffs] master b75fb81 1/4: Extend button.el to take callback data Basil L. Contovounesios
2019-08-01 12:22     ` Lars Ingebrigtsen
2019-08-01 15:19       ` Basil L. Contovounesios
2019-08-01 16:12         ` Lars Ingebrigtsen
2019-08-01 20:44           ` Stefan Monnier
2019-08-02 18:41             ` Lars Ingebrigtsen
2019-08-02  0:40           ` Basil L. Contovounesios [this message]
2019-08-02 18:46             ` Lars Ingebrigtsen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=878ssciemu.fsf@tcd.ie \
    --to=contovob@tcd.ie \
    --cc=emacs-devel@gnu.org \
    --cc=larsi@gnus.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.