Re: [PATCH] Enable persistent naming for tabs

[Juri Linkov]

> Hi all,
>
> Another day, another patch :)
>
>>>> But please give me an additional day to think about a possibility of
>>>> avoiding `no-auto-name` to not make code unnecessarily complicated.
>>> [...]
>>> Other than that, I'm happy to wait a little bit to get the best possible
>>> implementation for the benefit of everyone. Thanks for your attention
>>> and encouragement on this. :)
>>
>> Meanwhile I looked at prior art of how this feature in already implemented
>> in Emacs.  I think we should avoid needlessly introducing new concepts,
>> but instead use the existing solutions to reduce entropy and improve
>> maintainability of code.  Tabs were implemented to represent frame-like
>> window configurations inside one frame, so tab renaming could work like
>> frame renaming where M-x set-frame-name reads a new name and sets the
>> frame parameter `name`.  What it also does is to set additional parameter
>> `explicit-name`.  Please use the same parameter name in tabs to not
>> divert too much from frame name handling.
>
> Done. All instances of 'no-auto-name' changed to 'explicit-name'. That's
> honestly a much better name, and being consistent with frames is all the
> better. Thanks for the guidance on that.

Thanks for renaming.

>> What we could do is to improve both implementations, e.g. neither
>> set-frame-name nor your patch provide a default value, but it would
>> be useful to be able to type M-n in the renaming minibuffer to
>> bring the current name for editing.
>
> I'll look into making this happen in a different patch.

Right, this will simplify handling changes.

>> Also I don't see a need to add `(explicit-name . nil)` by default.
>> It seems it should be enough to add only `(explicit-name . t)`
>> after manual renaming.  But maybe prefilling all possible parameters
>> initially really simplifies implementation?
>
> I think that it simplifies implementation - no having to unstitch and
> restitch parameters, just do a couple of assqs. New frames get
> `(explicit-name . nil)` regardless, so I figure we should follow suit.

Yes, frames have `(explicit-name . nil)` for a reason.

>> Finally what needs to be improved is to make tab-bar--tab and
>> tab-bar--current-tab consistent.  In your patch tab-bar--tab
>> gets current-tab directly from parameters, but tab-bar--current-tab
>> accepts a tab from its argument.  Shouldn't tab-bar--tab also
>> accept the current tab from its argument?
>
> I'm going to argue no, and my reasoning is that tab-bar--tab is only
> ever called to convert an 'incomplete' current-tab alist into a full tab
> alist. IOW, we don't need to explicitly provide the context of which tab
> we're going to be using in this case - it will always whatever the
> current tab is. tab-bar--current-tab doesn't even need the context for
> most calls: of the 3 calls to it, only the one in the tab switching
> function provides context, the others take the implicit default of the
> current default tab.

I agree.

>> In other regards your choice of a command name `tab-bar-rename-tab-by-name`
>> fits nicely into naming convention with the existing `select-frame-by-name`.
>> We also have `tab-bar-close-tab-by-name`.  There is also the command
>> `tab-bar-switch-to-tab` that selects a tab by name, it was named after
>> `switch-to-buffer`, maybe better name would be
>> `tab-bar-select-tab-by-name`?
>
> I have no strong naming preference myself.

Maybe we should add an alias `tab-bar-select-tab-by-name`
to `tab-bar-switch-to-tab`.

> I also went ahead and added `C-x 6 r` to the keymap, as well as
> documented both it and `tab-bar-tab-name-function` in the manual.

Thanks, now your patch looks ok to push.

> Something else I would like input on - if you try to close the only tab
> in the frame, then it loses its explicit name if it has one. This is
> because the tab closing function blindly deletes the entry from the tabs
> alist, and hence the explicit-name attribute. What should we do? I think
> we need to signal user-error or similar in interactive contexts, but
> what about when calling from Lisp programs?

Indeed, signaling user-error when closing a single tab would be
the right thing even when called programmatically, because the caller
can check the number of tabs before calling tab-bar-close-tab.

> Thanks,
> -- 
> ~Robert Cochran
>
> GPG Fingerprint - BD0C 5F8B 381C 64F0 F3CE  E7B9 EC9A 872C 41B2 77C2

