From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: "Basil L. Contovounesios" Newsgroups: gmane.emacs.devel Subject: Re: [Emacs-diffs] master b75fb81 1/4: Extend button.el to take callback data Date: Thu, 01 Aug 2019 18:19:04 +0300 Message-ID: <87ef24vrpz.fsf@tcd.ie> References: <20190730132507.32385.70681@vcs0.savannah.gnu.org> <20190730132509.77D2820C0A@vcs0.savannah.gnu.org> <87lfwd2ihn.fsf@tcd.ie> <87pnlpm5x9.fsf@mouse.gnus.org> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="77450"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) Cc: emacs-devel@gnu.org To: Lars Ingebrigtsen Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Thu Aug 01 17:27:18 2019 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1htCz8-000K2S-61 for ged-emacs-devel@m.gmane.org; Thu, 01 Aug 2019 17:27:18 +0200 Original-Received: from localhost ([::1]:56750 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1htCz6-0002gF-Lp for ged-emacs-devel@m.gmane.org; Thu, 01 Aug 2019 11:27:16 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:36362) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1htCrR-0006jH-MH for emacs-devel@gnu.org; Thu, 01 Aug 2019 11:19:23 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1htCrO-0001VD-Da for emacs-devel@gnu.org; Thu, 01 Aug 2019 11:19:19 -0400 Original-Received: from mail-wr1-x42f.google.com ([2a00:1450:4864:20::42f]:35127) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1htCrK-0001TM-NJ for emacs-devel@gnu.org; Thu, 01 Aug 2019 11:19:16 -0400 Original-Received: by mail-wr1-x42f.google.com with SMTP id y4so74030876wrm.2 for ; Thu, 01 Aug 2019 08:19:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tcd-ie.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=EMRka61LMTOHCrJP06wx4wzOLPbBexPb7o/pvA2ahXQ=; b=Sz6C/we/6uUXkViJ9AilfSIDpuYR7cl5qLhcGZ3BMoNOEoQZNmJv6XzfRgIIRzqD/n 9YGWvBu29BE34ID5bJkSYxdL+ld8mdUKoA9NZFvBn79aZE9tP9rmI8VK9vAM8wf8WAsa zk8QriAJDb8LziN0y4D8pkb2jamBeM8YRQXnKOtLm6do2S6isMap21pVkS3vtylfWIyG W0lFwENWvZs1WYhxSMgy2qtS8NtR1fkoaMi6kqyOv4qeK918RCeFUYEN/k3hiHWpe93j lvyMxHa1gI70x/iuUw/Xux5tjpnVkNc2BW/MLBSwlcBSrFVbom/ASV8Vl1NqR9dcNdJ4 8tLA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=EMRka61LMTOHCrJP06wx4wzOLPbBexPb7o/pvA2ahXQ=; b=rZa/0CRMLPTMLKTzksz5gwkWTWvHzE82VPo47LnupQJz1iaBrMHCDdhoRPa1SxoqNu 2YKU/55wZr2YuDXmE+0SZBXTf/+t/kYMLDVnSRrXDDEWRSkLUwxXM4TdTgd+nzHA/e1P BgRuZRnmidvOWUlMri6ejoyRykGypXD5XsapOxcari9F/GcOSDSZYWRrZJKmkb86hDj8 n7YJSqag+WVfGq3eS8qY9kyaW3vFa6KABIR+KdOrIty02R+3uFsDSyxZHv2zIIcU6AMD qFKPvZBx1zhxj5aCg5FnKCmRa5d5rYZZ14ZKcd8HVFW9fs0H2dS2DikXPF/e7PYKG6a+ wVVg== X-Gm-Message-State: APjAAAX+OoHiqP2Ox2oHJvwXD1nU1ePjv66bVp0c+kqECLgCdHciBdDl Sa5lV7UZlRyLPIVzO3Mahb+FXQ== X-Google-Smtp-Source: APXvYqyz/KmAOpkugtqGfg/pwyXD0BGkQCcZeilCwKOdQoEUeumYOeqavDfGUQnCt7w5DGg9+M+Q+A== X-Received: by 2002:a5d:4950:: with SMTP id r16mr38598723wrs.136.1564672753080; Thu, 01 Aug 2019 08:19:13 -0700 (PDT) Original-Received: from localhost (adsl-131.91.140.89.tellas.gr. [91.140.89.131]) by smtp.gmail.com with ESMTPSA id c1sm163592259wrh.1.2019.08.01.08.19.11 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Thu, 01 Aug 2019 08:19:11 -0700 (PDT) In-Reply-To: <87pnlpm5x9.fsf@mouse.gnus.org> (Lars Ingebrigtsen's message of "Thu, 01 Aug 2019 14:22:26 +0200") X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2a00:1450:4864:20::42f 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.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:239107 Archived-At: Lars Ingebrigtsen writes: > "Basil L. Contovounesios" writes: > >> Can you please explain the rationale behind this? I'm not sure it's a >> good idea: >> >> 1. Arbitrary data can already be associated with buttons and retrieved >> in their action functions using button-get, button-put, and on button >> creation with make{,-text}-button et al. >> >> 2. AIUI this is a potentially backward-incompatible change, as existing >> action functions, which expect to receive a button, may now receive >> the value of the button-data property instead. >> >> So at first glance I'm not convinced the minor convenience of not having >> to manually (button-get button 'button-data) or similar in the action >> function is worth messing with the pre-existent and cleaner API. Am I >> missing something? > > Button callback functions currently often recreate the data they need by > looking at the extent of the buttons. 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. Alternatively, action functions can also be closures. > This is an awkward interface, Why? > because when creating the buttons, the functions have already > determined what the data should be. Shouldn't they store that data as a property of the button, or in a closure, then? > So it'd a way to de-duplicate code. Can you please give me an example of such duplication? > If you look at the functions in Gnus that use this now, you can see > how the callback functions can be completely oblivious to having be > called through a button.el callback, which is how it should be. Which functions are those? The ones that call gnus-article-add-button? If so, gnus-article-add-button should specify an adapter function as the button's action, rather than an arbitrary non-button-related function. I don't think it's good to change the button API in a backward-incompatible way just to allow specifying e.g. browse-url as a button action function: browse-url is inherently button-unaware, and should be wrapped for use with buttons. > As for the name collision issue -- that's why I didn't call it just > `data' or the like. button.el already uses property names like > `action', which is unfortunate, as that really is prone to naming > collisions, but I grepped through the tree, and there are no in-tree > usages of the `button-data' property. 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. For example, existing code like the following: (defun my-action (button) (browse-url (button-get button 'shr-url))) (define-button-type 'my-type 'action #'my-action) will now break if the button happens to be given a button-data property. Am I the only one who thinks this is a problem, and not as elegant or as flexible as the pre-existent API? Thanks, -- Basil