bug#42052: 28.0.50; tab-bar-mode should be frame-local

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#42052: 28.0.50; tab-bar-mode should be frame-local

Juri Linkov-2
> When setting tab-bar-show to `1`, I expect the tab bar to only show if
> there is more than one tab in the frame.
>
> This behaves as expected when there is only one frame, but if I have
> multiple frames open, then if any one of them has more than one tab, it
> turns on tab-bar-mode, which then makes any other frame show the tab
> bar, even if they only have one tab in them.
>
> Would it be possible to have tab-bar-mode showing be frame-local?

Thanks for the request, I see this means a request for a new feature of
enabling the tab-bar separately on each frame independently from
other frames.

When some time ago I asked on emacs-devel about the need for a such feature,
proposing to add a new function global-tab-bar-mode, no one wanted it.
But nevertheless it could be created now anyway.

If someone will point out an example of frame-local modes,
this could help in implementing the same for tab-bar-mode.

I can find only toggle-tool-bar-mode-from-frame, but actually
it's still not frame-local.



Reply | Threaded
Open this post in threaded view
|

bug#42052: 28.0.50; tab-bar-mode should be frame-local

James N. V. Cash
Juri Linkov <[hidden email]> writes:

>> When setting tab-bar-show to `1`, I expect the tab bar to only show if
>> there is more than one tab in the frame.
>>
> If someone will point out an example of frame-local modes,
> this could help in implementing the same for tab-bar-mode.

Looking at how the tab-bar-mode currently works, the thing that seems
simplest to me would be to change the various functions that
conditionally turn tab-bar-mode on/off (e.g. in tab-bar-new-tab-to)
to have an additional check if (natnump tab-bar-show), in which case
instead of calling (tab-bar-mode 1) or -1, setting the frame parameter
tab-bar-lines for that particular frame to be 1 or 0, as appropriate.

The wrinkle would be, I suppose, having to remove all the frame-local
settings if tab-bar-show changes, but presumably that wouldn't be
happening too often.

A frame-local toggling of the tab bar could work the same way.

If that makes sense (i.e. having the setting be frame local only when
tab-bar-show is 1), I can try submitting a patch later today.

James Cash



Reply | Threaded
Open this post in threaded view
|

bug#42052: 28.0.50; tab-bar-mode should be frame-local

James N. V. Cash
James N. V. Cash <[hidden email]> writes:

> Juri Linkov <[hidden email]> writes:
>
>>> When setting tab-bar-show to `1`, I expect the tab bar to only show if
>>> there is more than one tab in the frame.
>>>
>> If someone will point out an example of frame-local modes,
>> this could help in implementing the same for tab-bar-mode.
>
> Looking at how the tab-bar-mode currently works, the thing that seems
> simplest to me would be to change the various functions that
> conditionally turn tab-bar-mode on/off (e.g. in tab-bar-new-tab-to)
> to have an additional check if (natnump tab-bar-show), in which case
> instead of calling (tab-bar-mode 1) or -1, setting the frame parameter
> tab-bar-lines for that particular frame to be 1 or 0, as appropriate.
>
> The wrinkle would be, I suppose, having to remove all the frame-local
> settings if tab-bar-show changes, but presumably that wouldn't be
> happening too often.
>
> A frame-local toggling of the tab bar could work the same way.
>
> If that makes sense (i.e. having the setting be frame local only when
> tab-bar-show is 1), I can try submitting a patch later today.

Here's a simple patch I made that seems to act more like I expect

--- /home/james/src/emacs/lisp/tab-bar.el 2020-06-20 10:16:57.177037735 -0400
+++ tab-bar.el  2020-06-28 15:40:38.711147160 -0400
@@ -799,11 +799,16 @@
       (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)