>From d2b9eb3cbecf740191e33a9eb684671aff0fbbe9 Mon Sep 17 00:00:00 2001
From: Robert Cochran <address@hidden>
Date: Mon, 7 Oct 2019 13:41:47 -0700
Subject: [PATCH] Allow tabs to have consistent given names

* lisp/tab-bar.el (tab-bar--tab): Pull automatic name information from
current tab
(tab-bar--current-tab): Pull automatic name information from current
tab, or from new optional template argument
(tab-bar-select-tab): Pass the target tab as a template when setting
it as current tab
(tab-bar-rename-tab, tab-bar-rename-tab-by-name): New functions
* doc/emacs/frames.texi (Tab Bars): Document new tab rename functionality.
---
 doc/emacs/frames.texi |  4 ++
 lisp/tab-bar.el       | 93 +++++++++++++++++++++++++++++++++----------
 2 files changed, 77 insertions(+), 20 deletions(-)

diff --git a/doc/emacs/frames.texi b/doc/emacs/frames.texi
index 169eebab3e..5f64c7e8d5 100644
--- a/doc/emacs/frames.texi
+++ b/doc/emacs/frames.texi
@@ -1282,6 +1282,10 @@ Tab Bars
 @item C-x 6 d @var{directory} @key{RET}
 Select a Dired buffer for directory @var{directory} in another tab.
 This runs @code{dired-other-tab}.  @xref{Dired}.
+@item C-x 6 r @var{tabname} @key{RET}
+Renames the current tab to @var{tabname}.  You can control the
+programmatic name given to a tab by default by customizing the
+variable @code{tab-bar-tab-name-function}.
 @end table
 
 @vindex tab-bar-new-tab-choice
diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index c4eba8600a..2e96072a15 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -304,9 +304,13 @@ tab-bar-make-keymap-1
   (let* ((separator (or tab-bar-separator (if window-system " " "|")))
          (i 0)
          (tabs (funcall tab-bar-tabs-function))
-         (current-tab-name (assq 'name (assq 'current-tab tabs))))
-    (when current-tab-name
-      (setf (cdr current-tab-name) (funcall tab-bar-tab-name-function)))
+         (current-tab-name (assq 'name (assq 'current-tab tabs)))
+         (current-tab-explicit-name (assq 'explicit-name (assq 'current-tab 
tabs))))
+    (when (and current-tab-name
+               current-tab-explicit-name
+               (not (cdr current-tab-explicit-name)))
+      (setf (cdr current-tab-name)
+            (funcall tab-bar-tab-name-function)))
     (append
      '(keymap (mouse-1 . tab-bar-handle-mouse))
      (mapcan
@@ -358,16 +362,29 @@ tab-bar-make-keymap-1
 
 
 (defun tab-bar--tab ()
-  `(tab
-    (name . ,(funcall tab-bar-tab-name-function))
-    (time . ,(time-convert nil 'integer))
-    (wc . ,(current-window-configuration))
-    (ws . ,(window-state-get
-            (frame-root-window (selected-frame)) 'writable))))
-
-(defun tab-bar--current-tab ()
-  `(current-tab
-    (name . ,(funcall tab-bar-tab-name-function))))
+  (let* ((tab (assq 'current-tab (frame-parameter nil 'tabs)))
+         (tab-explicit-name (cdr (assq 'explicit-name tab))))
+    `(tab
+      (name . ,(if tab-explicit-name
+                   (cdr (assq 'name tab))
+                 (funcall tab-bar-tab-name-function)))
+      (explicit-name . ,tab-explicit-name)
+      (time . ,(time-convert nil 'integer))
+      (wc . ,(current-window-configuration))
+      (ws . ,(window-state-get
+              (frame-root-window (selected-frame)) 'writable)))))
+
+(defun tab-bar--current-tab (&optional tab)
+  ;; `tab` here is an argument meaning 'use tab as template'. This is
+  ;; necessary when switching tabs, otherwise the destination tab
+  ;; inherit the current tab's `explicit-name` parameter.
+  (let* ((tab (or tab (assq 'current-tab (frame-parameter nil 'tabs))))
+         (tab-explicit-name (cdr (assq 'explicit-name tab))))
+    `(current-tab
+      (name . ,(if tab-explicit-name
+                   (cdr (assq 'name tab))
+                 (funcall tab-bar-tab-name-function)))
+      (explicit-name . ,tab-explicit-name))))
 
 (defun tab-bar--current-tab-index (&optional tabs)
   ;; FIXME: could be replaced with 1-liner using seq-position
@@ -436,7 +453,7 @@ tab-bar-select-tab
 
         (when from-index
           (setf (nth from-index tabs) from-tab))
-        (setf (nth to-index tabs) (tab-bar--current-tab)))
+        (setf (nth to-index tabs) (tab-bar--current-tab (nth to-index tabs))))
 
       (when tab-bar-mode
         (force-mode-line-update)))))
@@ -594,16 +611,51 @@ tab-bar-close-other-tabs
           (force-mode-line-update)
         (message "Deleted all other tabs")))))
 
