From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Robert Cochran 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 15:05:08 -0800 Message-ID: <8736dyfk63.fsf@cochranmail.com> References: <87h83egmzy.fsf@cochranmail.com> <87r22fzhag.fsf@mail.linkov.net> <87zhgawdk9.fsf@cochranmail.com> <87o8wpm15x.fsf@mail.linkov.net> <87immvzvf8.fsf@cochranmail.com> <87lfrrn20p.fsf@mail.linkov.net> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="266892"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) Cc: Robert Cochran , emacs-devel@gnu.org To: Juri Linkov Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Dec 06 01:10:11 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 1id1CF-0017Iy-2P for ged-emacs-devel@m.gmane.org; Fri, 06 Dec 2019 01:10:11 +0100 Original-Received: from localhost ([::1]:33878 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1id1CD-0004Ev-Cl for ged-emacs-devel@m.gmane.org; Thu, 05 Dec 2019 19:10:09 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:45768) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1id0BV-0007jg-KW for emacs-devel@gnu.org; Thu, 05 Dec 2019 18:05:23 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1id0BR-0003pe-T6 for emacs-devel@gnu.org; Thu, 05 Dec 2019 18:05:20 -0500 Original-Received: from mail.cochranmail.com ([64.140.150.170]:59826) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1id0BR-0003iT-AV for emacs-devel@gnu.org; Thu, 05 Dec 2019 18:05:17 -0500 Original-Received: from SoraLaptop (unknown [71.212.134.29]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.cochranmail.com (Postfix) with ESMTPSA id 00163178E; Thu, 5 Dec 2019 15:05:12 -0800 (PST) In-Reply-To: <87lfrrn20p.fsf@mail.linkov.net> (Juri Linkov's message of "Thu, 05 Dec 2019 00:45:20 +0200") X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 64.140.150.170 X-Mailman-Approved-At: Thu, 05 Dec 2019 19:10:02 -0500 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:243172 Archived-At: --=-=-= Content-Type: text/plain Juri Linkov writes: >> 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? pre-close-functions and prevent-close-functions get all of the same information, yes. I decided to separate them into two separate things because otherwise you run into function dependency issues. For example, if tab-bar-pre-close-functions worked the way I had original defined it, where it did double duty of what I'm now proposing to be pre-close-functions and prevent-close-functions, then if the hook were to look something like this: ;; From the examples in the last email (#'tab-ex/kill-non-file-buffers-in-tab #'tab-ex/tab-has-unsaved-files) then when closing a tab, you'd first kill all of the non-file buffers, and then check to see if you should close the tab based on whether or not there are any visible unsaved buffers. But at this point, you've already run cleanup functions regardless of whether or not you ultimately decide to close the tab. Mixing cleanup tasks and deciding whether or not to close the tab leads to weird edge-cases and being dependent on function ordering. I think that's too much complexity to wrangle in a single hook. The only 'technical' problem with a post-close hook is that the tab information is now gone because it's been deleted from the list. A hook gets slightly less contextual information if we decide to hold off on running the hook until the end. While I'll agree that it might make a little more sense to run the hook at the end, I'm going to counter by saying that we should arrange for the hook to be able to get as much information as it can get, because you'll never know when someone wants all of it. It's better to have to filter out information you don't need than to be stuck without the information you do need. >> 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. > I tested this again just now and found out that it actually does work to just get the alist from frame-parameter and modify it directly. I had an incorrect understanding about what did and did not work. My appologies; I thought I had tested that better. With that misunderstanding cleared up, my original point is moot, and moving the call to run-hooks-with-args to after the tab is put into the frame-parameters list isn't problematic - life is still easy to modify tab parameters. I've fixed it in this version of the patch. Thanks, -- ~Robert Cochran --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0001-Add-hooks-for-after-tab-open-before-close-and-to-pre.patch Content-Description: Patch to add hooks for tab post-open, pre-close, and close prevention >From 0ec05e98ad28c5e5c96c2392dd971a14748fdcb9 Mon Sep 17 00:00:00 2001 From: Robert Cochran Date: Fri, 8 Nov 2019 11:29:43 -0800 Subject: [PATCH] Add hooks for after tab open, before close, and to prevent closing * lisp/tab-bar.el (tab-bar-tab-post-open-functions, tab-bar-tab-prevent-close-functions, tab-bar-tab-pre-close-functions): New defcustoms (tab-bar-new-tab-to, tab-bar-close-tab): Use new defcustoms --- lisp/tab-bar.el | 134 +++++++++++++++++++++++++++++++----------------- 1 file changed, 88 insertions(+), 46 deletions(-) diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el index acc4304def..ba4bc0afcd 100644 --- a/lisp/tab-bar.el +++ b/lisp/tab-bar.el @@ -692,6 +692,15 @@ tab-bar-new-tab-to :group 'tab-bar :version "27.1") +(defcustom tab-bar-tab-post-open-functions nil + "List of functions to call after creating a new tab. +The current tab is supplied as an argument. Any modifications +made to the tab argument will be applied after all functions are +called." + :type '(repeat function) + :group 'tab-bar + :version "27.1") + (defun tab-bar-new-tab-to (&optional to-index) "Add a new tab at the absolute position TO-INDEX. TO-INDEX counts from 1. If no TO-INDEX is specified, then add @@ -726,9 +735,13 @@ tab-bar-new-tab-to ('right (1+ (or from-index 0))))))) (setq to-index (max 0 (min (or to-index 0) (length tabs)))) (cl-pushnew to-tab (nthcdr to-index tabs)) + (when (eq to-index 0) ;; pushnew handles the head of tabs but not frame-parameter - (set-frame-parameter nil 'tabs tabs))) + (set-frame-parameter nil 'tabs tabs)) + + (run-hook-with-args 'tab-bar-tab-post-open-functions + (nth to-index tabs))) (when (and (not tab-bar-mode) (or (eq tab-bar-show t) @@ -780,6 +793,24 @@ tab-bar-close-last-tab-choice :group 'tab-bar :version "27.1") +(defcustom tab-bar-tab-prevent-close-functions nil + "List of functions to call to determine whether to close a tab. +The tab to be closed and a boolean indicating whether or not it +is the only tab in the frame are supplied as arguments. If any +function returns a non-nil value, the tab will not be closed." + :type '(repeat function) + :group 'tab-bar + :version "27.1") + +(defcustom tab-bar-tab-pre-close-functions nil + "List of functions to call before closing a tab. +The tab to be closed and a boolean indicating whether or not it +is the only tab in the frame are supplied as arguments, +respectively." + :type '(repeat function) + :group 'tab-bar + :version "27.1") + (defun tab-bar-close-tab (&optional arg to-index) "Close the tab specified by its absolute position ARG. If no ARG is specified, then close the current tab and switch @@ -792,52 +823,63 @@ tab-bar-close-tab (interactive "P") (let* ((tabs (funcall tab-bar-tabs-function)) (current-index (tab-bar--current-tab-index tabs)) - (close-index (if (integerp arg) (1- arg) current-index))) - (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)))) - - ;; More than one tab still open - (when (eq current-index close-index) - ;; Select another tab before deleting the current tab - (let ((to-index (or (if to-index (1- to-index)) - (pcase tab-bar-close-tab-select - ('left (1- current-index)) - ('right (if (> (length tabs) (1+ current-index)) - (1+ current-index) - (1- current-index))) - ('recent (tab-bar--tab-index-recent 1 tabs)))))) - (setq to-index (max 0 (min (or to-index 0) (1- (length tabs))))) - (tab-bar-select-tab (1+ to-index)) - ;; Re-read tabs after selecting another tab - (setq tabs (funcall tab-bar-tabs-function)))) - - (let ((close-tab (nth close-index tabs))) - (push `((frame . ,(selected-frame)) - (index . ,close-index) - (tab . ,(if (eq (car close-tab) 'current-tab) - (tab-bar--tab) - close-tab))) - tab-bar-closed-tabs) - (set-frame-parameter nil 'tabs (delq close-tab tabs))) - - (when (and tab-bar-mode - (and (natnump tab-bar-show) - (<= (length tabs) tab-bar-show))) - (tab-bar-mode -1)) + (close-index (if (integerp arg) (1- arg) current-index)) + (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 last-tab-p + (pcase tab-bar-close-last-tab-choice + ('nil + (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)))) + + ;; More than one tab still open + (when (eq current-index close-index) + ;; Select another tab before deleting the current tab + (let ((to-index (or (if to-index (1- to-index)) + (pcase tab-bar-close-tab-select + ('left (1- current-index)) + ('right (if (> (length tabs) (1+ current-index)) + (1+ current-index) + (1- current-index))) + ('recent (tab-bar--tab-index-recent 1 tabs)))))) + (setq to-index (max 0 (min (or to-index 0) (1- (length tabs))))) + (tab-bar-select-tab (1+ to-index)) + ;; Re-read tabs after selecting another tab + (setq tabs (funcall tab-bar-tabs-function)))) + + (let ((close-tab (nth close-index tabs))) + (push `((frame . ,(selected-frame)) + (index . ,close-index) + (tab . ,(if (eq (car close-tab) 'current-tab) + (tab-bar--tab) + close-tab))) + tab-bar-closed-tabs) + (set-frame-parameter nil 'tabs (delq close-tab tabs))) + + (when (and tab-bar-mode + (and (natnump tab-bar-show) + (<= (length tabs) tab-bar-show))) + (tab-bar-mode -1)) - (force-mode-line-update) - (unless tab-bar-mode - (message "Deleted tab and switched to %s" tab-bar-close-tab-select))))) + (force-mode-line-update) + (unless tab-bar-mode + (message "Deleted tab and switched to %s" tab-bar-close-tab-select)))))) (defun tab-bar-close-tab-by-name (name) "Close the tab by NAME." -- 2.23.0 --=-=-=--