-                   (and (natnump tab-bar-show)
-                        (> (length tabs) tab-bar-show))))
+    (cond
+     (tab-bar-mode)
+     ((eq tab-bar-show t)
       (tab-bar-mode 1))
+     ((and (natnump tab-bar-show)
+           (> (length tabs) tab-bar-show)
+           (zerop (frame-parameter nil 'tab-bar-lines)))
+      (progn
+        (message "show")
+        (set-frame-parameter nil 'tab-bar-lines 1))))

     (force-mode-line-update)
     (unless tab-bar-mode
@@ -936,10 +941,10 @@
                 tab-bar-closed-tabs)
           (set-frame-parameter nil 'tabs (delq close-tab tabs)))

-        (when (and tab-bar-mode
+        (when (and (not (zerop (frame-parameter nil 'tab-bar-lines)))
                    (and (natnump tab-bar-show)
                         (<= (length tabs) tab-bar-show)))
-          (tab-bar-mode -1))
+          (set-frame-parameter nil 'tab-bar-lines 0))

         (force-mode-line-update)
         (unless tab-bar-mode
@@ -975,10 +980,12 @@
           (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
-                 (and (natnump tab-bar-show)
-                      (<= 1 tab-bar-show)))
-        (tab-bar-mode -1))
+      (when (and (not (zerop (frame-parameter nil 'tab-bar-lines)))
+                   (and (natnump tab-bar-show)
+                        (<= (length tabs) tab-bar-show)))
+          (modify-frame-parameters
+           nil
+           (assq-delete-all 'tab-bar-lines (frame-parameters nil))))

       (force-mode-line-update)
       (unless tab-bar-mode



Reply | Threaded
Open this post in threaded view
|

bug#42052: 28.0.50; tab-bar-mode should be frame-local

Juri Linkov-2
>> Looking at how the tab-bar-mode currently works, the thing that seems
>> simplest to me would be to change the various functions that
>> conditionally turn tab-bar-mode on/off (e.g. in tab-bar-new-tab-to)
>> to have an additional check if (natnump tab-bar-show), in which case
>> instead of calling (tab-bar-mode 1) or -1, setting the frame parameter
>> tab-bar-lines for that particular frame to be 1 or 0, as appropriate.
>>
>> The wrinkle would be, I suppose, having to remove all the frame-local
>> settings if tab-bar-show changes, but presumably that wouldn't be
>> happening too often.
>>
>> A frame-local toggling of the tab bar could work the same way.
>>
>> If that makes sense (i.e. having the setting be frame local only when
>> tab-bar-show is 1), I can try submitting a patch later today.
>
> Here's a simple patch I made that seems to act more like I expect

Thanks for the patch.  One problem is that directly changing the
frame parameter tab-bar-lines avoids performing some other
settings in tab-bar-mode such as loading button images and changing
some keybindings.  OTOH, these keybindings can't be frame-local anyway,
so maybe this is not important for the case when tab-bar-show is 1.

> +     ((and (natnump tab-bar-show)
> +           (> (length tabs) tab-bar-show)
> +           (zerop (frame-parameter nil 'tab-bar-lines)))
> +      (progn
> +        (message "show")
> +        (set-frame-parameter nil 'tab-bar-lines 1))))

Please don't forget to remove this debugging message in the final patch.

> @@ -975,10 +980,12 @@
>            (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
> -                 (and (natnump tab-bar-show)
> -                      (<= 1 tab-bar-show)))
> -        (tab-bar-mode -1))
> +      (when (and (not (zerop (frame-parameter nil 'tab-bar-lines)))
> +                   (and (natnump tab-bar-show)
> +                        (<= (length tabs) tab-bar-show)))
> +          (modify-frame-parameters
> +           nil
> +           (assq-delete-all 'tab-bar-lines (frame-parameters nil))))

Unlike other places that simply use (set-frame-parameter nil 'tab-bar-lines 0)
this code is more complex.  But it seems it should work with just
(set-frame-parameter nil 'tab-bar-lines 0) as well.

BTW, I see a problem in the old code: the variable 'tabs' is not always updated
and sometimes contains obsolete data.  It should help to replace

  (<= (length tabs) tab-bar-show)

with

  (<= (length (funcall tab-bar-tabs-function)) tab-bar-show)

that gets fresh data.



Reply | Threaded
Open this post in threaded view
|

bug#42052: 28.0.50; tab-bar-mode should be frame-local

James N. V. Cash
James N. V. Cash <[hidden email]> writes:

>> Unlike other places that simply use (set-frame-parameter nil 'tab-bar-lines 0)
>> this code is more complex.  But it seems it should work with just
>> (set-frame-parameter nil 'tab-bar-lines 0) as well.
>
> Ah, right. I was thinking that

Oof, sorry, my brain is just melting today.

Meant to write: "I was thinking that removing the tab-bar-lines would
allow switching tab-bar-mode on or off global work properly, but then
looking at the implementation, I see that it sets the frame parameter
explicitly when toggling on/off in any case."