unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH] tab-bar.el: add defcustoms for functions to call after opening and before closing
@ 2019-11-08 19:57 Robert Cochran
  2019-11-10 20:42 ` Juri Linkov
  0 siblings, 1 reply; 9+ messages in thread
From: Robert Cochran @ 2019-11-08 19:57 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 1785 bytes --]

Hi all,

A rough version of the patch we were talking about in the other thread
on persistent naming. To recap:

New defcustoms, tab-bar-post-open-functions and
tab-bar-pre-close-functions. Call all of the function in
post-open-functions at the end of tab-bar-new-tab-to, passing in the new
tab as an argument. Similar for pre-close-functions, but slightly more
complicated to lend some expressive power. Each of the functions take 2
arguments, the tab itself, and a flag indicating whether or not it's the
last tab. The big thing is that the return values are kept track of, and
if *any* of the functions return a non-nil value
(ie `(cl-some (mapcar (lambda (x) (funcall x tab last-tab))) pre-close-functions)`
is non-nil), it short-circuits and doesn't run the rest of
tab-bar-close-tab. This would be useful to provide to users so
that they can do other clean-up tasks when the tab is closed, as well as
providing a way to protect a tab from being closed. Ditto for being able
to do some tab set-up when you open a new tab.

This is going to take some additional work. I couldn't think of a clean
way to gracefully provide a way to easily make permanent changes to the
tab for the purposes of tab-bar-post-open-functions - the functions
within have to grovel through the tabs frame parameter and write back a value
themselves. I personally don't find this an optimal user experience, but
feedback on that point is welcome.

This will also need to be documented in the manual, but I can do that
after we hash out some details - I just didn't want to make changes to
the manual until we're satisfied with the semantics of this whole thing.

Patch follows. Thoughts on it appreciated.

Thanks,
-- 
~Robert Cochran

GPG Fingerprint - BD0C 5F8B 381C 64F0 F3CE  E7B9 EC9A 872C 41B2 77C2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: New variables for functions to call after opening/before closing a tab --]
[-- Type: text/x-patch, Size: 7563 bytes --]

From 696c7598d716a6c0bfb873b224e9c63c756dd45e Mon Sep 17 00:00:00 2001
From: Robert Cochran <robert-git@cochranmail.com>
Date: Fri, 8 Nov 2019 11:29:43 -0800
Subject: [PATCH] Add variables for functions to call after tab open and before
 close

* lisp/tab-bar.el (tab-bar-tab-post-open-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 | 127 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 82 insertions(+), 45 deletions(-)

diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index e915930c27..eac785abaf 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -659,6 +659,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 need to be stored to the frame's parameter
+alist - see `frame-parameter' and `set-frame-parameter'."
+  :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
@@ -697,6 +706,13 @@ tab-bar-new-tab-to
         ;; pushnew handles the head of tabs but not frame-parameter
         (set-frame-parameter nil 'tabs tabs)))
 
+    ;;; FIXME: Ideally, one should be able to change attributes of the
+    ;;; tab without having to write it back via `set-frame-parameter'
+    ;;; or cousins. A possible solution is to provide tab attribute
+    ;;; accessors.
+    (dolist (fun tab-bar-tab-post-open-functions)
+      (funcall fun (tab-bar--tab)))
+
     (when (and (not tab-bar-mode)
                (or (eq tab-bar-show t)
                    (and (natnump tab-bar-show)
@@ -747,6 +763,17 @@ tab-bar-close-last-tab-choice
   :group 'tab-bar
   :version "27.1")
 
+(defcustom tab-bar-tab-pre-close-functions nil
+  "List of functions to call before closing a tab.
+The current tab and a boolean indicating whether or not it is the
+only tab in the frame are supplied as arguments, respectively.
+Each function's returned value is used to determine whether or
+not the tab will be closed. If any of the functions return
+non-nil, then the tab will not be closed."
+  :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
@@ -759,52 +786,62 @@ 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))
+         prevent-close)
+
+    (dolist (fun tab-bar-tab-pre-close-functions)
+      (let ((function-value (funcall fun
+                                     (nth close-index tabs)
+                                     (= 1 (length tabs)))))
+        ;; Make non-nil return values 'stick'
+        (setf prevent-close (or prevent-close function-value))))
+
+    (unless prevent-close
+      (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))
 
-      (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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] tab-bar.el: add defcustoms for functions to call after opening and before closing
  2019-11-08 19:57 [PATCH] tab-bar.el: add defcustoms for functions to call after opening and before closing Robert Cochran
@ 2019-11-10 20:42 ` Juri Linkov
  2019-12-02 22:46   ` Robert Cochran
  0 siblings, 1 reply; 9+ messages in thread
From: Juri Linkov @ 2019-11-10 20:42 UTC (permalink / raw)
  To: Robert Cochran; +Cc: emacs-devel

> New defcustoms, tab-bar-post-open-functions and
> tab-bar-pre-close-functions. Call all of the function in
> post-open-functions at the end of tab-bar-new-tab-to, passing in the new
> tab as an argument. Similar for pre-close-functions, but slightly more
> complicated to lend some expressive power. Each of the functions take 2
> arguments, the tab itself, and a flag indicating whether or not it's the
> last tab. The big thing is that the return values are kept track of, and
> if *any* of the functions return a non-nil value
> (ie `(cl-some (mapcar (lambda (x) (funcall x tab last-tab))) pre-close-functions)`
> is non-nil), it short-circuits and doesn't run the rest of
> tab-bar-close-tab.

Also similar to run-hook-with-args-until-success.

> This would be useful to provide to users so that they can do other
> clean-up tasks when the tab is closed, as well as providing a way to
> protect a tab from being closed. Ditto for being able to do some tab
> set-up when you open a new tab.

Please bring up some examples of usages for these options because
without examples it's hard to understand how these options are
intended to be used, e.g. how to rename a new tab after creating,
or do a clean-up task on closing, or prevent from closing, etc.

> This is going to take some additional work. I couldn't think of a clean
> way to gracefully provide a way to easily make permanent changes to the
> tab for the purposes of tab-bar-post-open-functions - the functions
> within have to grovel through the tabs frame parameter and write back a value
> themselves. I personally don't find this an optimal user experience, but
> feedback on that point is welcome.

But you provide a new tab as a parameter to tab-bar-post-open-functions.
Could it be used to make changes in a new tab?

> Patch follows.

Thanks.



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] tab-bar.el: add defcustoms for functions to call after opening and before closing
  2019-11-10 20:42 ` Juri Linkov
@ 2019-12-02 22:46   ` Robert Cochran
  2019-12-03 23:19     ` Juri Linkov
  0 siblings, 1 reply; 9+ messages in thread
From: Robert Cochran @ 2019-12-02 22:46 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Robert Cochran, emacs-devel

[-- Attachment #1: Type: text/plain, Size: 1283 bytes --]

Hi all,

I've taken a little bit of time to rethink how I wanted this to
work. Namely:

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'.

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.

To be honest, I've been having a hard time coming up with good examples
of how to use these hooks that aren't arbitrary, contrived, or don't
otherwise require support in other places (more than one of the possible
examples I was thinking of works best when another mode or command, such
as gdb or mpc, has the ability to automatically start in a new
tab).

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,
-- 
~Robert Cochran


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch for post-open, pre-close, and close prevent hooks for tabs --]
[-- Type: text/x-patch, Size: 7700 bytes --]

From d433b8075ec461ea293a2f95f3dbb7768fc0ad1e Mon Sep 17 00:00:00 2001
From: Robert Cochran <robert-git@cochranmail.com>
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 | 132 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 87 insertions(+), 45 deletions(-)

diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index 5eb332884c..51cbc16aba 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -659,6 +659,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
@@ -693,6 +702,10 @@ 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))
+
+      (run-hook-with-args 'tab-bar-tab-post-open-functions
+                          (nth to-index tabs))
+
       (when (eq to-index 0)
         ;; pushnew handles the head of tabs but not frame-parameter
         (set-frame-parameter nil 'tabs tabs)))
@@ -747,6 +760,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
@@ -759,52 +790,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 (= 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))
 
-      (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


[-- Attachment #3: Type: text/plain, Size: 56 bytes --]


---
patch ends here
---
tab-buffer.el begins here
---


[-- Attachment #4: Example functions for pre-open/post-close/prevent-close hooks --]
[-- Type: text/plain, Size: 3349 bytes --]

;; -*- lexical-binding: t -*-

(defmacro tab-ex/save-tab-excursion (&rest body)
  (declare (indent defun))
  (let ((old-tab-sym (gensym)))
    `(let ((,old-tab-sym (tab-bar--current-tab-index)))
       (unwind-protect (progn ,@body)
         (tab-bar-select-tab (1+ ,old-tab-sym))))))

(defmacro tab-ex/with-current-tab (new-tab &rest body)
  (declare (indent 1))
  `(tab-ex/save-tab-excursion
     (tab-bar-select-tab (1+ (tab-bar--tab-index ,new-tab)))
     ;; No need to wrap body in a progn here; we're already in a progn in the
     ;; parent macro.
     ,@body))

(defun tab-ex/tab-buffers (tab)
  ;; Windows aren't usable after exiting `tab-ex/with-current-tab's
  ;; scope, so you have to extract the buffers out of them *then*,
  ;; otherwise they go stale (likely as a result of them no longer
  ;; being 'visible') and you can't pull the buffers out of them
  ;; anymore
  (delete-dups (if (eq (car tab) 'current-tab)
                   (mapcar #'window-buffer
                           (window-list-1 (frame-first-window) 'nomini))
                 (tab-ex/with-current-tab tab
                   (mapcar #'window-buffer
                           (window-list-1 (frame-first-window) 'nomini))))))

(defun tab-ex/gud-tab-explicit-name (tab)
  "Rename tab and enable gud-many-buffers when tab is made with the gud core buffer.

When using GUD in many-buffers mode, rename the tab to something
static so that it's easy to tell at a glance what the tab
contains despite the various buffer names."
  (let ((tab-buffers (tab-ex/tab-buffers tab)))
    (dolist (buf tab-buffers)
      (with-current-buffer buf
        (when (and (eq major-mode 'gud-mode))
          ;; It's probably better to have a way to just have gud make
          ;; a new tab and always have many windows on, but this is
          ;; just an example.
          (gdb-many-windows)
          (save-match-data
            (string-match "\\*gud-\\(.*\\)\\*" (buffer-name))
            (setf (alist-get 'name tab) (format "GUD: %s"
                                                (match-string 1 (buffer-name)))
                  (alist-get 'explicit-name tab) t)))))))

(defun tab-ex/tab-has-unsaved-files (tab _lastp)
  ;; Don't let the user close a tab if they have unsaved buffers, to
  ;; make sure they don't lose track of unsaved buffers, for
  ;; example. In this case, it doesn't matter if this tab is the last
  ;; one or not.
  (cl-loop for buffer in (tab-ex/tab-buffers tab)
           always (and (with-current-buffer buffer buffer-file-name)
                       (not (buffer-modified-p buffer)))))

(defun tab-ex/kill-non-file-buffers-in-tab (tab lastp)
  ;; Don't kill the buffers if the tab is going to still be visible in
  ;; the end of it.
  (unless lastp
    (tab-ex/with-current-tab tab
      ;; Just kill all the visible non-file buffers, for the sake of example.
      (mapc #'kill-buffer (cl-loop for buffer in (tab-ex/tab-buffers tab)
                                   unless (with-current-buffer buffer buffer-file-name)
                                   collect buffer)))))

(add-hook 'tab-bar-tab-post-open-functions
          #'tab-ex/gud-tab-explicit-name)

(add-hook 'tab-bar-tab-prevent-close-functions
          #'tab-ex/tab-has-unsaved-files)

(add-hook 'tab-bar-tab-pre-close-functions
          #'tab-ex/kill-non-file-buffers-in-tab)

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] tab-bar.el: add defcustoms for functions to call after opening and before closing
  2019-12-02 22:46   ` Robert Cochran
@ 2019-12-03 23:19     ` Juri Linkov
  2019-12-04 20:29       ` Robert Cochran
  0 siblings, 1 reply; 9+ messages in thread
From: Juri Linkov @ 2019-12-03 23:19 UTC (permalink / raw)
  To: Robert Cochran; +Cc: emacs-devel

> 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.

> 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.

> 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.

> (defun tab-ex/tab-has-unsaved-files (tab _lastp)
>   ;; Don't let the user close a tab if they have unsaved buffers, to
>   ;; make sure they don't lose track of unsaved buffers, for
>   ;; example. In this case, it doesn't matter if this tab is the last
>   ;; one or not.
>   (cl-loop for buffer in (tab-ex/tab-buffers tab)
>            always (and (with-current-buffer buffer buffer-file-name)
>                        (not (buffer-modified-p buffer)))))

Good example - some editors also prepend "*" to the tab name
of an unsaved file, and don't allow closing it without saving.



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] tab-bar.el: add defcustoms for functions to call after opening and before closing
  2019-12-03 23:19     ` Juri Linkov
@ 2019-12-04 20:29       ` Robert Cochran
  2019-12-04 22:45         ` Juri Linkov
  0 siblings, 1 reply; 9+ messages in thread
From: Robert Cochran @ 2019-12-04 20:29 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Robert Cochran, emacs-devel

Juri Linkov <juri@linkov.net> writes:

>> 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.

>> 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"), 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.

>> 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.

>> (defun tab-ex/tab-has-unsaved-files (tab _lastp)
>>   ;; Don't let the user close a tab if they have unsaved buffers, to
>>   ;; make sure they don't lose track of unsaved buffers, for
>>   ;; example. In this case, it doesn't matter if this tab is the last
>>   ;; one or not.
>>   (cl-loop for buffer in (tab-ex/tab-buffers tab)
>>            always (and (with-current-buffer buffer buffer-file-name)
>>                        (not (buffer-modified-p buffer)))))
>
> Good example - some editors also prepend "*" to the tab name
> of an unsaved file, and don't allow closing it without saving.

Sidenote for later, but it sounds like having hooks of the nature that
we're hashing out here would also be useful for tab-line-mode as
well. That's a different discussion though.

Thanks,
-- 
~Robert Cochran



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] tab-bar.el: add defcustoms for functions to call after opening and before closing
  2019-12-04 20:29       ` Robert Cochran
@ 2019-12-04 22:45         ` Juri Linkov
  2019-12-05 23:05           ` Robert Cochran
  0 siblings, 1 reply; 9+ messages in thread
From: Juri Linkov @ 2019-12-04 22:45 UTC (permalink / raw)
  To: Robert Cochran; +Cc: emacs-devel

>>> 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.



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] tab-bar.el: add defcustoms for functions to call after opening and before closing
  2019-12-04 22:45         ` Juri Linkov
@ 2019-12-05 23:05           ` Robert Cochran
  2019-12-07 22:25             ` Juri Linkov
  0 siblings, 1 reply; 9+ messages in thread
From: Robert Cochran @ 2019-12-05 23:05 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Robert Cochran, emacs-devel

[-- Attachment #1: Type: text/plain, Size: 4247 bytes --]

Juri Linkov <juri@linkov.net> 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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch to add hooks for tab post-open, pre-close, and close prevention --]
[-- Type: text/x-patch, Size: 7805 bytes --]

From 0ec05e98ad28c5e5c96c2392dd971a14748fdcb9 Mon Sep 17 00:00:00 2001
From: Robert Cochran <robert-git@cochranmail.com>
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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] tab-bar.el: add defcustoms for functions to call after opening and before closing
  2019-12-05 23:05           ` Robert Cochran
@ 2019-12-07 22:25             ` Juri Linkov
  2019-12-11 18:32               ` Robert Cochran
  0 siblings, 1 reply; 9+ messages in thread
From: Juri Linkov @ 2019-12-07 22:25 UTC (permalink / raw)
  To: Robert Cochran; +Cc: Robert Cochran, emacs-devel

> ;; 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.

Indeed, this is better than to depend on the ordering of function calls
in the 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.

Right, more contextual information is available when the tab is still
attached to the tab bar.

> 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.

Then let's agree on a general principle: both open and close hooks
should operate on the tab when it is still attached to the tab bar.

> I've fixed it in this version of the patch.

Thanks, now your patch is pushed to master.



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] tab-bar.el: add defcustoms for functions to call after opening and before closing
  2019-12-07 22:25             ` Juri Linkov
@ 2019-12-11 18:32               ` Robert Cochran
  0 siblings, 0 replies; 9+ messages in thread
From: Robert Cochran @ 2019-12-11 18:32 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Robert Cochran, emacs-devel

[-- Attachment #1: Type: text/plain, Size: 310 bytes --]

Juri Linkov <juri@linkov.net> writes:

>> I've fixed it in this version of the patch.
>
> Thanks, now your patch is pushed to master.

Thanks.

Just realized though that I forgot to change tab-bar-close-other-tabs to
also use the new custom variables, so here's a patch for that.

Cheers,
-- 
~Robert Cochran


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Have tab-bar-close-other-tabs use tab close customs --]
[-- Type: text/x-patch, Size: 1423 bytes --]

From b777800d60c02745b20ea31ce4b10cf010168889 Mon Sep 17 00:00:00 2001
From: Robert Cochran <robert-git@cochranmail.com>
Date: Wed, 11 Dec 2019 10:29:00 -0800
Subject: [PATCH] * lisp/tab-bar.el (tab-bar-close-other-tabs): Use tab close
 customs

---
 lisp/tab-bar.el | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index ba4bc0afcd..59cba2c877 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -896,11 +896,18 @@ tab-bar-close-other-tabs
          (current-index (tab-bar--current-tab-index tabs)))
     (when current-index
       (dotimes (index (length tabs))
-        (unless (eq index current-index)
+        (unless (or (eq index current-index)
+                    (run-hook-with-args-until-success
+                     'tab-bar-tab-prevent-close-functions
+                     (nth index tabs)
+                     ; last-tab-p logically can't ever be true if we
+                     ; make it this far
+                     nil))
           (push `((frame . ,(selected-frame))
                   (index . ,index)
                   (tab . ,(nth index tabs)))
-                tab-bar-closed-tabs)))
+                tab-bar-closed-tabs)
+          (run-hook-with-args 'tab-bar-tab-pre-close-functions (nth index tabs) nil)))
       (set-frame-parameter nil 'tabs (list (nth current-index tabs)))
 
       (when (and tab-bar-mode
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-12-11 18:32 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-08 19:57 [PATCH] tab-bar.el: add defcustoms for functions to call after opening and before closing Robert Cochran
2019-11-10 20:42 ` Juri Linkov
2019-12-02 22:46   ` Robert Cochran
2019-12-03 23:19     ` Juri Linkov
2019-12-04 20:29       ` Robert Cochran
2019-12-04 22:45         ` Juri Linkov
2019-12-05 23:05           ` Robert Cochran
2019-12-07 22:25             ` Juri Linkov
2019-12-11 18:32               ` Robert Cochran

Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).