bug#69578: 30.0.50; tab-bar-mode binding of (control tab) not always useful

bug#69578: 30.0.50; tab-bar-mode binding of (control tab) not always useful

[Gerd Möllmann]

tab-bar--define-keys makes bindings for TAG like this:

  (unless (global-key-binding [(control tab)])
    (global-set-key [(control tab)] #'tab-next))
  (unless (global-key-binding [(control shift tab)])
    (global-set-key [(control shift tab)] #'tab-previous))
  (unless (global-key-binding [(control shift iso-lefttab)])
    (global-set-key [(control shift iso-lefttab)] #'tab-previous))

These bindings stop taking effect if a mode has its own bindings for
control tab, for instance. A prominent example is Magit.

So, maybe these bindings should not be done?

In GNU Emacs 30.0.50 (build 1, x86_64-apple-darwin23.3.0, NS
 appkit-2487.40 Version 14.3.1 (Build 23D60)) of 2024-03-05 built on
 Pro.fritz.box
Repository revision: a3d7092114db09fee392ccc8187fde03376f2089
Repository branch: HEAD
Windowing system distributor 'Apple', version 10.3.2487
System Description:  macOS 14.3.1

bug#69578: 30.0.50; tab-bar-mode binding of (control tab) not always useful

[Juri Linkov]

> tab-bar--define-keys makes bindings for TAG like this:
>
>   (unless (global-key-binding [(control tab)])
>     (global-set-key [(control tab)] #'tab-next))
>   (unless (global-key-binding [(control shift tab)])
>     (global-set-key [(control shift tab)] #'tab-previous))
>   (unless (global-key-binding [(control shift iso-lefttab)])
>     (global-set-key [(control shift iso-lefttab)] #'tab-previous))
>
> These bindings stop taking effect if a mode has its own bindings for
> control tab, for instance. A prominent example is Magit.

The developers of Org mode took courage and
replaced their C-TAB bindings with C-c C-TAB:
https://lists.gnu.org/archive/html/bug-gnu-emacs/2020-09/msg00341.html
The developers of Magit could do the same.

bug#69578: 30.0.50; tab-bar-mode binding of (control tab) not always useful

[Gerd Möllmann]

Juri Linkov <juri@linkov.net> writes:

>> tab-bar--define-keys makes bindings for TAG like this:
>>
>>   (unless (global-key-binding [(control tab)])
>>     (global-set-key [(control tab)] #'tab-next))
>>   (unless (global-key-binding [(control shift tab)])
>>     (global-set-key [(control shift tab)] #'tab-previous))
>>   (unless (global-key-binding [(control shift iso-lefttab)])
>>     (global-set-key [(control shift iso-lefttab)] #'tab-previous))
>>
>> These bindings stop taking effect if a mode has its own bindings for
>> control tab, for instance. A prominent example is Magit.
>
> The developers of Org mode took courage and
> replaced their C-TAB bindings with C-c C-TAB:
> https://lists.gnu.org/archive/html/bug-gnu-emacs/2020-09/msg00341.html
> The developers of Magit could do the same.

I've submitted https://github.com/magit/magit/issues/5106 to the Magit
project.

bug#69578: 30.0.50; tab-bar-mode binding of (control tab) not always useful

[Eli Zaretskii]

> Cc: 69578@debbugs.gnu.org
> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Date: Sun, 10 Mar 2024 06:31:48 +0100
> 
> Juri Linkov <juri@linkov.net> writes:
> 
> >> tab-bar--define-keys makes bindings for TAG like this:
> >>
> >>   (unless (global-key-binding [(control tab)])
> >>     (global-set-key [(control tab)] #'tab-next))
> >>   (unless (global-key-binding [(control shift tab)])
> >>     (global-set-key [(control shift tab)] #'tab-previous))
> >>   (unless (global-key-binding [(control shift iso-lefttab)])
> >>     (global-set-key [(control shift iso-lefttab)] #'tab-previous))
> >>
> >> These bindings stop taking effect if a mode has its own bindings for
> >> control tab, for instance. A prominent example is Magit.
> >
> > The developers of Org mode took courage and
> > replaced their C-TAB bindings with C-c C-TAB:
> > https://lists.gnu.org/archive/html/bug-gnu-emacs/2020-09/msg00341.html
> > The developers of Magit could do the same.
> 
> I've submitted https://github.com/magit/magit/issues/5106 to the Magit
> project.

Thanks, should we therefore close this bug?

bug#69578: 30.0.50; tab-bar-mode binding of (control tab) not always useful

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks, should we therefore close this bug?

I'll close it in a second, thanks.

bug#69578: 30.0.50; tab-bar-mode binding of (control tab) not always useful

[Jonas Bernoulli]

> The developers of Org mode took courage and replaced their C-TAB
> bindings with C-c C-TAB:

It indeed takes courage to change key bindings; there will always be
a few users who get very upset by such a change.

> These bindings stop taking effect if a mode has its own bindings for
> control tab, for instance. A prominent example is Magit.

I have addressed this in Magit as described in this commit message:

magit-section-cycle: Pivot to tab-next if there is a binding conflict

If `tab-bar-mode' is enable, then "C-<tab>" is bound to `tab-next'
in the global map.  That conflicts with our local (and much older)
binding for `magit-section-cycle'.

Address this conflict by teaching `magit-section-cycle' to pivot to
`tab-next', but only if `tab-bar-mode' is enabled.  That way, users
who do not use `tab-bar-mode' (i.e., the majority), are not affected
by this unfortunate conflict.

`tab-bar-mode' users will have to get used to the much less convenient
"C-c TAB" binding to cycle section visibility.  Alternatively they can
advice `tab-bar--define-keys' to bind another key to `tab-next'.  It
would be nice if `tab-bar-mode', instead of modifying the global map,
used a mode map, and thus didn't make it so very hard to change its
key bindings.

> So, maybe these bindings should not be done?

Too late for that now, but they should definitely be done in a way that
users can customize, without having to advice tab-bar--define-keys (and
tab-bar--undefine-keys).  Maybe by using a mode map?  If there is some
reason this cannot be done, then maybe a "dummy keymap" could be used?
(User could then manipulate the fake tab-bar-mode-map like any keymap,
but the bindings it contains would then somehow be "transplanted" into
the global map by tab-bar--define-keys.)

>> Thanks, should we therefore close this bug?
>
> I'll close it in a second, thanks.

This seems premature, making it so hard for users to change the tab-bar
bindings seems like something that needs to be fixed.

bug#69578: 30.0.50; tab-bar-mode binding of (control tab) not always useful

[Gerd Möllmann]

Jonas Bernoulli <jonas@bernoul.li> writes:

>>> Thanks, should we therefore close this bug?
>>
>> I'll close it in a second, thanks.
>
> This seems premature, making it so hard for users to change the tab-bar
> bindings seems like something that needs to be fixed.

No problem, I've re-opened it. Thanks, Jonas!

bug#69578: 30.0.50; tab-bar-mode binding of (control tab) not always useful

[Gerd Möllmann]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> No problem, I've re-opened it. Thanks, Jonas!

At least I tried, but reopen doesn't seem to take effect.

bug#69578: 30.0.50; tab-bar-mode binding of (control tab) not always useful

[Eli Zaretskii]

> Cc: 69578@debbugs.gnu.org
> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Date: Mon, 11 Mar 2024 16:02:26 +0100
> 
> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> 
> > No problem, I've re-opened it. Thanks, Jonas!
> 
> At least I tried, but reopen doesn't seem to take effect.

It did.  The bug is now open.

bug#69578: 30.0.50; tab-bar-mode binding of (control tab) not always useful

[Juri Linkov]

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

>> So, maybe these bindings should not be done?
>
> Too late for that now, but they should definitely be done in a way that
> users can customize, without having to advice tab-bar--define-keys (and
> tab-bar--undefine-keys).

Thanks for bringing up this issue.  Customization of keys was not supported
since no once asked for it, I guess because tab-bar--define-keys
respected the existing global keybindings before defining own.
But indeed this doesn't work for local keymaps.

> Maybe by using a mode map?  If there is some reason this cannot be
> done, then maybe a "dummy keymap" could be used?  (User could then
> manipulate the fake tab-bar-mode-map like any keymap, but the bindings
> it contains would then somehow be "transplanted" into the global map
> by tab-bar--define-keys.)

Just adding a simple mode map completely makes it customizable
since 'define-minor-mode' is able to pick it by naming convention:

  (defvar tab-bar-mode-map
    (let ((map (make-sparse-keymap)))
      (define-key map [(control tab)] #'tab-next)
      (define-key map [(control shift tab)] #'tab-previous)
      (define-key map [(control shift iso-lefttab)] #'tab-previous)
      map)
    "Tab Bar mode map.")

The rest of complexity in the following patch comes
from the need to address the customization of
'tab-bar-select-tab-modifiers'.

There is a slight backward incompatibility for users who have own
global keybindings for C-TAB.  They will need to unbind these keys
in the new map.  Then this change should be announced in NEWS:

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: tab-bar-mode-map.patch --]
[-- Type: text/x-diff, Size: 4358 bytes --]

diff --git a/etc/NEWS b/etc/NEWS
index 19cd170e5c7..0aab2d04ca2 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -285,6 +285,11 @@ selected or deselected at the end of executing the current command.
 
 ** Tab Bars and Tab Lines
 
+---
+*** New keymap 'tab-bar-mode-map'.
+If you have global keybinding for 'C-TAB', then you might want
+to unbind the same keybinding in 'tab-bar-mode-map'.
+
 ---
 *** New user option 'tab-bar-tab-name-format-functions'.
 It can be used to add, remove and reorder functions that change
diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index 61efa332e0b..07470f072e1 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -104,10 +104,11 @@ tab-bar-select-tab-modifiers
               (const alt))
   :initialize #'custom-initialize-default
   :set (lambda (sym val)
+         (when tab-bar-mode
+           (tab-bar--undefine-keys))
          (set-default sym val)
          ;; Reenable the tab-bar with new keybindings
          (when tab-bar-mode
-           (tab-bar--undefine-keys)
            (tab-bar--define-keys)))
   :group 'tab-bar
   :version "27.1")
@@ -115,21 +116,17 @@ tab-bar-select-tab-modifiers
 (defun tab-bar--define-keys ()
   "Install key bindings to switch between tabs if so configured."
   (when tab-bar-select-tab-modifiers
-    (global-set-key (vector (append tab-bar-select-tab-modifiers (list ?0)))
-                    #'tab-recent)
+    (define-key tab-bar-mode-map
+                (vector (append tab-bar-select-tab-modifiers (list ?0)))
+                #'tab-recent)
     (dotimes (i 8)
-      (global-set-key (vector (append tab-bar-select-tab-modifiers
-                                      (list (+ i 1 ?0))))
-                      #'tab-bar-select-tab))
-    (global-set-key (vector (append tab-bar-select-tab-modifiers (list ?9)))
-                    #'tab-last))
-  ;; Don't override user customized key bindings
-  (unless (global-key-binding [(control tab)])
-    (global-set-key [(control tab)] #'tab-next))
-  (unless (global-key-binding [(control shift tab)])
-    (global-set-key [(control shift tab)] #'tab-previous))
-  (unless (global-key-binding [(control shift iso-lefttab)])
-    (global-set-key [(control shift iso-lefttab)] #'tab-previous))
+      (define-key tab-bar-mode-map
+                  (vector (append tab-bar-select-tab-modifiers
+                                  (list (+ i 1 ?0))))
+                  #'tab-bar-select-tab))
+    (define-key tab-bar-mode-map
+                (vector (append tab-bar-select-tab-modifiers (list ?9)))
+                #'tab-last))
 
   ;; Replace default value with a condition that supports displaying
   ;; global-mode-string in the tab bar instead of the mode line.
@@ -144,12 +141,18 @@ tab-bar--define-keys
 
 (defun tab-bar--undefine-keys ()
   "Uninstall key bindings previously bound by `tab-bar--define-keys'."
-  (when (eq (global-key-binding [(control tab)]) 'tab-next)
-    (global-unset-key [(control tab)]))
-  (when (eq (global-key-binding [(control shift tab)]) 'tab-previous)
-    (global-unset-key [(control shift tab)]))
-  (when (eq (global-key-binding [(control shift iso-lefttab)]) 'tab-previous)
-    (global-unset-key [(control shift iso-lefttab)])))
+  (when tab-bar-select-tab-modifiers
+    (define-key tab-bar-mode-map
+                (vector (append tab-bar-select-tab-modifiers (list ?0)))
+                nil)
+    (dotimes (i 8)
+      (define-key tab-bar-mode-map
+                  (vector (append tab-bar-select-tab-modifiers
+                                  (list (+ i 1 ?0))))
+                  nil))
+    (define-key tab-bar-mode-map
+                (vector (append tab-bar-select-tab-modifiers (list ?9)))
+                nil)))
 
 (defun tab-bar--load-buttons ()
   "Load the icons for the tab buttons."
@@ -239,6 +242,14 @@ tab-bar--update-tab-bar-lines
                       (if (and tab-bar-mode (eq tab-bar-show t)) 1 0))
                 (assq-delete-all 'tab-bar-lines default-frame-alist)))))
 
+(defvar tab-bar-mode-map
+  (let ((map (make-sparse-keymap)))
+    (define-key map [(control tab)] #'tab-next)
+    (define-key map [(control shift tab)] #'tab-previous)
+    (define-key map [(control shift iso-lefttab)] #'tab-previous)
+    map)
+  "Tab Bar mode map.")
+
 (define-minor-mode tab-bar-mode
   "Toggle the tab bar in all graphical frames (Tab Bar mode)."
   :global t

bug#69578: 30.0.50; tab-bar-mode binding of (control tab) not always useful

[Juri Linkov]

> There is a slight backward incompatibility for users who have own
> global keybindings for C-TAB.  They will need to unbind these keys
> in the new map.  Then this change should be announced in NEWS:
>
> diff --git a/etc/NEWS b/etc/NEWS
> +---
> +*** New keymap 'tab-bar-mode-map'.
> +If you have global keybinding for 'C-TAB', then you might want
> +to unbind the same keybinding in 'tab-bar-mode-map'.

I still feel uneasy about this backward incompatibility
since users will suddenly get tab-switching on C-TAB
when they use another global keybinding for C-TAB.
So we need to rethink this change.

bug#69578: 30.0.50; tab-bar-mode binding of (control tab) not always useful

[Juri Linkov]

close 69578 30.0.50
thanks

>> There is a slight backward incompatibility for users who have own
>> global keybindings for C-TAB.  They will need to unbind these keys
>> in the new map.  Then this change should be announced in NEWS:
>>
>> diff --git a/etc/NEWS b/etc/NEWS
>> +---
>> +*** New keymap 'tab-bar-mode-map'.
>> +If you have global keybinding for 'C-TAB', then you might want
>> +to unbind the same keybinding in 'tab-bar-mode-map'.
>
> I still feel uneasy about this backward incompatibility
> since users will suddenly get tab-switching on C-TAB
> when they use another global keybinding for C-TAB.
> So we need to rethink this change.

Ok, now pushed a backward-compatibile fix.
So can close now.