From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Juri Linkov Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] tab-bar.el: add defcustoms for functions to call after opening and before closing Date: Thu, 05 Dec 2019 00:45:20 +0200 Organization: LINKOV.NET Message-ID: <87lfrrn20p.fsf@mail.linkov.net> References: <87h83egmzy.fsf@cochranmail.com> <87r22fzhag.fsf@mail.linkov.net> <87zhgawdk9.fsf@cochranmail.com> <87o8wpm15x.fsf@mail.linkov.net> <87immvzvf8.fsf@cochranmail.com> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="269169"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (x86_64-pc-linux-gnu) Cc: emacs-devel@gnu.org To: Robert Cochran Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Thu Dec 05 00:32:32 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 1ice8E-0017rP-9l for ged-emacs-devel@m.gmane.org; Thu, 05 Dec 2019 00:32:30 +0100 Original-Received: from localhost ([::1]:48084 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ice8C-0004Ar-MN for ged-emacs-devel@m.gmane.org; Wed, 04 Dec 2019 18:32:28 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:53678) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ice3N-000222-3G for emacs-devel@gnu.org; Wed, 04 Dec 2019 18:27:30 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ice3L-0002nW-8j for emacs-devel@gnu.org; Wed, 04 Dec 2019 18:27:28 -0500 Original-Received: from crocodile.birch.relay.mailchannels.net ([23.83.209.45]:13013) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ice3K-0002ki-Rh for emacs-devel@gnu.org; Wed, 04 Dec 2019 18:27:27 -0500 X-Sender-Id: dreamhost|x-authsender|jurta@jurta.org Original-Received: from relay.mailchannels.net (localhost [127.0.0.1]) by relay.mailchannels.net (Postfix) with ESMTP id 77F68E0F24; Wed, 4 Dec 2019 23:27:25 +0000 (UTC) Original-Received: from pdx1-sub0-mail-a19.g.dreamhost.com (100-96-92-172.trex.outbound.svc.cluster.local [100.96.92.172]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id F0D6CE0F04; Wed, 4 Dec 2019 23:27:24 +0000 (UTC) X-Sender-Id: dreamhost|x-authsender|jurta@jurta.org Original-Received: from pdx1-sub0-mail-a19.g.dreamhost.com ([TEMPUNAVAIL]. [64.90.62.162]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384) by 0.0.0.0:2500 (trex/5.18.5); Wed, 04 Dec 2019 23:27:25 +0000 X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|jurta@jurta.org X-MailChannels-Auth-Id: dreamhost X-Little-Wide-Eyed: 1c18afd8152d4457_1575502045215_281540657 X-MC-Loop-Signature: 1575502045215:871378453 X-MC-Ingress-Time: 1575502045215 Original-Received: from pdx1-sub0-mail-a19.g.dreamhost.com (localhost [127.0.0.1]) by pdx1-sub0-mail-a19.g.dreamhost.com (Postfix) with ESMTP id 86217B76F3; Wed, 4 Dec 2019 15:27:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=linkov.net; h=from:to:cc :subject:references:date:in-reply-to:message-id:mime-version :content-type; s=linkov.net; bh=XeKDuBcBIjIxCpr1gZ2m3tjJWqI=; b= w7hVaF/qvULkTAvHbPUOFDf11OpxByEURT+0W5L74n+u/c7rh12wNDEA1TkGuQsp 5pICEYANCJcuiIy/zArwgmJdJ8xd7MM4IgQsBsTW9S5hqc1BqLQJP1cbp9JCLicv aArb7ROQsFvSNpb5orirUxUoAQUkmQphofglovCOkmc= Original-Received: from mail.jurta.org (m91-129-96-42.cust.tele2.ee [91.129.96.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: jurta@jurta.org) by pdx1-sub0-mail-a19.g.dreamhost.com (Postfix) with ESMTPSA id CDABAB76EF; Wed, 4 Dec 2019 15:27:17 -0800 (PST) X-DH-BACKEND: pdx1-sub0-mail-a19 In-Reply-To: <87immvzvf8.fsf@cochranmail.com> (Robert Cochran's message of "Wed, 04 Dec 2019 12:29:31 -0800") X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 23.83.209.45 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:243130 Archived-At: >>> 1. Trying to make a single hook do two separate things (determining >>> whether to prevent tab closing and tasks on tab close) just made things >>> overly complicated. Tab close prevention has been split off into a new >>> hook, `tab-bar-prevent-close-functions'. >> >> Then better to have two hooks: >> tab-bar-tab-prevent-close-functions, >> tab-bar-tab-post-close-functions. >> >> The first can be used not only for close prevention, but also >> for other tasks when necessary. But the primary hook for tasks >> on closing the tab could be the post hook invoked after the tab is closed. >> This helps to run tasks such as killing the buffers when no other >> remaining tabs still use the buffers after closing the tab. > > To be clear, `tab-bar-tab-pre-close-functions` didn't go away. I'm not > exactly sure of a situation where it really really matters from the > perspective of a hook function whether or not it's clean up task happens > before or after a tab is technically closed, but I'm not opposed to > having both a pre and post close hook. However, I do believe that > there's more information available to a hook function about a tab before > the tab is formally closed. Does tab-bar-tab-prevent-close-functions have the same information? Then why should we have two exactly the same pre-hooks? To me it makes more sense cleaning the workplace after the work is done, not before. So killing the buffers is safer after closing the tab. If something goes wrong, then buffers are still here when the tab is not closed. Or are there some technical problems with using a post-close hook for such task? >>> 2. I realized that it's possible when creating a new tab to just delay >>> the actual creation of the tab in the frame's tab list. This makes it >>> possible to directly modify the tab passed in to >>> tab-bar-tab-post-open-functions, ie `(setf (alist-get 'name tab) "New Name")` >>> from within a hook function. This means it's not really >>> necessary to make new accessors. >> >> It would be better to run the hook after the tab is added to >> the frame's tab list, not before. This will widen the usability >> of the hook. For example, when display-buffer-in-tab from >> (info "(gnus) Tabbed Interface") creates a new tab for Gnus Summary, >> I want to use such a new hook to move a new tab to the predefined place >> (to keep the same order of Summary tabs as groups are sorted in the >> Group buffer). When the hook will be called after a new tab is added >> to the frame's tab list, it can be used to move a new tab automatically. > > Perhaps then we should also split `tab-bar-tab-post-open-functions` into > pre and post variants (run before and after the tab is added to the > frame parameters respectively)? I won't deny that an arrangement like > that could possibly be confusing (Ex: "why is my hook not working to rename > my tab?" -> "oh, you have to put that in the pre open hook, not the post > open hook"), Why tab renaming works only in the pre-open hook, but not in the post-open hook? I see no possible reasons why this shouldn't be possible. > but I personally find that a more attractive alternative > that expecting hooks to have to grovel through the frame parameters > themselves or to write a bunch of accessor functions to cover all the > possible use-cases. I think the user should have the final word - after the command that created a new tab and added it to the frame's tab list, the user should have freedom to customize the result of the command using hooks. This is possible only when the hook is run at the very end of the command. >>> New patch, as well as a file of examples for the new hooks, follow. >>> Again, these new hooks still need to be documented in the manual, >>> which I will be glad to do as soon as a design is nailed down. >> >> Thanks for the new patch. An additional comment below. >> >>> + (last-tab-p (= 1 (length tabs))) >>> + (prevent-close (run-hook-with-args-until-success >>> + 'tab-bar-tab-prevent-close-functions >>> + (nth close-index tabs) >>> + last-tab-p))) >>> + >>> + (unless prevent-close >>> + (run-hook-with-args 'tab-bar-tab-pre-close-functions >>> + (nth close-index tabs) >>> + last-tab-p) >>> + >>> + (if (= 1 (length tabs)) >>> + (pcase tab-bar-close-last-tab-choice >>> + ('nil >>> + (signal 'user-error '("Attempt to delete the sole tab in a frame"))) >>> + ('delete-frame >>> + (delete-frame)) >>> + ('tab-bar-mode-disable >>> + (tab-bar-mode -1)) >>> + ((pred functionp) >>> + ;; Give the handler function the full extent of the tab's >>> + ;; data, not just it's name and explicit-name flag. >>> + (funcall tab-bar-close-last-tab-choice (tab-bar--tab)))) >> >> There is the same condition '(= 1 (length tabs))' used twice. >> This suggests that the code containing 'pcase tab-bar-close-last-tab-choice' >> could be moved to a new function added to tab-bar-tab-pre-close-functions >> by default. It could check for '(= 1 (length tabs))', then does its pcase >> and returns nil to prevent other code in tab-bar-close-tab from running. >> This is just an optimization. > > It's probably better to reuse the variable last-tab-p in this case, > yes. I'll make sure that gets fixed if the code remains in the next > patch revision, as well as simplify the signaling of the user-error. I'm > shying away from putting that code into a hook though because: > > A) It might confuse someone who, for whatever reason, has managed to > remove the function handling `tab-bar-close-last-tab-choice` and is now > left wondering why the variable isn't being respected. > > B) Part of the point of this particular segment of code was to fix a bug > where tabs lost their explicit names when the user attempted to close > them. The solution was having a well-defined point at which the function > is short-circuited if there is only one tab. This part of the logic is > dispatched by a defcustom because there were 3-4 ideas on what the right > answer was to the user closing the last time. Right, this is better. Thanks.