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

From: Juri Linkov <[email-masked]>
Date: Mon, 29 Jun 2020 02:18:57 +0300

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