+(defun tab-bar-rename-tab (name &optional arg)
+  "Rename the tab specified by its absolute position ARG.
+If no ARG is specified, then rename the current tab.
+ARG counts from 1.
+If NAME is the empty string, then use the automatic name
+function `tab-bar-tab-name-function'."
+  (interactive "sNew name for tab (leave blank for automatic naming): \nP")
+  (let* ((tabs (tab-bar-tabs))
+         (tab-index (if arg
+                        (1- (max 0 (min arg (length tabs))))
+                      (tab-bar--current-tab-index tabs)))
+         (tab-to-rename (nth tab-index tabs))
+         (tab-explicit-name (> (length name) 0))
+         (tab-new-name (if tab-explicit-name
+                           name
+                         (funcall tab-bar-tab-name-function))))
+    (setf (cdr (assq 'name tab-to-rename)) tab-new-name
+          (cdr (assq 'explicit-name tab-to-rename)) tab-explicit-name
+          (frame-parameter nil 'tabs) tabs)
+    (if (tab-bar-mode)
+        (force-mode-line-update)
+      (message "Renamed tab to '%s'" tab-new-name))))
+
+(defun tab-bar-rename-tab-by-name (tab-name new-name)
+  "Rename the tab named TAB-NAME.
+If NEW-NAME is the empty string, then use the automatic name
+function `tab-bar-tab-name-function'."
+  (interactive (list (completing-read "Rename tab by name: "
+                                      (mapcar (lambda (tab)
+                                                (cdr (assq 'name tab)))
+                                              (tab-bar-tabs)))
+                     (read-from-minibuffer "New name for tab (leave blank for 
automatic naming): ")))
+  (tab-bar-rename-tab new-name (tab-bar--tab-index-by-name tab-name)))
+
 
 ;;; Short aliases
 
-(defalias 'tab-new         'tab-bar-new-tab)
-(defalias 'tab-close       'tab-bar-close-tab)
+(defalias 'tab-new      'tab-bar-new-tab)
+(defalias 'tab-close    'tab-bar-close-tab)
 (defalias 'tab-close-other 'tab-bar-close-other-tabs)
-(defalias 'tab-select      'tab-bar-select-tab)
-(defalias 'tab-next        'tab-bar-switch-to-next-tab)
-(defalias 'tab-previous    'tab-bar-switch-to-prev-tab)
-(defalias 'tab-list        'tab-bar-list)
+(defalias 'tab-select   'tab-bar-select-tab)
+(defalias 'tab-next     'tab-bar-switch-to-next-tab)
+(defalias 'tab-previous 'tab-bar-switch-to-prev-tab)
+(defalias 'tab-rename   'tab-bar-rename-tab)
+(defalias 'tab-list     'tab-bar-list)
 
 
 ;;; Non-graphical access to frame-local tabs (named window configurations)
@@ -860,6 +912,7 @@ ctl-x-6-map
 (define-key ctl-x-6-map "b" 'switch-to-buffer-other-tab)
 (define-key ctl-x-6-map "f" 'find-file-other-tab)
 (define-key ctl-x-6-map "\C-f" 'find-file-other-tab)
+(define-key ctl-x-6-map "r" 'tab-rename)
 
 
 (provide 'tab-bar)
-- 
2.21.0