GNU bug report logs - #46299
28.0.50; Value of tab-bar-show not respected in new frames.

Previous Next

Package: emacs;

Reported by: Bastian Beischer <bastian.beischer <at> gmail.com>

Date: Thu, 4 Feb 2021 16:16:02 UTC

Severity: normal

Tags: fixed

Fixed in version 28.0.50

Done: Juri Linkov <juri <at> linkov.net>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 46299 in the body.
You can then email your comments to 46299 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Thu, 04 Feb 2021 16:16:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Bastian Beischer <bastian.beischer <at> gmail.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Thu, 04 Feb 2021 16:16:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beischer <bastian.beischer <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.0.50; Value of tab-bar-show not respected in new frames.
Date: Thu, 04 Feb 2021 17:14:55 +0100
I noticed that with the latest master branch setting tab-bar-show
to "1" does not work work for new frames. On those the tabs are shown
even if tab-bar-show is set to 1. I suppose a hook is needed which
applies the correct setting to the new frame?

This can be reproduced by:

1) "emacs -Q"
   (no tab bar yet)

2) M-x tab-bar-mode
   (tab bar appears in the current frame with one tab)

3) M-x customize-variable tab-bar-show ->
   Set to "When more than one tab".
   (tab bar disappears in the current frame)

4) C-x 5 2
   (tab bar visible in the new frame with one tab)


In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.24)
 of 2021-02-03 built on bastian-desktop
Repository revision: 20e48b6fd6cade60e468140a66127d326abfb8ff
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12010000
System Description: Arch Linux

Configured using:
 'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/var --mandir=/usr/share/man --with-gameuser=:games
 --with-sound=alsa --with-modules --without-gconf --without-gsettings
 --enable-link-time-optimization --with-x-toolkit=gtk3 --without-xaw3d
 --without-cairo --without-compress-install 'CFLAGS=-march=native -O2
 -pipe -fno-plt -flto -fuse-linker-plugin -flto -fuse-linker-plugin'
 CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now'

Configured features:
ACL DBUS FREETYPE GIF GLIB GMP GNUTLS GPM HARFBUZZ JPEG JSON LCMS2
LIBOTF LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY INOTIFY PDUMPER PNG
RSVG SOUND THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE XFT XIM XPM GTK3
ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  recentf-mode: t
  whitespace-mode: t
  subword-mode: t
  helm-fuzzier-mode: t
  async-bytecomp-package-mode: t
  helm-flx-mode: t
  projectile-mode: t
  yas-global-mode: t
  yas-minor-mode: t
  company-mode: t
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  flx-ido-mode: t
  ido-everywhere: t
  shell-dirtrack-mode: t
  show-paren-mode: t
  global-hi-lock-mode: t
  hi-lock-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  prettify-symbols-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t
  hs-minor-mode: t

Load-path shadows:
/home/beischer/.emacs.d/elpa/cmake-mode-20210104.1831/cmake-mode hides /usr/share/emacs/site-lisp/cmake-mode
~/.emacs.d/lisp/buff-menu+ hides /usr/share/emacs/site-lisp/various/buff-menu+
~/.emacs.d/lisp/my-term hides /usr/share/emacs/site-lisp/various/my-term
~/.emacs.d/lisp/buff-menu hides /usr/share/emacs/site-lisp/various/buff-menu
~/.emacs.d/lisp/qt-pro hides /usr/share/emacs/site-lisp/various/qt-pro
~/.emacs.d/lisp/buff-menu hides /usr/share/emacs/28.0.50/lisp/buff-menu

Features:
(shadow sort mail-extr emacsbug sendmail recentf help-fns radix-tree
magit-extras bug-reference eterm-256color xterm-color mule-util log-view
vc-mtn vc-hg vc-bzr vc-src vc-sccs vc-svn vc-rcs vc vc-dispatcher
misearch multi-isearch mm-archive gnutls url-cache debbugs-gnu debbugs
soap-client url-http url-auth url-gw rng-xsd rng-dt rng-util xsd-regexp
xml winner helm-ring helm-elisp helm-files helm-buffers helm-occur
helm-tags helm-locate helm-grep helm-regexp helm-eval edebug backtrace
helm-info helm-types helm-utils helm-help ido-completing-read+ memoize
cus-edit minibuf-eldef whitespace cap-words superword subword
company-oddmuse company-keywords company-etags etags fileloop generator
company-gtags company-dabbrev-code company-dabbrev company-files
company-clang company-capf company-cmake company-semantic
company-template company-bbdb company-edbi edbi sql view company-jedi
jedi-core python-environment epc ctable concurrent deferred
tree-sitter-langs tree-sitter-langs-build tar-mode arc-mode archive-mode
pp tree-sitter-hl tree-sitter tree-sitter-load tree-sitter-cli tsc
tsc-dyn tsc-dyn-get dired-aux tsc-obsolete ccls ccls-member-hierarchy
ccls-inheritance-hierarchy ccls-call-hierarchy ccls-tree ccls-code-lens
ccls-semantic-highlight ccls-common lsp-ui lsp-ui-flycheck lsp-ui-doc
goto-addr lsp-ui-imenu lsp-ui-peek lsp-ui-sideline flycheck lsp-ui-util
lsp-mode lsp-protocol xref project tree-widget wid-edit spinner
network-stream nsm markdown-mode lv inline ht ewoc dash-functional
bindat cmake-project helm-fuzzier helm async-bytecomp
helm-global-bindings helm-easymenu helm-source eieio-compat
helm-multi-match helm-lib async helm-flx tempo xml-parse doxymacs f s
tramp-cache projectile ibuf-ext ibuffer ibuffer-loaddefs dropdown-list
yasnippet-snippets yasnippet my-term vterm face-remap term disp-table
ehelp vterm-module term/xterm xterm cmake-mode rst qt-pro pastebin
ams-meeting calc-mouse calc-yank calc-ext calc calc-loaddefs calc-macs
realgud realgud-zshdb realgud:zshdb-track-mode realgud:zshdb-core
realgud:zshdb-init realgud-trepan3k realgud:trepan3k-track-mode
realgud:trepan3k-core realgud:trepan3k-init realgud-trepan2
realgud:trepan2-track-mode realgud:trepan2-core realgud:trepan2-init
realgud-trepanpl realgud:trepanpl-track-mode realgud:trepanpl-core
realgud:trepanpl-init realgud-trepanjs realgud:trepanjs-track-mode
realgud:trepanjs-core realgud:trepanjs-init realgud-lang-js
realgud-trepan realgud:trepan-track-mode realgud:trepan-core
realgud:trepan-init realgud-remake realgud:remake-track-mode
realgud:remake-core realgud:remake-init realgud-rdebug
realgud-rdebug-track-mode realgud-rdebug-core realgud-rdebug-init
realgud-lang-ruby realgud-perldb realgud:perldb-track-mode
realgud:perldb-core realgud:perldb-init realgud-lang-perl realgud-pdb
realgud:pdb-track-mode realgud:pdb-core realgud:pdb-init
realgud-lang-python python tramp-sh realgud-kshdb
realgud:kshdb-track-mode realgud:kshdb-core realgud:kshdb-init
realgud-gub realgud:gub-track-mode realgud:gub-core realgud:gub-init
realgud-gdb realgud:gdb-track-mode realgud:gdb-init realgud:gdb-core
realgud-bashdb realgud:bashdb-track-mode realgud:bashdb-core
realgud:bashdb-init realgud-lang-posix-shell realgud:run
realgud-locals-mode realgud-breakpoint-mode realgud-backtrack-mode
realgud-track-mode realgud-backtrace-mode realgud-attach
realgud-lang-java realgud-track realgud-shortkey realgud-menu
realgud-eval realgud-cmds realgud-send realgud-window realgud-utils
eshell realgud-init realgud-file realgud-core realgud-reset
realgud-buffer-helper realgud-buffer-breakpoint realgud-buffer-backtrace
realgud-locals realgud-buffer-locals realgud-buffer-command
realgud-buffer-info realgud-lochist org ob ob-tangle ob-ref ob-lob
ob-table ob-exp org-macro org-footnote org-src ob-comint org-pcomplete
org-list org-faces org-entities noutline outline org-version
ob-emacs-lisp ob-core ob-eval org-table ol org-keys org-compat org-macs
org-loaddefs find-func cal-menu calendar cal-loaddefs realgud-bp
realgud-bp-image-data realgud-lang esh-mode esh-cmd esh-ext esh-opt
esh-proc esh-io esh-arg esh-module esh-groups esh-util realgud-loc
realgud-buffer-source realgud-key key realgud-follow realgud-fringe
realgud-helper loc-changes realgud-regexp realgud-custom load-relative
ivy delsel ivy-faces ivy-overlay colir color company hide-lines
buff-menu+ magit-topgit magit-submodule magit-obsolete magit-popup
magit-blame magit-stash magit-reflog magit-bisect magit-push magit-pull
magit-fetch magit-clone magit-remote magit-commit magit-sequence
magit-notes magit-worktree magit-tag magit-merge magit-branch
magit-reset magit-files magit-refs magit-status magit magit-repos
magit-apply magit-wip magit-log magit-diff smerge-mode diff git-commit
log-edit message rmc puny rfc822 mml mml-sec epa epg epg-config
gnus-util rmail rmail-loaddefs mm-decode mm-bodies mm-encode mail-parse
rfc2231 rfc2047 rfc2045 mm-util ietf-drums mail-prsvr mailabbrev
mail-utils gmm-utils mailheader pcvs-util add-log magit-core
magit-autorevert autorevert filenotify magit-margin magit-transient
magit-process with-editor server magit-mode transient cl-extra help-mode
magit-git magit-section derived benchmark magit-utils pcase which-func
imenu vc-git diff-mode easy-mmode crm dash hideshow flx-ido advice flx
ido dired-x dired dired-loaddefs cc-mode cc-fonts cc-guess cc-menus
cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs flymake-proc
flymake warnings thingatpt vc-cvs finder-inf edmacro kmacro
emacs-x-theme tramp tramp-loaddefs trampver tramp-integration files-x
tramp-compat shell pcomplete parse-time iso8601 time-date ls-lisp
format-spec paren grep compile text-property-search comint ansi-color
ring linum hi-lock cus-start cus-load tex-site rx
realgud-recursive-autoloads info package easymenu browse-url url
url-proxy url-privacy url-expand url-methods url-history url-cookie
url-domsuf url-util mailcap url-handlers url-parse auth-source cl-seq
eieio eieio-core cl-macs eieio-loaddefs password-cache json subr-x map
url-vars seq byte-opt gv bytecomp byte-compile cconv cl-loaddefs cl-lib
iso-transl tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors frame minibuffer cl-generic
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads dbusbind
inotify lcms2 dynamic-setting font-render-setting move-toolbar gtk
x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 1238442 1372937)
 (symbols 48 61528 25)
 (strings 32 264002 435110)
 (string-bytes 1 7786784)
 (vectors 16 91754)
 (vector-slots 8 1303782 321774)
 (floats 8 4099 2219)
 (intervals 56 26046 11337)
 (buffers 984 25))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Fri, 05 Feb 2021 09:23:01 GMT) Full text and rfc822 format available.

Message #8 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Bastian Beischer <bastian.beischer <at> gmail.com>
Cc: 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Fri, 05 Feb 2021 10:54:03 +0200
[Message part 1 (text/plain, inline)]
> I noticed that with the latest master branch setting tab-bar-show
> to "1" does not work work for new frames. On those the tabs are shown
> even if tab-bar-show is set to 1.

Thanks for finding a case that is still unhandled.

> I suppose a hook is needed which applies the correct setting
> to the new frame?

Generally, Emacs core packages should avoid adding own code
to hooks, because hooks are intended mostly for users, such as
for example, configuring to enable tab-bar selectively:

  (add-hook 'after-make-frame-functions 'toggle-frame-tab-bar)

Fortunately, frames provide a better way to set their default values
with default-frame-alist, that tab-bar-mode already modifies.
So doing something similar fixes the problem:

[tab-bar-show-default-frame-alist.patch (text/x-diff, inline)]
diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index 6720d82b47..0cf74a7833 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -252,6 +252,12 @@ tab-bar-show
          (set-default sym val)
          ;; Preload button images
          (tab-bar-mode 1)
+         ;; New frames have only one tab, so hide it by default
+         (when (eq val 1)
+           (setq default-frame-alist
+              (cons (cons 'tab-bar-lines 0)
+                    (assq-delete-all 'tab-bar-lines
+                                     default-frame-alist))))
          ;; Then handle each frame individually
          (dolist (frame (frame-list))
            (set-frame-parameter

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Fri, 05 Feb 2021 10:11:01 GMT) Full text and rfc822 format available.

Message #11 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50;
 Value of tab-bar-show not respected in new frames.
Date: Fri, 5 Feb 2021 11:10:01 +0100
Hey Juri,

On Fri, Feb 5, 2021 at 10:22 AM Juri Linkov <juri <at> linkov.net> wrote:
>
> > I noticed that with the latest master branch setting tab-bar-show
> > to "1" does not work work for new frames. On those the tabs are shown
> > even if tab-bar-show is set to 1.
>
> Thanks for finding a case that is still unhandled.
>
> > I suppose a hook is needed which applies the correct setting
> > to the new frame?
>
> Generally, Emacs core packages should avoid adding own code
> to hooks, because hooks are intended mostly for users, such as
> for example, configuring to enable tab-bar selectively:
>
>   (add-hook 'after-make-frame-functions 'toggle-frame-tab-bar)
>
> Fortunately, frames provide a better way to set their default values
> with default-frame-alist, that tab-bar-mode already modifies.

Oh I see. Yes that would also work. Although I would say that in
general it should be more robust to have a dynamic function which
counts the numbers of tabs and adapts the number of tab-bar-lines
according to the value of tab-bar show. Is it guaranteed that new
frames only have one tab?

> So doing something similar fixes the problem:

That patch looks fine, except that my bug report translates equally to
the case when tab-bar-show is nil, so (eq val 1) should be adapted to
catch both "1" and "nil".

Thanks for your help!
Bastian




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Fri, 05 Feb 2021 14:13:02 GMT) Full text and rfc822 format available.

Message #14 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50;
 Value of tab-bar-show not respected in new frames.
Date: Fri, 5 Feb 2021 15:11:39 +0100
Hello Juri,

I now installed your patch and I don't think it is complete yet.

1) Is the :set function actually used the next time emacs starts after
customizations have been written to .emacs and variables are
initialized to customized values using (custom-set-variables ...)? I
don't think it is, right?

2) Switching tab-bar-mode on and off seems to overwrite the
tab-bar-lines information in default-frame-alist:

    ;; If the user has given `default-frame-alist' a `tab-bar-lines'
    ;; parameter, replace it.
    (if (assq 'tab-bar-lines default-frame-alist)
        (setq default-frame-alist
              (cons (cons 'tab-bar-lines val)
                    (assq-delete-all 'tab-bar-lines
                                     default-frame-alist)))))

This code should depend on the value of tab-bar-show, right?

Cheers
Bastian




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Sat, 06 Feb 2021 12:18:01 GMT) Full text and rfc822 format available.

Message #17 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50;
 Value of tab-bar-show not respected in new frames.
Date: Sat, 6 Feb 2021 13:16:54 +0100
[Message part 1 (text/plain, inline)]
Hello Juri,

I added code to make the frame setting of tab-bar-lines as well as the
default-frame-alist value dependent on tab-bar-show to the
tab-bar-mode function.
I think with this the part to which sets frame parameters in
tab-bar-show :set is not needed because (tab-bar-mode 1) is called
anyway, which already does everything.

What do you think about the attached patch?

Cheers
Bastian

On Fri, Feb 5, 2021 at 3:11 PM Bastian Beranek
<bastian.beischer <at> gmail.com> wrote:
>
> Hello Juri,
>
> I now installed your patch and I don't think it is complete yet.
>
> 1) Is the :set function actually used the next time emacs starts after
> customizations have been written to .emacs and variables are
> initialized to customized values using (custom-set-variables ...)? I
> don't think it is, right?
>
> 2) Switching tab-bar-mode on and off seems to overwrite the
> tab-bar-lines information in default-frame-alist:
>
>     ;; If the user has given `default-frame-alist' a `tab-bar-lines'
>     ;; parameter, replace it.
>     (if (assq 'tab-bar-lines default-frame-alist)
>         (setq default-frame-alist
>               (cons (cons 'tab-bar-lines val)
>                     (assq-delete-all 'tab-bar-lines
>                                      default-frame-alist)))))
>
> This code should depend on the value of tab-bar-show, right?
>
> Cheers
> Bastian
[tab-bar.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Sun, 07 Feb 2021 19:50:01 GMT) Full text and rfc822 format available.

Message #20 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Bastian Beranek <bastian.beischer <at> gmail.com>
Cc: 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Sun, 07 Feb 2021 21:05:12 +0200
> I added code to make the frame setting of tab-bar-lines as well as the
> default-frame-alist value dependent on tab-bar-show to the
> tab-bar-mode function.
> I think with this the part to which sets frame parameters in
> tab-bar-show :set is not needed because (tab-bar-mode 1) is called
> anyway, which already does everything.
>
> What do you think about the attached patch?

I think your earlier idea was better - to have a dynamic function which
counts the numbers of tabs and adapts the number of tab-bar-lines
according to the value of tab-bar-show.

Then such a function could be called from many places:
- tab-bar-mode
- tab-bar-show :set
- from the end of tab-bar-new-tab-to
- from the end of tab-bar-close-tab
- from the end of tab-bar-close-other-tabs

to sync the actual tabs with the value of tab-bar-show.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Sun, 07 Feb 2021 23:04:01 GMT) Full text and rfc822 format available.

Message #23 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50;
 Value of tab-bar-show not respected in new frames.
Date: Mon, 8 Feb 2021 00:03:26 +0100
[Message part 1 (text/plain, inline)]
Hello Juri,

On Sun, Feb 7, 2021 at 8:48 PM Juri Linkov <juri <at> linkov.net> wrote:
>
> > I added code to make the frame setting of tab-bar-lines as well as the
> > default-frame-alist value dependent on tab-bar-show to the
> > tab-bar-mode function.
> > I think with this the part to which sets frame parameters in
> > tab-bar-show :set is not needed because (tab-bar-mode 1) is called
> > anyway, which already does everything.
> >
> > What do you think about the attached patch?
>
> I think your earlier idea was better - to have a dynamic function which
> counts the numbers of tabs and adapts the number of tab-bar-lines
> according to the value of tab-bar-show.
>
> Then such a function could be called from many places:
> - tab-bar-mode
> - tab-bar-show :set
> - from the end of tab-bar-new-tab-to
> - from the end of tab-bar-close-tab
> - from the end of tab-bar-close-other-tabs
>
> to sync the actual tabs with the value of tab-bar-show.

Makes sense! How about the attached?

Cheers
Bastian
[tab-bar_v2.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Mon, 08 Feb 2021 17:52:02 GMT) Full text and rfc822 format available.

Message #26 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50;
 Value of tab-bar-show not respected in new frames.
Date: Mon, 8 Feb 2021 18:50:40 +0100
[Message part 1 (text/plain, inline)]
Hello Juri,

I have tried to clean up my patch a bit more, please see the attached version 3.

Best
Bastian
[tab-bar_v3.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Mon, 08 Feb 2021 18:24:02 GMT) Full text and rfc822 format available.

Message #29 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Bastian Beranek <bastian.beischer <at> gmail.com>
Cc: 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Mon, 08 Feb 2021 20:19:12 +0200
Hello Bastian,

> I have tried to clean up my patch a bit more, please see the attached version 3.

Thanks, everything looks right, except one thing that you removed
the most important rule:

> -    (cond
> -     ((eq tab-bar-show t)
> -      (tab-bar-mode 1))

This is the core reason of existence of tab-bar-show separate from tab-bar-mode:
when the user creates a new tab, the tab-bar should be activated,
unless the user has customized tab-bar-show to nil.

Actually, a more proper condition should be:

  (when tab-bar-show
    (tab-bar-mode 1))

and then after that you can use the remaining code:

> +    ;; Recalculate tab-bar-lines and update frames
> +    (tab-bar--update-tab-bar-lines (selected-frame))
> +    (when tab-bar-mode
> +      (tab-bar--load-buttons)
> +      (tab-bar--define-keys))

In tab-bar-close-other-tabs:

> +      ;; Recalculate tab-bar-lines and update frames
> +      (tab-bar--update-tab-bar-lines)

It could affect only the selected frame too, i.e.:
(tab-bar--update-tab-bar-lines (selected-frame))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Mon, 08 Feb 2021 19:06:01 GMT) Full text and rfc822 format available.

Message #32 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50;
 Value of tab-bar-show not respected in new frames.
Date: Mon, 8 Feb 2021 20:04:59 +0100
[Message part 1 (text/plain, inline)]
Hello Juri,

On Mon, Feb 8, 2021 at 7:23 PM Juri Linkov <juri <at> linkov.net> wrote:
>
> Hello Bastian,
>
> > I have tried to clean up my patch a bit more, please see the attached version 3.
>
> Thanks, everything looks right, except one thing that you removed
> the most important rule:
>
> > -    (cond
> > -     ((eq tab-bar-show t)
> > -      (tab-bar-mode 1))
>
> This is the core reason of existence of tab-bar-show separate from tab-bar-mode:
> when the user creates a new tab, the tab-bar should be activated,
> unless the user has customized tab-bar-show to nil.
>
> Actually, a more proper condition should be:
>
>   (when tab-bar-show
>     (tab-bar-mode 1))
>
> and then after that you can use the remaining code:
>
> > +    ;; Recalculate tab-bar-lines and update frames
> > +    (tab-bar--update-tab-bar-lines (selected-frame))
> > +    (when tab-bar-mode
> > +      (tab-bar--load-buttons)
> > +      (tab-bar--define-keys))
>
> In tab-bar-close-other-tabs:
>
> > +      ;; Recalculate tab-bar-lines and update frames
> > +      (tab-bar--update-tab-bar-lines)
>
> It could affect only the selected frame too, i.e.:
> (tab-bar--update-tab-bar-lines (selected-frame))

Thank you for your comments. I have updated the patch according to
your suggestions. If it looks fine to you now, could you install it
for me?

Best
Bastian
[tab-bar_v4.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Tue, 09 Feb 2021 08:01:01 GMT) Full text and rfc822 format available.

Message #35 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Bastian Beranek <bastian.beischer <at> gmail.com>, Juri Linkov <juri <at> linkov.net>
Cc: 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Tue, 9 Feb 2021 09:00:44 +0100
> +  "Update frame parameter tab-bar-line.
> +When the optional frame parameter is omitted all frames as well
> +as the default for new frames are updated. Otherwise only the
> +given frame is modified."

This is confusing wrt our parameter/argument terminology.  Please just
say "FRAME" wherever it stands for the function's argument and use
"parameter of FRAME" when you talk about the frame parameter.  Also
please clarify: Does the value of the 'tab-bar-lines' parameter now
stand for the number of (matrix) rows occupied by the tab bar (for
example, 2 if the tab bar wraps once) or the number of frame lines
calculated by dividing the pixel size of the tab bar by the frame's
default line height?

And finally: Why can't we wrap tab bars at tab boundaries so one tab
never spans two lines unless it is too long for the frame (in which case
it should be just truncated IMO).

martin




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Tue, 09 Feb 2021 08:17:02 GMT) Full text and rfc822 format available.

Message #38 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 46299 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#46299: 28.0.50;
 Value of tab-bar-show not respected in new frames.
Date: Tue, 9 Feb 2021 09:15:50 +0100
Hey Martin,

On Tue, Feb 9, 2021 at 9:00 AM martin rudalics <rudalics <at> gmx.at> wrote:
>
>  > +  "Update frame parameter tab-bar-line.
>  > +When the optional frame parameter is omitted all frames as well
>  > +as the default for new frames are updated. Otherwise only the
>  > +given frame is modified."
>
> This is confusing wrt our parameter/argument terminology.  Please just
> say "FRAME" wherever it stands for the function's argument and use
> "parameter of FRAME" when you talk about the frame parameter.

I'll do my best to update the docstring. Note that I do not contribute
to emacs often, nor do I regularly code in elisp, so I'm not familiar
with the conventions and you are encouraged to modify my changes as
you see fit.

I propose to change the docstring as follows:

diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index 9666fb2460..2c3d976f28 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -135,7 +135,7 @@ Possible modifier keys are `control', `meta',
`shift', `hyper', `super' and
                         tab-bar-close-button)))

(defun tab-bar--tab-bar-lines-for-frame (frame)
-  "Compute the correct value of tab-bar-lines for the given frame."
+  "Compute the correct value of tab-bar-lines for FRAME."
  (if (not tab-bar-mode)
      0
    (cond
@@ -145,10 +145,10 @@ Possible modifier keys are `control', `meta',
`shift', `hyper', `super' and
     (t 0))))

(defun tab-bar--update-tab-bar-lines (&optional frame)
-  "Update frame parameter tab-bar-line.
-When the optional frame parameter is omitted all frames as well
-as the default for new frames are updated. Otherwise only the
-given frame is modified."
+  "Update tab-bar-line parameter in frames.
+When the optional parameter FRAME is omitted all frames as well
+as the default for new frames are updated. Otherwise only FRAME
+is modified."
  (if frame
      ;; Set frame parameters for the given frame
      (set-frame-parameter frame 'tab-bar-lines
(tab-bar--tab-bar-lines-for-frame frame))

> Also
> please clarify: Does the value of the 'tab-bar-lines' parameter now
> stand for the number of (matrix) rows occupied by the tab bar (for
> example, 2 if the tab bar wraps once) or the number of frame lines
> calculated by dividing the pixel size of the tab bar by the frame's
> default line height?
>
> And finally: Why can't we wrap tab bars at tab boundaries so one tab
> never spans two lines unless it is too long for the frame (in which case
> it should be just truncated IMO).

As far as I understand, tab-bar-lines is always just 1 or 0, meaning
whether to show the tab-bar at all or not. Maybe it would be better to
just rename the parameter? I guess if that is done then it does not
necessarily need further explanation in docstrings.

>
> martin

Bastian




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Tue, 09 Feb 2021 08:59:02 GMT) Full text and rfc822 format available.

Message #41 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Bastian Beranek <bastian.beischer <at> gmail.com>
Cc: 46299 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Tue, 9 Feb 2021 09:58:39 +0100
> I'll do my best to update the docstring. Note that I do not contribute
> to emacs often, nor do I regularly code in elisp, so I'm not familiar
> with the conventions and you are encouraged to modify my changes as
> you see fit.

Don't worry.  These "-lines" frame parameters are a minefield - we just
should try to clarify things the best possible way while you're working
on it.

> (defun tab-bar--tab-bar-lines-for-frame (frame)
> -  "Compute the correct value of tab-bar-lines for the given frame."
> +  "Compute the correct value of tab-bar-lines for FRAME."

But what "is" the correct value and why and how would we want to
"compute" it?

> As far as I understand, tab-bar-lines is always just 1 or 0, meaning
> whether to show the tab-bar at all or not. Maybe it would be better to
> just rename the parameter? I guess if that is done then it does not
> necessarily need further explanation in docstrings.

That ship has sailed long ago.  Neither the 'menu-bar-lines' nor the
'tool-bar-lines' parameters convey useful information in this regard and
'tab-bar-lines' just follows suit.  Their only practical (and completely
misguided IMHO) purpose is to show the corresponding bar by setting the
parameter to a non-zero value and remove it by setting the parameter to
zero.

Sometimes, as with our native tool bars, their value gives the number of
frame lines occupied by the bar.  And very occasionally setting the
parameter to a non-zero value can have strange effects: On a non-toolkit
build setting menu-bar-lines to 7 will show six blank lines below a
one-line menu bar which does not wrap anyway.

In either case, we can hardly change the names of these frame parameters
because they probably appear in too many applications and init files out
there.  We could state somewhere that these are, in fact, booleans and
should be set and interpreted in that sense.  Even that is not entirely
trivial.

martin




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Tue, 09 Feb 2021 09:24:01 GMT) Full text and rfc822 format available.

Message #44 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 46299 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#46299: 28.0.50;
 Value of tab-bar-show not respected in new frames.
Date: Tue, 9 Feb 2021 10:23:33 +0100
Hello Martin,

On Tue, Feb 9, 2021 at 9:58 AM martin rudalics <rudalics <at> gmx.at> wrote:
>
>  > I'll do my best to update the docstring. Note that I do not contribute
>  > to emacs often, nor do I regularly code in elisp, so I'm not familiar
>  > with the conventions and you are encouraged to modify my changes as
>  > you see fit.
>
> Don't worry.  These "-lines" frame parameters are a minefield - we just
> should try to clarify things the best possible way while you're working
> on it.

I see.

>
>  > (defun tab-bar--tab-bar-lines-for-frame (frame)
>  > -  "Compute the correct value of tab-bar-lines for the given frame."
>  > +  "Compute the correct value of tab-bar-lines for FRAME."
>
> But what "is" the correct value and why and how would we want to
> "compute" it?

How about this:

diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index 9666fb2460..1fc8537ccf 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -135,7 +135,11 @@ Possible modifier keys are `control', `meta',
`shift', `hyper', `super' and
                         tab-bar-close-button)))

(defun tab-bar--tab-bar-lines-for-frame (frame)
-  "Compute the correct value of tab-bar-lines for the given frame."
+  "Determine the value of tab-bar-lines for FRAME.
+The returned value will either be 1 or 0, meaning whether to show
+the tab-bar or not. If tab-bar-mode is off, the returned value is
+0. Otherwise the result depends on the value of the customizable
+variable `tab-bar-show'."
  (if (not tab-bar-mode)
      0
    (cond
@@ -145,10 +149,10 @@ Possible modifier keys are `control', `meta',
`shift', `hyper', `super' and
     (t 0))))

(defun tab-bar--update-tab-bar-lines (&optional frame)
-  "Update frame parameter tab-bar-line.
-When the optional frame parameter is omitted all frames as well
-as the default for new frames are updated. Otherwise only the
-given frame is modified."
+  "Update the tab-bar-line parameter in frames.
+When the optional parameter FRAME is omitted all frames as well
+as the default for new frames are updated. Otherwise only FRAME
+is modified."
  (if frame
      ;; Set frame parameters for the given frame
      (set-frame-parameter frame 'tab-bar-lines
(tab-bar--tab-bar-lines-for-frame frame))

>
>  > As far as I understand, tab-bar-lines is always just 1 or 0, meaning
>  > whether to show the tab-bar at all or not. Maybe it would be better to
>  > just rename the parameter? I guess if that is done then it does not
>  > necessarily need further explanation in docstrings.
>
> That ship has sailed long ago.  Neither the 'menu-bar-lines' nor the
> 'tool-bar-lines' parameters convey useful information in this regard and
> 'tab-bar-lines' just follows suit.  Their only practical (and completely
> misguided IMHO) purpose is to show the corresponding bar by setting the
> parameter to a non-zero value and remove it by setting the parameter to
> zero.
>
> Sometimes, as with our native tool bars, their value gives the number of
> frame lines occupied by the bar.  And very occasionally setting the
> parameter to a non-zero value can have strange effects: On a non-toolkit
> build setting menu-bar-lines to 7 will show six blank lines below a
> one-line menu bar which does not wrap anyway.
>
> In either case, we can hardly change the names of these frame parameters
> because they probably appear in too many applications and init files out
> there.  We could state somewhere that these are, in fact, booleans and
> should be set and interpreted in that sense.  Even that is not entirely
> trivial.

Makes sense!

>
> martin
Have a nice day
Bastian




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Tue, 09 Feb 2021 09:46:01 GMT) Full text and rfc822 format available.

Message #47 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Bastian Beranek <bastian.beischer <at> gmail.com>
Cc: 46299 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Tue, 9 Feb 2021 10:45:39 +0100
> +  "Determine the value of tab-bar-lines for FRAME.
> +The returned value will either be 1 or 0, meaning whether to show
> +the tab-bar or not. If tab-bar-mode is off, the returned value is
> +0. Otherwise the result depends on the value of the customizable
> +variable `tab-bar-show'."

Please use active voice and always make sure to leave two spaces in
front of a new sentence.  I would write it as

(defun tab-bar--tab-bar-lines-for-frame (frame)
  "Return new value of `tab-bar-lines' parameter for specified FRAME.
Return 0 if `tab-bar-mode' is not enabled on FRAME or ....
Return 1 if `tab-bar-mode' is enabled on FRAME and the variable
`tab-bar-show' ...

Call this function when ..."

where the last sentence would describe when and why this function should
be called, justifying the "new" in the doc-string.  Instead of "new" you
can also use "adjusted" if the value gets just adjusted or your earlier
"correct" provided an earlier calculation was "incorrect".

martin




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Tue, 09 Feb 2021 11:46:01 GMT) Full text and rfc822 format available.

Message #50 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 46299 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#46299: 28.0.50;
 Value of tab-bar-show not respected in new frames.
Date: Tue, 9 Feb 2021 12:44:47 +0100
[Message part 1 (text/plain, inline)]
On Tue, Feb 9, 2021 at 10:45 AM martin rudalics <rudalics <at> gmx.at> wrote:
>
>  > +  "Determine the value of tab-bar-lines for FRAME.
>  > +The returned value will either be 1 or 0, meaning whether to show
>  > +the tab-bar or not. If tab-bar-mode is off, the returned value is
>  > +0. Otherwise the result depends on the value of the customizable
>  > +variable `tab-bar-show'."
>
> Please use active voice and always make sure to leave two spaces in
> front of a new sentence.  I would write it as
>
> (defun tab-bar--tab-bar-lines-for-frame (frame)
>    "Return new value of `tab-bar-lines' parameter for specified FRAME.
> Return 0 if `tab-bar-mode' is not enabled on FRAME or ....
> Return 1 if `tab-bar-mode' is enabled on FRAME and the variable
> `tab-bar-show' ...
>
> Call this function when ..."
>
> where the last sentence would describe when and why this function should
> be called, justifying the "new" in the doc-string.  Instead of "new" you
> can also use "adjusted" if the value gets just adjusted or your earlier
> "correct" provided an earlier calculation was "incorrect".

Thanks for your comments. I went with

(defun tab-bar--tab-bar-lines-for-frame (frame)
  "Determine and return the value of `tab-bar-lines' for FRAME.
Return 0 if `tab-bar-mode' is not enabled.  Otherwise return
either 1 or 0 depending on the value of the customizable variable
`tab-bar-show', which see."

Please see the attached v5 of the complete patch.

I don't think tab-bar-mode can vary from frame to frame? I also did
not add the "Call this function" part, because this is an internal
helper function that is only used from
"tab-bar--update-tab-bar-lines", just below. It just returns 0 or 1
and does not modify anything, the actual frame parameter modification
happens in the other function.

Hope this is satisfactory, if not please feel free to adjust as you wish.

Do I need to do the contribution paperworks for this patch? So far I
haven't done that.

Best
Bastian
[tab-bar_v5.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Tue, 09 Feb 2021 17:33:01 GMT) Full text and rfc822 format available.

Message #53 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Bastian Beranek <bastian.beischer <at> gmail.com>
Cc: 46299 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Tue, 9 Feb 2021 18:32:40 +0100
> (defun tab-bar--tab-bar-lines-for-frame (frame)
>    "Determine and return the value of `tab-bar-lines' for FRAME.
> Return 0 if `tab-bar-mode' is not enabled.  Otherwise return
> either 1 or 0 depending on the value of the customizable variable
> `tab-bar-show', which see."

Fine with me.

> I don't think tab-bar-mode can vary from frame to frame?

I think only the 'tab-bar-lines' parameter can vary.

> I also did
> not add the "Call this function" part, because this is an internal
> helper function that is only used from
> "tab-bar--update-tab-bar-lines", just below. It just returns 0 or 1
> and does not modify anything, the actual frame parameter modification
> happens in the other function.
>
> Hope this is satisfactory, if not please feel free to adjust as you wish.

I leave that to Juri.

> Do I need to do the contribution paperworks for this patch? So far I
> haven't done that.

I suppose you need to do that.

Thanks, martin




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Tue, 09 Feb 2021 18:16:02 GMT) Full text and rfc822 format available.

Message #56 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Bastian Beranek <bastian.beischer <at> gmail.com>
Cc: martin rudalics <rudalics <at> gmx.at>, 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Tue, 09 Feb 2021 20:06:07 +0200
> I don't think tab-bar-mode can vary from frame to frame?

Indeed, currently tab-bar-mode is global, but not frame-local.
It's just a convenient way to enable the tab-bar without creating
a new tab first (when tab-bar-show is t).  Another convenience of
tab-bar-mode is enabling the C-TAB keybindings for switching tabs.

> Hope this is satisfactory, if not please feel free to adjust as you wish.

Thanks, looks nice, but needs a little more testing with different settings.

> Do I need to do the contribution paperworks for this patch? So far I
> haven't done that.

I don't know.  Please ask Eli or Lars if your contributions
fit into the updated rule in https://debbugs.gnu.org/44834#73




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Tue, 09 Feb 2021 18:47:01 GMT) Full text and rfc822 format available.

Message #59 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>, Eli Zaretskii <eliz <at> gnu.org>,
 Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: martin rudalics <rudalics <at> gmx.at>, 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50;
 Value of tab-bar-show not respected in new frames.
Date: Tue, 9 Feb 2021 19:46:14 +0100
On Tue, Feb 9, 2021 at 7:15 PM Juri Linkov <juri <at> linkov.net> wrote:
>
> > I don't think tab-bar-mode can vary from frame to frame?
>
> Indeed, currently tab-bar-mode is global, but not frame-local.
> It's just a convenient way to enable the tab-bar without creating
> a new tab first (when tab-bar-show is t).  Another convenience of
> tab-bar-mode is enabling the C-TAB keybindings for switching tabs.
>
> > Hope this is satisfactory, if not please feel free to adjust as you wish.
>
> Thanks, looks nice, but needs a little more testing with different settings.
>

Thanks. I have been running it myself for a while now and didn't
notice anything, but more testing is always good!

> > Do I need to do the contribution paperworks for this patch? So far I
> > haven't done that.
>
> I don't know.  Please ask Eli or Lars if your contributions
> fit into the updated rule in https://debbugs.gnu.org/44834#73

Eli, Lars, what do you think? Since I already contributed a few
patches I would guess I need to assign copyright. I am fine with that,
maybe it's time to do it even if my contribution could be accepted
without. Could you please send me the form?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Tue, 09 Feb 2021 19:03:02 GMT) Full text and rfc822 format available.

Message #62 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46299 <at> debbugs.gnu.org, bastian.beischer <at> gmail.com
Subject: Re: bug#46299: 28.0.50;
 Value of tab-bar-show not respected in new frames.
Date: Tue, 09 Feb 2021 21:01:48 +0200
> From: Juri Linkov <juri <at> linkov.net>
> Date: Tue, 09 Feb 2021 20:06:07 +0200
> Cc: 46299 <at> debbugs.gnu.org
> 
> > Do I need to do the contribution paperworks for this patch? So far I
> > haven't done that.
> 
> I don't know.  Please ask Eli or Lars if your contributions
> fit into the updated rule in https://debbugs.gnu.org/44834#73

They definitely exceed the threshold.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Tue, 09 Feb 2021 19:09:01 GMT) Full text and rfc822 format available.

Message #65 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Bastian Beranek <bastian.beischer <at> gmail.com>
Cc: rudalics <at> gmx.at, larsi <at> gnus.org, 46299 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#46299: 28.0.50;
 Value of tab-bar-show not respected in new frames.
Date: Tue, 09 Feb 2021 21:08:15 +0200
> From: Bastian Beranek <bastian.beischer <at> gmail.com>
> Date: Tue, 9 Feb 2021 19:46:14 +0100
> Cc: martin rudalics <rudalics <at> gmx.at>, 46299 <at> debbugs.gnu.org
> 
> Eli, Lars, what do you think? Since I already contributed a few
> patches I would guess I need to assign copyright. I am fine with that,
> maybe it's time to do it even if my contribution could be accepted
> without. Could you please send me the form?

Sent off-list.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Wed, 10 Feb 2021 18:42:02 GMT) Full text and rfc822 format available.

Message #68 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Bastian Beranek <bastian.beischer <at> gmail.com>
Cc: martin rudalics <rudalics <at> gmx.at>, 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Wed, 10 Feb 2021 20:20:56 +0200
>>  > +  "Update frame parameter tab-bar-line.
>>  > +When the optional frame parameter is omitted all frames as well
>>  > +as the default for new frames are updated. Otherwise only the
>>  > +given frame is modified."
>>
>> This is confusing wrt our parameter/argument terminology.  Please just
>> say "FRAME" wherever it stands for the function's argument and use
>> "parameter of FRAME" when you talk about the frame parameter.
>
> I'll do my best to update the docstring. Note that I do not contribute
> to emacs often, nor do I regularly code in elisp, so I'm not familiar
> with the conventions and you are encouraged to modify my changes as
> you see fit.

I don't know if Martin will agree, but most frame functions
interpret their optional FRAME argument in such a way that
if it's nil or omitted, FRAME defaults to the selected frame.

For example, 'set-frame-font' documents this:

  If FRAMES is nil, apply the font to the selected frame only.
  If FRAMES is non-nil, it should be a list of frames to act upon,
  or t meaning all existing graphical frames.

and uses such implementation:

   (let ((frame-lst (cond ((null frames)
                           (list (selected-frame)))
                          ((eq frames t)
                           (frame-list))
                          (t frames))))
      (dolist (f frame-lst)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Wed, 10 Feb 2021 18:42:02 GMT) Full text and rfc822 format available.

Message #71 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Bastian Beranek <bastian.beischer <at> gmail.com>
Cc: martin rudalics <rudalics <at> gmx.at>, 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Wed, 10 Feb 2021 20:24:49 +0200
> Hope this is satisfactory, if not please feel free to adjust as you wish.

Thanks, please see more comments:

> +(defun tab-bar--tab-bar-lines-for-frame (frame)
> +  (if (not tab-bar-mode)
> +      0
> +    (cond
> +     ((eq tab-bar-show t) 1)
> +     ((natnump tab-bar-show)
> +      (if (> (length (funcall tab-bar-tabs-function frame)) tab-bar-show) 1 0))
> +     (t 0))))

A small optimization:

  ((not tab-bar-mode) 0)

could be added as the first condition of the same 'cond'.

>    :set (lambda (sym val)
>           (set-default sym val)
>           ;; Preload button images
> +         ;; Note: tab-bar-mode updates tab-bar-lines as well.
> +         (tab-bar-mode 1))

Not sure whether the users would want to enable tab-bar-mode
unconditionally after customizing tab-bar-show.

Maybe when customized tab-bar-show to nil, only call
tab-bar--update-tab-bar-lines in all frames?
Or maybe simply to disable the tab bar with (tab-bar-mode 0)
when customized to nil?

> @@ -852,16 +867,15 @@ After the tab is created, the hooks in
> +    ;; Switch on tab-bar-mode, since a tab was created
> +    (when tab-bar-show
>        (tab-bar-mode 1))
> +
> +    ;; Recalculate tab-bar-lines and update frames
> +    (tab-bar--update-tab-bar-lines (selected-frame))
> +    (when tab-bar-mode
> +      (tab-bar--load-buttons)
> +      (tab-bar--define-keys))

Would you agree that here in tab-bar-new-tab-to, the first call of
tab-bar-mode should already do all these calls: tab-bar--update-tab-bar-lines,
tab-bar--load-buttons, tab-bar--define-keys?  So maybe it should be
sufficient just to leave these 2 lines here:

    (when tab-bar-show
       (tab-bar-mode 1))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Thu, 11 Feb 2021 12:16:01 GMT) Full text and rfc822 format available.

Message #74 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: martin rudalics <rudalics <at> gmx.at>, 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50;
 Value of tab-bar-show not respected in new frames.
Date: Thu, 11 Feb 2021 13:14:53 +0100
[Message part 1 (text/plain, inline)]
Hello Juri,

I have updated my patch (see v6 attached). Please find my comments inline.

I have also finished the copyright assignment procedure now.

On Wed, Feb 10, 2021 at 7:41 PM Juri Linkov <juri <at> linkov.net> wrote:
> I don't know if Martin will agree, but most frame functions
> interpret their optional FRAME argument in such a way that
> if it's nil or omitted, FRAME defaults to the selected frame.
>
> For example, 'set-frame-font' documents this:
>
>   If FRAMES is nil, apply the font to the selected frame only.
>   If FRAMES is non-nil, it should be a list of frames to act upon,
>   or t meaning all existing graphical frames.
>
> and uses such implementation:
>
>    (let ((frame-lst (cond ((null frames)
>                            (list (selected-frame)))
>                           ((eq frames t)
>                            (frame-list))
>                           (t frames))))
>       (dolist (f frame-lst)

That's true and I noticed this inconsistency as well. So thanks for
the suggestion, I have updated the patch accordingly.

On Wed, Feb 10, 2021 at 7:41 PM Juri Linkov <juri <at> linkov.net> wrote:
>
> > Hope this is satisfactory, if not please feel free to adjust as you wish.
>
> Thanks, please see more comments:
>
> > +(defun tab-bar--tab-bar-lines-for-frame (frame)
> > +  (if (not tab-bar-mode)
> > +      0
> > +    (cond
> > +     ((eq tab-bar-show t) 1)
> > +     ((natnump tab-bar-show)
> > +      (if (> (length (funcall tab-bar-tabs-function frame)) tab-bar-show) 1 0))
> > +     (t 0))))
>
> A small optimization:
>
>   ((not tab-bar-mode) 0)
>
> could be added as the first condition of the same 'cond'.

Thanks! Done.

>
> >    :set (lambda (sym val)
> >           (set-default sym val)
> >           ;; Preload button images
> > +         ;; Note: tab-bar-mode updates tab-bar-lines as well.
> > +         (tab-bar-mode 1))
>
> Not sure whether the users would want to enable tab-bar-mode
> unconditionally after customizing tab-bar-show.
>
> Maybe when customized tab-bar-show to nil, only call
> tab-bar--update-tab-bar-lines in all frames?
> Or maybe simply to disable the tab bar with (tab-bar-mode 0)
> when customized to nil?

I am not sure either. I think the best is to just leave tab-bar-mode
as it is to be honest. All this entanglement doesn't seem very clean
to me. Yes this would mean that the user needs to manually enable
tab-bar-mode after customizing the variable, but on the other hand
tab-bar-mode is on by default, so the user must have switched it off
in his .emacs by choice. So I just added the call the
tab-bar--update-tab-bar-lines for all frames, because this is
necessary for sure. On the other hand I don't fully understand the
comment about 'Preload button images'. I think the images and
keybindings are loaded when tab-bar-mode is switched on and afterwards
whenever a new tab is created in tab-bar-new-to, so it seems
independent of tab-bar-show. When tab-bar-show is customized they are
either already loaded because tab-bar-mode is on, or if it is not they
are not required and will be loaded when tab-bar-mode is activated.

>
> > @@ -852,16 +867,15 @@ After the tab is created, the hooks in
> > +    ;; Switch on tab-bar-mode, since a tab was created
> > +    (when tab-bar-show
> >        (tab-bar-mode 1))
> > +
> > +    ;; Recalculate tab-bar-lines and update frames
> > +    (tab-bar--update-tab-bar-lines (selected-frame))
> > +    (when tab-bar-mode
> > +      (tab-bar--load-buttons)
> > +      (tab-bar--define-keys))
>
> Would you agree that here in tab-bar-new-tab-to, the first call of
> tab-bar-mode should already do all these calls: tab-bar--update-tab-bar-lines,
> tab-bar--load-buttons, tab-bar--define-keys?  So maybe it should be
> sufficient just to leave these 2 lines here:
>
>     (when tab-bar-show
>        (tab-bar-mode 1))

Yes I agree that tab-bar--update-tab-bar-lines is not needed. It
happens in the line before when tab-bar-show is not nil and doesn't
matter otherwise. I have left these two lines, though:

    (when tab-bar-mode
      (tab-bar--load-buttons)
      (tab-bar--define-keys))

Because I think defining the keys is useful even if tab-bar-show is
nil, so you can switch to another tab using the key bindings even if
you can't see the tab-bar. As for the buttons, I think it makes sense
to load them so that in case tab-bar-show is customized to another
value afterwards they are available directly.
[tab-bar_v6.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Thu, 11 Feb 2021 17:35:01 GMT) Full text and rfc822 format available.

Message #77 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: martin rudalics <rudalics <at> gmx.at>, 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50;
 Value of tab-bar-show not respected in new frames.
Date: Thu, 11 Feb 2021 18:34:26 +0100
[Message part 1 (text/plain, inline)]
On Thu, Feb 11, 2021 at 1:14 PM Bastian Beranek
<bastian.beischer <at> gmail.com> wrote:
>
> Hello Juri,
>
> I have updated my patch (see v6 attached). Please find my comments inline.
>
> I have also finished the copyright assignment procedure now.
>
> On Wed, Feb 10, 2021 at 7:41 PM Juri Linkov <juri <at> linkov.net> wrote:
> > I don't know if Martin will agree, but most frame functions
> > interpret their optional FRAME argument in such a way that
> > if it's nil or omitted, FRAME defaults to the selected frame.
> >
> > For example, 'set-frame-font' documents this:
> >
> >   If FRAMES is nil, apply the font to the selected frame only.
> >   If FRAMES is non-nil, it should be a list of frames to act upon,
> >   or t meaning all existing graphical frames.
> >
> > and uses such implementation:
> >
> >    (let ((frame-lst (cond ((null frames)
> >                            (list (selected-frame)))
> >                           ((eq frames t)
> >                            (frame-list))
> >                           (t frames))))
> >       (dolist (f frame-lst)
>
> That's true and I noticed this inconsistency as well. So thanks for
> the suggestion, I have updated the patch accordingly.
>
> On Wed, Feb 10, 2021 at 7:41 PM Juri Linkov <juri <at> linkov.net> wrote:
> >
> > > Hope this is satisfactory, if not please feel free to adjust as you wish.
> >
> > Thanks, please see more comments:
> >
> > > +(defun tab-bar--tab-bar-lines-for-frame (frame)
> > > +  (if (not tab-bar-mode)
> > > +      0
> > > +    (cond
> > > +     ((eq tab-bar-show t) 1)
> > > +     ((natnump tab-bar-show)
> > > +      (if (> (length (funcall tab-bar-tabs-function frame)) tab-bar-show) 1 0))
> > > +     (t 0))))
> >
> > A small optimization:
> >
> >   ((not tab-bar-mode) 0)
> >
> > could be added as the first condition of the same 'cond'.
>
> Thanks! Done.
>
> >
> > >    :set (lambda (sym val)
> > >           (set-default sym val)
> > >           ;; Preload button images
> > > +         ;; Note: tab-bar-mode updates tab-bar-lines as well.
> > > +         (tab-bar-mode 1))
> >
> > Not sure whether the users would want to enable tab-bar-mode
> > unconditionally after customizing tab-bar-show.
> >
> > Maybe when customized tab-bar-show to nil, only call
> > tab-bar--update-tab-bar-lines in all frames?
> > Or maybe simply to disable the tab bar with (tab-bar-mode 0)
> > when customized to nil?
>
> I am not sure either. I think the best is to just leave tab-bar-mode
> as it is to be honest. All this entanglement doesn't seem very clean
> to me. Yes this would mean that the user needs to manually enable
> tab-bar-mode after customizing the variable, but on the other hand
> tab-bar-mode is on by default, so the user must have switched it off
> in his .emacs by choice. So I just added the call the
> tab-bar--update-tab-bar-lines for all frames, because this is
> necessary for sure. On the other hand I don't fully understand the
> comment about 'Preload button images'. I think the images and
> keybindings are loaded when tab-bar-mode is switched on and afterwards
> whenever a new tab is created in tab-bar-new-to, so it seems
> independent of tab-bar-show. When tab-bar-show is customized they are
> either already loaded because tab-bar-mode is on, or if it is not they
> are not required and will be loaded when tab-bar-mode is activated.

I was wrong about the "tab-bar-mode is on by default" part, so maybe I
like your suggestion more. I added:

diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index 7afbb96212..93573d8a75 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -276,8 +276,9 @@ tab-bar-show
  :initialize 'custom-initialize-default
  :set (lambda (sym val)
         (set-default sym val)
-         ;; Recalculate tab-bar-lines for all frames
-         (tab-bar--update-tab-bar-lines t))
+         (if val
+             (tab-bar-mode 1)
+           (tab-bar--update-tab-bar-lines t)))
  :group 'tab-bar
  :version "27.1")

with respect to v6, v7 is attached.

>
> >
> > > @@ -852,16 +867,15 @@ After the tab is created, the hooks in
> > > +    ;; Switch on tab-bar-mode, since a tab was created
> > > +    (when tab-bar-show
> > >        (tab-bar-mode 1))
> > > +
> > > +    ;; Recalculate tab-bar-lines and update frames
> > > +    (tab-bar--update-tab-bar-lines (selected-frame))
> > > +    (when tab-bar-mode
> > > +      (tab-bar--load-buttons)
> > > +      (tab-bar--define-keys))
> >
> > Would you agree that here in tab-bar-new-tab-to, the first call of
> > tab-bar-mode should already do all these calls: tab-bar--update-tab-bar-lines,
> > tab-bar--load-buttons, tab-bar--define-keys?  So maybe it should be
> > sufficient just to leave these 2 lines here:
> >
> >     (when tab-bar-show
> >        (tab-bar-mode 1))
>
> Yes I agree that tab-bar--update-tab-bar-lines is not needed. It
> happens in the line before when tab-bar-show is not nil and doesn't
> matter otherwise. I have left these two lines, though:
>
>     (when tab-bar-mode
>       (tab-bar--load-buttons)
>       (tab-bar--define-keys))
>
> Because I think defining the keys is useful even if tab-bar-show is
> nil, so you can switch to another tab using the key bindings even if
> you can't see the tab-bar. As for the buttons, I think it makes sense
> to load them so that in case tab-bar-show is customized to another
> value afterwards they are available directly.
[tab-bar_v7.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Fri, 12 Feb 2021 09:58:02 GMT) Full text and rfc822 format available.

Message #80 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Bastian Beranek <bastian.beischer <at> gmail.com>
Cc: martin rudalics <rudalics <at> gmx.at>, 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Fri, 12 Feb 2021 11:31:19 +0200
Hello Bastian,

Everything in your patch v7 is correct now, except one case of
tab-bar-new-tab-to:

>> Would you agree that here in tab-bar-new-tab-to, the first call of
>> tab-bar-mode should already do all these calls: tab-bar--update-tab-bar-lines,
>> tab-bar--load-buttons, tab-bar--define-keys?  So maybe it should be
>> sufficient just to leave these 2 lines here:
>>
>>     (when tab-bar-show
>>        (tab-bar-mode 1))

I noticed this could be optimized not to call tab-bar-mode again every time
when tab-bar-mode was already enabled.  Maybe use something like this:

    (when (and (not tab-bar-mode) tab-bar-show)
      (tab-bar-mode 1))

> Yes I agree that tab-bar--update-tab-bar-lines is not needed. It
> happens in the line before when tab-bar-show is not nil and doesn't
> matter otherwise. I have left these two lines, though:
>
>     (when tab-bar-mode
>       (tab-bar--load-buttons)
>       (tab-bar--define-keys))

I still have doubts whether these lines are needed at all.

> Because I think defining the keys is useful even if tab-bar-show is
> nil, so you can switch to another tab using the key bindings even if
> you can't see the tab-bar.

The problem is that tab-bar--define-keys defines only two keys
C-TAB and S-C-TAB and [modifier-digit] keys to select a tab by its
displayed number that mostly make sense with the visible tab bar.

So one of the purposes of the nil value of tab-bar-show was to
allow the users also to disable the C-TAB and digit keys.  Then
users could use C-TAB bindings from other packages, while still
using global tab-switching keys such as 'C-x t o', and also to
select tabs by names using 'C-x t b', whereas selecting by numbers
makes sense only when the tab bar is visible.

> As for the buttons, I think it makes sense to load them so that in
> case tab-bar-show is customized to another value afterwards they are
> available directly.

tab-bar--load-buttons and tab-bar--define-keys are called anyway
when enabling the tab bar with tab-bar-mode.  So these two functions
could be called only in tab-bar-mode, but afterwards when
it's already enabled, there is no need to call them again.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Fri, 12 Feb 2021 10:25:02 GMT) Full text and rfc822 format available.

Message #83 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: martin rudalics <rudalics <at> gmx.at>, 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50;
 Value of tab-bar-show not respected in new frames.
Date: Fri, 12 Feb 2021 11:24:26 +0100
[Message part 1 (text/plain, inline)]
Hey Juri,

On Fri, Feb 12, 2021 at 10:57 AM Juri Linkov <juri <at> linkov.net> wrote:
>
> Hello Bastian,
>
> Everything in your patch v7 is correct now, except one case of
> tab-bar-new-tab-to:
>
> >> Would you agree that here in tab-bar-new-tab-to, the first call of
> >> tab-bar-mode should already do all these calls: tab-bar--update-tab-bar-lines,
> >> tab-bar--load-buttons, tab-bar--define-keys?  So maybe it should be
> >> sufficient just to leave these 2 lines here:
> >>
> >>     (when tab-bar-show
> >>        (tab-bar-mode 1))
>
> I noticed this could be optimized not to call tab-bar-mode again every time
> when tab-bar-mode was already enabled.  Maybe use something like this:
>
>     (when (and (not tab-bar-mode) tab-bar-show)
>       (tab-bar-mode 1))
>

I've adjusted the patch accordingly, but we do need a call to
tab-bar--update-tab-bar-lines whenever a tab is created (because we
need to check if there is more than one tab now, which changes the
display criterion). So I added:

    (when tab-bar-show
      (if (not tab-bar-mode)
          ;; Switch on tab-bar-mode, since a tab was created
          ;; Note: This also updates tab-bar-lines
          (tab-bar-mode 1)
        (tab-bar--update-tab-bar-lines)))


> > Yes I agree that tab-bar--update-tab-bar-lines is not needed. It
> > happens in the line before when tab-bar-show is not nil and doesn't
> > matter otherwise. I have left these two lines, though:
> >
> >     (when tab-bar-mode
> >       (tab-bar--load-buttons)
> >       (tab-bar--define-keys))
>
> I still have doubts whether these lines are needed at all.
>
> > Because I think defining the keys is useful even if tab-bar-show is
> > nil, so you can switch to another tab using the key bindings even if
> > you can't see the tab-bar.
>
> The problem is that tab-bar--define-keys defines only two keys
> C-TAB and S-C-TAB and [modifier-digit] keys to select a tab by its
> displayed number that mostly make sense with the visible tab bar.
>
> So one of the purposes of the nil value of tab-bar-show was to
> allow the users also to disable the C-TAB and digit keys.  Then
> users could use C-TAB bindings from other packages, while still
> using global tab-switching keys such as 'C-x t o', and also to
> select tabs by names using 'C-x t b', whereas selecting by numbers
> makes sense only when the tab bar is visible.
>
> > As for the buttons, I think it makes sense to load them so that in
> > case tab-bar-show is customized to another value afterwards they are
> > available directly.
>
> tab-bar--load-buttons and tab-bar--define-keys are called anyway
> when enabling the tab bar with tab-bar-mode.  So these two functions
> could be called only in tab-bar-mode, but afterwards when
> it's already enabled, there is no need to call them again.

I see now that you are right and I removed those lines.

Cheers
Bastian
[tab-bar_v8.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Fri, 12 Feb 2021 14:48:01 GMT) Full text and rfc822 format available.

Message #86 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: martin rudalics <rudalics <at> gmx.at>, 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50;
 Value of tab-bar-show not respected in new frames.
Date: Fri, 12 Feb 2021 15:47:24 +0100
Hey Juri,

just to let you know, that I've found a problem with my patch on
text-mode frames. If I create multiple tabs one after another they
start to appear in a second row and visuals begin to glitch.

I'll try to understand why that is happening.

Bastian




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Fri, 12 Feb 2021 19:24:01 GMT) Full text and rfc822 format available.

Message #89 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: martin rudalics <rudalics <at> gmx.at>, 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50;
 Value of tab-bar-show not respected in new frames.
Date: Fri, 12 Feb 2021 20:23:26 +0100
[Message part 1 (text/plain, inline)]
On Fri, Feb 12, 2021 at 3:47 PM Bastian Beranek
<bastian.beischer <at> gmail.com> wrote:
>
> Hey Juri,
>
> just to let you know, that I've found a problem with my patch on
> text-mode frames. If I create multiple tabs one after another they
> start to appear in a second row and visuals begin to glitch.
>
> I'll try to understand why that is happening.

I found out what the problem was. I had customized
tab-bar-select-tab-modifiers in my .emacs and the :set function of
that variable contained (tab-bar-mode -1) followed by (tab-bar-mode
1). For some reason this caused issues (to reproduce this, apply my
v8.patch, create test.el (as below) and start emacs as follows:

$ cat test.el
(customize-set-variable 'tab-bar-select-tab-modifiers '(super))
(customize-set-variable 'tab-bar-show 1)

$ src/emacs -nw -Q -l test.el
and then create two or more tabs (C-x t 2)

I have now modified the set function to:

diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index 87c9fd719d..4e47ae2c10 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -89,8 +89,9 @@ tab-bar-select-tab-modifiers
  :set (lambda (sym val)
         (set-default sym val)
         ;; Reenable the tab-bar with new keybindings
-         (tab-bar-mode -1)
-         (tab-bar-mode 1))
+         (when tab-bar-mode
+           (tab-bar-mode -1)
+           (tab-bar-mode 1)))
  :group 'tab-bar
  :version "27.1")

This seems to fix the issue. I can't say I fully understand why
though. It must have something to do with running
tab-bar--update-tab-bar-lines in early initialization? We could also
wrap the call to tab-bar--update-tab-bar-lines.

Please find the full v9 patch attached.

Cheers
Bastian
[tab-bar_v9.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Sat, 13 Feb 2021 18:34:01 GMT) Full text and rfc822 format available.

Message #92 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Bastian Beranek <bastian.beischer <at> gmail.com>
Cc: martin rudalics <rudalics <at> gmx.at>, 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Sat, 13 Feb 2021 20:23:20 +0200
Hey Bastian,

Thank you for working on this patch.  Please prepare the ChangeLog
commit message, so your patch could be pushed to master.  Then it would be
easier to reason about further changes and base them on the pushed version.

> @@ -89,8 +89,9 @@ tab-bar-select-tab-modifiers
>   :set (lambda (sym val)
>          (set-default sym val)
>          ;; Reenable the tab-bar with new keybindings
> -         (tab-bar-mode -1)
> -         (tab-bar-mode 1))
> +         (when tab-bar-mode
> +           (tab-bar-mode -1)
> +           (tab-bar-mode 1)))
>   :group 'tab-bar
>   :version "27.1")
>
> This seems to fix the issue. I can't say I fully understand why
> though. It must have something to do with running
> tab-bar--update-tab-bar-lines in early initialization? We could also
> wrap the call to tab-bar--update-tab-bar-lines.

The problem is that currently the function tab-bar-mode contains:

  (if tab-bar-mode
      (tab-bar--define-keys)
    ;; Unset only keys bound by tab-bar
    (when (eq (global-key-binding [(control tab)]) 'tab-next)
      (global-unset-key [(control tab)]))
    ...

If the global-unset-key part would be refactored into a separate
function, then tab-bar-select-tab-modifiers could call two
functions sequentially: a new function that undefines old keys,
then the existing separate function tab-bar--define-keys
that will define keys with customized modifier.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Sat, 13 Feb 2021 19:03:01 GMT) Full text and rfc822 format available.

Message #95 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: martin rudalics <rudalics <at> gmx.at>, 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50;
 Value of tab-bar-show not respected in new frames.
Date: Sat, 13 Feb 2021 20:02:26 +0100
On Sat, Feb 13, 2021 at 7:33 PM Juri Linkov <juri <at> linkov.net> wrote:
>
> Hey Bastian,
>
> Thank you for working on this patch.  Please prepare the ChangeLog
> commit message, so your patch could be pushed to master.  Then it would be
> easier to reason about further changes and base them on the pushed version.

I will do that and set a proper patch for git am.

>
> > @@ -89,8 +89,9 @@ tab-bar-select-tab-modifiers
> >   :set (lambda (sym val)
> >          (set-default sym val)
> >          ;; Reenable the tab-bar with new keybindings
> > -         (tab-bar-mode -1)
> > -         (tab-bar-mode 1))
> > +         (when tab-bar-mode
> > +           (tab-bar-mode -1)
> > +           (tab-bar-mode 1)))
> >   :group 'tab-bar
> >   :version "27.1")
> >
> > This seems to fix the issue. I can't say I fully understand why
> > though. It must have something to do with running
> > tab-bar--update-tab-bar-lines in early initialization? We could also
> > wrap the call to tab-bar--update-tab-bar-lines.
>
> The problem is that currently the function tab-bar-mode contains:
>
>   (if tab-bar-mode
>       (tab-bar--define-keys)
>     ;; Unset only keys bound by tab-bar
>     (when (eq (global-key-binding [(control tab)]) 'tab-next)
>       (global-unset-key [(control tab)]))
>     ...
>
> If the global-unset-key part would be refactored into a separate
> function, then tab-bar-select-tab-modifiers could call two
> functions sequentially: a new function that undefines old keys,
> then the existing separate function tab-bar--define-keys
> that will define keys with customized modifier.

But how would that cause visual glitches, such as the tab bar
splitting into two lines, menu bar not showing and general visual
issues (lines jumping around etc.). This worries me a bit, because I
don't understand the problem (just worked around it by not running the
(tab-bar-mode -1) (tab-bar-mode 1) sequence. Did you try the recipe I
reported above?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Sat, 13 Feb 2021 20:03:02 GMT) Full text and rfc822 format available.

Message #98 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Bastian Beranek <bastian.beischer <at> gmail.com>
Cc: martin rudalics <rudalics <at> gmx.at>, 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Sat, 13 Feb 2021 21:46:00 +0200
> But how would that cause visual glitches, such as the tab bar
> splitting into two lines, menu bar not showing and general visual
> issues (lines jumping around etc.). This worries me a bit, because I
> don't understand the problem (just worked around it by not running the
> (tab-bar-mode -1) (tab-bar-mode 1) sequence. Did you try the recipe I
> reported above?

Yes, I see the same.  A simpler test case is:

  (tab-bar-mode -1)
  (tab-bar-mode 1)
  (tab-bar-mode -1)

This means that running (tab-bar-mode -1) (tab-bar-mode 1)
should be avoided when emacs is started, because it disables
the menu bar.  Maybe there is a bug in the display code.
But adding 'redisplay' in the middle fixes the mess
created by enabling/disabling the tab bar:

  (tab-bar-mode -1)
  (tab-bar-mode 1)
  (redisplay)
  (tab-bar-mode -1)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Sun, 14 Feb 2021 13:10:02 GMT) Full text and rfc822 format available.

Message #101 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50;
 Value of tab-bar-show not respected in new frames.
Date: Sun, 14 Feb 2021 14:08:55 +0100
[Message part 1 (text/plain, inline)]
> Thank you for working on this patch.  Please prepare the ChangeLog
> commit message, so your patch could be pushed to master.  Then it would be
> easier to reason about further changes and base them on the pushed version.

Hello Juri,

please find the patch including the commit message with ChangeLog attached.

Cheers
Bastian
[0001-Fix-showing-and-hiding-of-tab-bar-on-new-frames-bug-.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Sun, 14 Feb 2021 18:54:02 GMT) Full text and rfc822 format available.

Message #104 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Bastian Beranek <bastian.beischer <at> gmail.com>
Cc: 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Sun, 14 Feb 2021 20:44:20 +0200
>> But how would that cause visual glitches, such as the tab bar
>> splitting into two lines, menu bar not showing and general visual
>> issues (lines jumping around etc.). This worries me a bit, because I
>> don't understand the problem (just worked around it by not running the
>> (tab-bar-mode -1) (tab-bar-mode 1) sequence. Did you try the recipe I
>> reported above?
>
> Yes, I see the same.  A simpler test case is:
>
>   (tab-bar-mode -1)
>   (tab-bar-mode 1)
>   (tab-bar-mode -1)

More low-level test case:

  (set-frame-parameter nil 'tab-bar-lines 0)
  (set-frame-parameter nil 'tab-bar-lines 1)
  (set-frame-parameter nil 'tab-bar-lines 0)

that breaks the menu bar.  I don't know if this is a bug in display code,
or it's required to use 'redisplay' in-between.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Sun, 14 Feb 2021 18:54:03 GMT) Full text and rfc822 format available.

Message #107 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Bastian Beranek <bastian.beischer <at> gmail.com>
Cc: 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Sun, 14 Feb 2021 20:50:40 +0200
>> Thank you for working on this patch.  Please prepare the ChangeLog
>> commit message, so your patch could be pushed to master.  Then it would be
>> easier to reason about further changes and base them on the pushed version.
>
> please find the patch including the commit message with ChangeLog attached.

Thanks, now your patch is pushed to master.

While using it, I noticed a new problem.
There is this code in 'tab-switcher':

    (let ((tab-bar-new-tab-choice t)
          ;; Don't enable tab-bar-mode if it's disabled
          (tab-bar-show nil))
      (tab-bar-new-tab))

Before your patch, I did nothing with the enabled tab-bar,
but now it disables the tab-bar, and doesn't enable it again later,
because tab-bar-show is let-bound to nil.

A good solution would be to add a new choice value to tab-bar-show.
Something like 'do-not-change-tab-bar-lines', but shorter.
Then when let-bound, it should do nothing with the tab-bar-lines.

Also this is related to another problem:
What if the user wants to manually enable the tab bar on one frame only
without enabling tab-bar-mode?  Currently it's possible with

  (add-hook 'after-make-frame-functions 'toggle-frame-tab-bar)

But tab-bar--update-tab-bar-lines will disable it sooner or later.
Customizing to the same value like 'do-not-change-tab-bar-lines'
will solve this problem as well.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Sun, 14 Feb 2021 20:11:01 GMT) Full text and rfc822 format available.

Message #110 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Bastian Beranek <bastian.beischer <at> gmail.com>
Cc: 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Sun, 14 Feb 2021 21:28:43 +0200
> A good solution would be to add a new choice value to tab-bar-show.
> Something like 'do-not-change-tab-bar-lines', but shorter.
> Then when let-bound, it should do nothing with the tab-bar-lines.

For example, a new value 'ignore'.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Mon, 15 Feb 2021 08:17:02 GMT) Full text and rfc822 format available.

Message #113 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>, Bastian Beranek <bastian.beischer <at> gmail.com>
Cc: 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Mon, 15 Feb 2021 09:16:40 +0100
> What if the user wants to manually enable the tab bar on one frame only
> without enabling tab-bar-mode?  Currently it's possible with
>
>    (add-hook 'after-make-frame-functions 'toggle-frame-tab-bar)

As a rule, this should be done by setting the 'tab-bar-lines' parameter
of that frame to non-zero, leaving it at zero for all other frames.

'toggle-frame-tab-bar' should stick to that rule.

martin




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Mon, 15 Feb 2021 09:51:01 GMT) Full text and rfc822 format available.

Message #116 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 46299 <at> debbugs.gnu.org, Bastian Beranek <bastian.beischer <at> gmail.com>
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Mon, 15 Feb 2021 11:07:37 +0200
>> What if the user wants to manually enable the tab bar on one frame only
>> without enabling tab-bar-mode?  Currently it's possible with
>>
>>    (add-hook 'after-make-frame-functions 'toggle-frame-tab-bar)
>
> As a rule, this should be done by setting the 'tab-bar-lines' parameter
> of that frame to non-zero, leaving it at zero for all other frames.
>
> 'toggle-frame-tab-bar' should stick to that rule.

'toggle-frame-tab-bar' already does exactly this - sets the
'tab-bar-lines' parameter to non-zero, and doesn't change it
on all other frames.

But currently 'tab-bar-show' overrides this user setting,
and affects all other frames.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Mon, 15 Feb 2021 10:07:02 GMT) Full text and rfc822 format available.

Message #119 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Mon, 15 Feb 2021 11:05:41 +0100
Juri Linkov <juri <at> linkov.net> writes:

>>> But how would that cause visual glitches, such as the tab bar
>>> splitting into two lines, menu bar not showing and general visual
>>> issues (lines jumping around etc.). This worries me a bit, because I
>>> don't understand the problem (just worked around it by not running the
>>> (tab-bar-mode -1) (tab-bar-mode 1) sequence. Did you try the recipe I
>>> reported above?
>>
>> Yes, I see the same.  A simpler test case is:
>>
>>   (tab-bar-mode -1)
>>   (tab-bar-mode 1)
>>   (tab-bar-mode -1)
>
> More low-level test case:
>
>   (set-frame-parameter nil 'tab-bar-lines 0)
>   (set-frame-parameter nil 'tab-bar-lines 1)
>   (set-frame-parameter nil 'tab-bar-lines 0)
>
> that breaks the menu bar.  I don't know if this is a bug in display code,
> or it's required to use 'redisplay' in-between.

Interesting. Should this be reported as a separate bug?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Mon, 15 Feb 2021 10:09:01 GMT) Full text and rfc822 format available.

Message #122 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46299 <at> debbugs.gnu.org, Bastian Beranek <bastian.beischer <at> gmail.com>
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Mon, 15 Feb 2021 11:08:10 +0100
> 'toggle-frame-tab-bar' already does exactly this - sets the
> 'tab-bar-lines' parameter to non-zero, and doesn't change it
> on all other frames.
>
> But currently 'tab-bar-show' overrides this user setting,
> and affects all other frames.

I forgot.  This is inherently broken for all frame parameters and
unlikely to change ever.

martin




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Mon, 15 Feb 2021 10:10:02 GMT) Full text and rfc822 format available.

Message #125 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Mon, 15 Feb 2021 11:09:39 +0100
Juri Linkov <juri <at> linkov.net> writes:

> Thanks, now your patch is pushed to master.

Thanks!

>
> While using it, I noticed a new problem.
> There is this code in 'tab-switcher':
>
>     (let ((tab-bar-new-tab-choice t)
>           ;; Don't enable tab-bar-mode if it's disabled
>           (tab-bar-show nil))
>       (tab-bar-new-tab))
>
> Before your patch, I did nothing with the enabled tab-bar,
> but now it disables the tab-bar, and doesn't enable it again later,
> because tab-bar-show is let-bound to nil.
>

Could you please describe the desired behavior of tab-switcher in a few
more words? I can't see anything wrong with it if I try it out here,
although I can sort of see what you're getting at: While the
tab-swicher is active there should be no tab bar, and it should return
when it is finished?

What I see is that the tab-bar just stays on all the time, with a
temporary tab for the tab-switcher itself. I have tab-bar-show
customized to "1" here.

Side question: Why does tab-switcher need to create a tab for its
purpose? Doesn't it make more sense to use a regular buffer for that?

> A good solution would be to add a new choice value to tab-bar-show.
> Something like 'do-not-change-tab-bar-lines', but shorter.
> Then when let-bound, it should do nothing with the tab-bar-lines.

Wouldn't it make more sense to have a different variable for that?
Because tab-bar-show is a defcustom and we wouldn't want to expose this
special value to users, right?

>
> Also this is related to another problem:
> What if the user wants to manually enable the tab bar on one frame only
> without enabling tab-bar-mode?  Currently it's possible with
>
>   (add-hook 'after-make-frame-functions 'toggle-frame-tab-bar)
>
> But tab-bar--update-tab-bar-lines will disable it sooner or later.
> Customizing to the same value like 'do-not-change-tab-bar-lines'
> will solve this problem as well.

That's true. I can see how tab-bar--update-tab-bar-lines can interfere
with toggle-frame-tab-bar.

But I think we would need a frame dependent variable to fix this. We
can't use a global single variable, because toggle-frame-tab-bar is not
supposed to change the behavior of tabs on other frames. I have to think
about a good solution for a bit longer. Is attaching a new parameter to
frames similiar to this a possibility?

diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index 4e47ae2c10..cbda0c032b 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -161,7 +161,8 @@ tab-bar--update-tab-bar-lines
                          (t frames))))
     ;; Loop over all frames and update default-frame-alist
     (dolist (frame frame-lst)
-      (set-frame-parameter frame 'tab-bar-lines (tab-bar--tab-bar-lines-for-frame frame))))
+      (unless (frame-parameter frame 'tab-bar-lines-do-not-change)
+        (set-frame-parameter frame 'tab-bar-lines (tab-bar--tab-bar-lines-for-frame frame)))))
   (when (eq frames t)
     (setq default-frame-alist
           (cons (cons 'tab-bar-lines (if (and tab-bar-mode (eq tab-bar-show t)) 1 0))
@@ -233,7 +234,8 @@ toggle-frame-tab-bar
   (add-hook 'after-make-frame-functions 'toggle-frame-tab-bar)"
   (interactive)
   (set-frame-parameter frame 'tab-bar-lines
-                       (if (> (frame-parameter frame 'tab-bar-lines) 0) 0 1)))
+                       (if (> (frame-parameter frame 'tab-bar-lines) 0) 0 1))
+  (set-frame-parameter frame 'tab-bar-lines-do-not-change t))
 
 (defvar tab-bar-map (make-sparse-keymap)
   "Keymap for the tab bar.

Note that I'm not yet suggesting that we do it exactly as the above,
this has other issues - toggling twice does leave the do-not-change
frame parameter in place for example, so it's not the same as doing
nothing.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Mon, 15 Feb 2021 10:13:01 GMT) Full text and rfc822 format available.

Message #128 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 46299 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Mon, 15 Feb 2021 11:12:27 +0100
martin rudalics <rudalics <at> gmx.at> writes:

>> What if the user wants to manually enable the tab bar on one frame only
>> without enabling tab-bar-mode?  Currently it's possible with
>>
>>    (add-hook 'after-make-frame-functions 'toggle-frame-tab-bar)
>
> As a rule, this should be done by setting the 'tab-bar-lines' parameter
> of that frame to non-zero, leaving it at zero for all other frames.
>
> 'toggle-frame-tab-bar' should stick to that rule.
>
> martin

This is the case. But the problem is that there is now a new function
which restores the value of tab-bar-lines to its original value,
whenever tabs are created or closed, or whenever tab-bar-mode is
switched off and on, for example.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Mon, 15 Feb 2021 15:27:01 GMT) Full text and rfc822 format available.

Message #131 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46299 <at> debbugs.gnu.org, bastian.beischer <at> gmail.com
Subject: Re: bug#46299: 28.0.50;
 Value of tab-bar-show not respected in new frames.
Date: Mon, 15 Feb 2021 17:26:25 +0200
> From: Juri Linkov <juri <at> linkov.net>
> Date: Sun, 14 Feb 2021 20:44:20 +0200
> Cc: 46299 <at> debbugs.gnu.org
> 
> More low-level test case:
> 
>   (set-frame-parameter nil 'tab-bar-lines 0)
>   (set-frame-parameter nil 'tab-bar-lines 1)
>   (set-frame-parameter nil 'tab-bar-lines 0)
> 
> that breaks the menu bar.

It doesn't break the menu bar here.  What kind of breakage are we
talking about, and in what Emacs configuration?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Mon, 15 Feb 2021 15:33:02 GMT) Full text and rfc822 format available.

Message #134 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 46299 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#46299: 28.0.50;
 Value of tab-bar-show not respected in new frames.
Date: Mon, 15 Feb 2021 16:32:24 +0100
On Mon, Feb 15, 2021 at 4:26 PM Eli Zaretskii <eliz <at> gnu.org> wrote:
>
> > From: Juri Linkov <juri <at> linkov.net>
> > Date: Sun, 14 Feb 2021 20:44:20 +0200
> > Cc: 46299 <at> debbugs.gnu.org
> >
> > More low-level test case:
> >
> >   (set-frame-parameter nil 'tab-bar-lines 0)
> >   (set-frame-parameter nil 'tab-bar-lines 1)
> >   (set-frame-parameter nil 'tab-bar-lines 0)
> >
> > that breaks the menu bar.
>
> It doesn't break the menu bar here.  What kind of breakage are we
> talking about, and in what Emacs configuration?

For this to trigger you need to put those lines in a .el file (say
foo.el) and run emacs in text mode and load that file:

emacs -nw -Q -l foo.el

For me there is no menu-bar anymore and using this emacs instance
quickly reveals visual glitches (mode line jumping around for
example). A good way to provoke this is to create a few tabs with C-x
t 2.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Mon, 15 Feb 2021 15:54:02 GMT) Full text and rfc822 format available.

Message #137 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Bastian Beranek <bastian.beischer <at> gmail.com>
Cc: 46299 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#46299: 28.0.50;
 Value of tab-bar-show not respected in new frames.
Date: Mon, 15 Feb 2021 17:53:26 +0200
> From: Bastian Beranek <bastian.beischer <at> gmail.com>
> Date: Mon, 15 Feb 2021 16:32:24 +0100
> Cc: Juri Linkov <juri <at> linkov.net>, 46299 <at> debbugs.gnu.org
> 
> > >   (set-frame-parameter nil 'tab-bar-lines 0)
> > >   (set-frame-parameter nil 'tab-bar-lines 1)
> > >   (set-frame-parameter nil 'tab-bar-lines 0)
> > >
> > > that breaks the menu bar.
> >
> > It doesn't break the menu bar here.  What kind of breakage are we
> > talking about, and in what Emacs configuration?
> 
> For this to trigger you need to put those lines in a .el file (say
> foo.el) and run emacs in text mode and load that file:
> 
> emacs -nw -Q -l foo.el
> 
> For me there is no menu-bar anymore and using this emacs instance
> quickly reveals visual glitches (mode line jumping around for
> example). A good way to provoke this is to create a few tabs with C-x
> t 2.

I see the problem on GNU/Linux, but not on MS-Windows.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Mon, 15 Feb 2021 17:08:01 GMT) Full text and rfc822 format available.

Message #140 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Bastian Beranek <bastian.beischer <at> gmail.com>
Cc: 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Mon, 15 Feb 2021 19:01:17 +0200
>> Before your patch, I did nothing with the enabled tab-bar,
>> but now it disables the tab-bar, and doesn't enable it again later,
>> because tab-bar-show is let-bound to nil.
>
> Could you please describe the desired behavior of tab-switcher in a few
> more words? I can't see anything wrong with it if I try it out here,

Sorry, this failed in one of the previous versions of the patch,
but works fine in the last version of your patch pushed to master.
So there is no problem, sorry for false alarm.

> although I can sort of see what you're getting at: While the
> tab-swicher is active there should be no tab bar, and it should return
> when it is finished?

Maybe this could provide a nice visual effect, but the problem
is that it's not easy to detect the moment when it is finished.

I often use tab-switcher to create a new tab, and sometimes instead
of selecting an existing tab from the tab list, I change the mind
and switch to another buffer in the same new tab.

> What I see is that the tab-bar just stays on all the time, with a
> temporary tab for the tab-switcher itself. I have tab-bar-show
> customized to "1" here.

This is fine.

> Side question: Why does tab-switcher need to create a tab for its
> purpose? Doesn't it make more sense to use a regular buffer for that?

This is to emulate window switching feature that exists on most window managers:
all windows are unselected when the window switcher is activated.

>> A good solution would be to add a new choice value to tab-bar-show.
>> Something like 'do-not-change-tab-bar-lines', but shorter.
>> Then when let-bound, it should do nothing with the tab-bar-lines.
>
> Wouldn't it make more sense to have a different variable for that?
> Because tab-bar-show is a defcustom and we wouldn't want to expose this
> special value to users, right?

Exposing such value to users is fine, but anyway it seems there is
no need to add a new value or variable.

>> Also this is related to another problem:
>> What if the user wants to manually enable the tab bar on one frame only
>> without enabling tab-bar-mode?  Currently it's possible with
>>
>>   (add-hook 'after-make-frame-functions 'toggle-frame-tab-bar)
>>
>> But tab-bar--update-tab-bar-lines will disable it sooner or later.
>> Customizing to the same value like 'do-not-change-tab-bar-lines'
>> will solve this problem as well.
>
> That's true. I can see how tab-bar--update-tab-bar-lines can interfere
> with toggle-frame-tab-bar.
>
> But I think we would need a frame dependent variable to fix this. We
> can't use a global single variable, because toggle-frame-tab-bar is not
> supposed to change the behavior of tabs on other frames. I have to think
> about a good solution for a bit longer. Is attaching a new parameter to
> frames similiar to this a possibility?

Good idea.  Actually frame parameters could serve as a kind of
"frame-local variables".

> Note that I'm not yet suggesting that we do it exactly as the above,
> this has other issues - toggling twice does leave the do-not-change
> frame parameter in place for example, so it's not the same as doing
> nothing.

Also toggling once should handle two cases:

1. while tab-bar-lines is enabled in all frames, it should disable
   tab-bar-lines in the specified frame;

2. while tab-bar-lines is disabled in all frames, it should enable
   tab-bar-lines in the specified frame.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Mon, 15 Feb 2021 22:12:01 GMT) Full text and rfc822 format available.

Message #143 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Mon, 15 Feb 2021 23:10:53 +0100
Juri Linkov <juri <at> linkov.net> writes:

> Sorry, this failed in one of the previous versions of the patch,
> but works fine in the last version of your patch pushed to master.
> So there is no problem, sorry for false alarm.
>

Good. Thanks!

> Good idea.  Actually frame parameters could serve as a kind of
> "frame-local variables".
>

Right, that would be the idea. Is there precedent for this? It seems to
be the easiest way forward, but I don't know if this is considered
acceptable. Another option would be a global alist variable that maps
frames to the value.

>> Note that I'm not yet suggesting that we do it exactly as the above,
>> this has other issues - toggling twice does leave the do-not-change
>> frame parameter in place for example, so it's not the same as doing
>> nothing.
>
> Also toggling once should handle two cases:
>
> 1. while tab-bar-lines is enabled in all frames, it should disable
>    tab-bar-lines in the specified frame;
>
> 2. while tab-bar-lines is disabled in all frames, it should enable
>    tab-bar-lines in the specified frame.

That much is clear. The only question is what should happen if
tab-bar-show is 1 initially and then toggle-frame-tab-bar is used:

- If there is more than 1 tab, the tab-bar will be shown before. Then
  toggle-frame-tab-bar should disable it.

- If there is only 1 tab, the tab-bar will not be shown before. Then
  toggle-frame-tab-bar should enable it.

But how to go back? It seems that tab-bar-show should go back to "1" (in
order to make it a real toggle, i.e. it undoes itself). However, that
means that after the second toggle-frame-tab-bar the tab-bar will either
be shown or not, depending on the number of tabs opened at that specific
time. We have to consider that the user created or closed tabs in
between, so that means that there will be situations in which
toggle-frame-tab-bar does not really seem to do anything... For example:

- 1 tab (tab bar hidden)
- create tab -> 2 tabs (tab bar shown)
- toggle-frame-tab-bar (tab bar hidden)
- close tab (tab bar hidden)
- toggle-frame-tab-bar (tab bar still hidden, because only 1 tab)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Tue, 16 Feb 2021 02:10:02 GMT) Full text and rfc822 format available.

Message #146 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Bastian Beranek <bastian.beischer <at> gmail.com>, Juri Linkov <juri <at> linkov.net>
Cc: "46299 <at> debbugs.gnu.org" <46299 <at> debbugs.gnu.org>
Subject: RE: [External] : bug#46299: 28.0.50; Value of tab-bar-show not
 respected in new frames.
Date: Tue, 16 Feb 2021 02:08:56 +0000
> > Good idea.  Actually frame parameters could
> > serve as a kind of "frame-local variables".

I haven't followed this thread.  Just happened
to see that comment.

Frame parameters are already, in effect,
frame-local "variables".  Just as symbol
properties are symbol-local "variables".
And text properties are char-local "variables".
And overlay properties are buffer position-local
"variables".

Nothing new about this.  At least that's how
I've always looked at it.

But perhaps you are doing something slightly
different/new, or there wouldn't be any reason
to say "could serve" - frame parameters do
serve as a kind of frame-local variables.  (We
just don't call them "variables".)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Tue, 16 Feb 2021 10:42:02 GMT) Full text and rfc822 format available.

Message #149 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 46299 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Tue, 16 Feb 2021 11:40:58 +0100
Eli Zaretskii <eliz <at> gnu.org> writes:
>
> I see the problem on GNU/Linux, but not on MS-Windows.

Interesting. I will report a new bug for this issue.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Tue, 16 Feb 2021 11:00:02 GMT) Full text and rfc822 format available.

Message #152 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Tue, 16 Feb 2021 11:59:14 +0100
[Message part 1 (text/plain, inline)]
Bastian Beranek <bastian.beischer <at> gmail.com> writes:

> But how to go back? It seems that tab-bar-show should go back to "1" (in
> order to make it a real toggle, i.e. it undoes itself). However, that
> means that after the second toggle-frame-tab-bar the tab-bar will either
> be shown or not, depending on the number of tabs opened at that specific
> time. We have to consider that the user created or closed tabs in
> between, so that means that there will be situations in which
> toggle-frame-tab-bar does not really seem to do anything... For example:
>
> - 1 tab (tab bar hidden)
> - create tab -> 2 tabs (tab bar shown)
> - toggle-frame-tab-bar (tab bar hidden)
> - close tab (tab bar hidden)
> - toggle-frame-tab-bar (tab bar still hidden, because only 1 tab)

I think what we could do is:

a) always toggle the current visibility: That seems to be a must, it
   would be confusing otherwise.

b) The first time toggle-frame-tab-bar is called, add a notice to the
   frame parameters that prevents tab-bar--update-tab-bar-lines from
   changing that frames tab-bar visibility. This means that after the
   toggle the tab-bar visibility keeps its state until
   toggle-frame-tab-bar is called again.

c) When it is called a second time toggle-frame-tab-bar sets the new
   frame parameter to nil and flips the visibility again (see a).

d) Then the tab-bar--update-tab-bar-lines logic will be active again,
   but only after a new tab is created or closed on that frame.

This prevents the problem listed in my last mail, in the last step the
tab-bar will be shown after toggling (although tab-bar-show is 1 and
there is only one tab), but then once you create more tabs or close tabs
it will revert to the usual behavior.

Is this acceptable?

Proposed patch is attached.
[0001-Fix-behavior-of-toggle-frame-tab-bar-bug-46299.patch (text/x-patch, inline)]
From 5f158962c1bc63c10ebaa49357a16e45664d5579 Mon Sep 17 00:00:00 2001
From: Bastian Beranek <bastian.beischer <at> rwth-aachen.de>
Date: Tue, 16 Feb 2021 11:35:35 +0100
Subject: [PATCH] Fix behavior of toggle-frame-tab-bar (bug #46299)

* lisp/tab-bar.el (toggle-frame-tab-bar): Add frame attribute to
protect tab bar state from changing.
(tab-bar--update-tab-bar-lines): Check for new attribute
---
 lisp/tab-bar.el | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index 4e47ae2c10..2fa34385f1 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -161,7 +161,8 @@ tab-bar--update-tab-bar-lines
                          (t frames))))
     ;; Loop over all frames and update default-frame-alist
     (dolist (frame frame-lst)
-      (set-frame-parameter frame 'tab-bar-lines (tab-bar--tab-bar-lines-for-frame frame))))
+      (unless (frame-parameter frame 'tab-bar-lines-keep-state)
+        (set-frame-parameter frame 'tab-bar-lines (tab-bar--tab-bar-lines-for-frame frame)))))
   (when (eq frames t)
     (setq default-frame-alist
           (cons (cons 'tab-bar-lines (if (and tab-bar-mode (eq tab-bar-show t)) 1 0))
@@ -233,7 +234,9 @@ toggle-frame-tab-bar
   (add-hook 'after-make-frame-functions 'toggle-frame-tab-bar)"
   (interactive)
   (set-frame-parameter frame 'tab-bar-lines
-                       (if (> (frame-parameter frame 'tab-bar-lines) 0) 0 1)))
+                       (if (> (frame-parameter frame 'tab-bar-lines) 0) 0 1))
+  (set-frame-parameter frame 'tab-bar-lines-keep-state
+                       (not (frame-parameter nil 'tab-bar-lines-keep-state))))
 
 (defvar tab-bar-map (make-sparse-keymap)
   "Keymap for the tab bar.
-- 
2.30.1


Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Tue, 16 Feb 2021 15:33:02 GMT) Full text and rfc822 format available.

Message #155 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Bastian Beranek <bastian.beischer <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Tue, 16 Feb 2021 16:31:59 +0100
[Message part 1 (text/plain, inline)]
Bastian Beranek <bastian.beischer <at> gmail.com> writes:

> Is this acceptable?
>
> Proposed patch is attached.
>

I made a mistake in that patch: In (frame-parameter ... ) the first
argument should have been "frame" not "nil". Please find a fixed
version attached.

[0001-lisp-tab-bar.el-Fix-behavior-of-toggle-frame-tab-bar.patch (text/x-patch, inline)]
From d4d40915ad6537fdd11555dfed2273303c564fb9 Mon Sep 17 00:00:00 2001
From: Bastian Beranek <bastian.beischer <at> rwth-aachen.de>
Date: Tue, 16 Feb 2021 11:35:35 +0100
Subject: [PATCH] * lisp/tab-bar.el: Fix behavior of toggle-frame-tab-bar (bug
 #46299)

(toggle-frame-tab-bar): Add frame parameter to protect tab bar state.
(tab-bar--update-tab-bar-lines): Check parameter.
---
 lisp/tab-bar.el | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index 4e47ae2c10..f0210e1a42 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -161,7 +161,8 @@ tab-bar--update-tab-bar-lines
                          (t frames))))
     ;; Loop over all frames and update default-frame-alist
     (dolist (frame frame-lst)
-      (set-frame-parameter frame 'tab-bar-lines (tab-bar--tab-bar-lines-for-frame frame))))
+      (unless (frame-parameter frame 'tab-bar-lines-keep-state)
+        (set-frame-parameter frame 'tab-bar-lines (tab-bar--tab-bar-lines-for-frame frame)))))
   (when (eq frames t)
     (setq default-frame-alist
           (cons (cons 'tab-bar-lines (if (and tab-bar-mode (eq tab-bar-show t)) 1 0))
@@ -233,7 +234,9 @@ toggle-frame-tab-bar
   (add-hook 'after-make-frame-functions 'toggle-frame-tab-bar)"
   (interactive)
   (set-frame-parameter frame 'tab-bar-lines
-                       (if (> (frame-parameter frame 'tab-bar-lines) 0) 0 1)))
+                       (if (> (frame-parameter frame 'tab-bar-lines) 0) 0 1))
+  (set-frame-parameter frame 'tab-bar-lines-keep-state
+                       (not (frame-parameter frame 'tab-bar-lines-keep-state))))
 
 (defvar tab-bar-map (make-sparse-keymap)
   "Keymap for the tab bar.
-- 
2.30.1


Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Tue, 16 Feb 2021 17:31:01 GMT) Full text and rfc822 format available.

Message #158 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Bastian Beranek <bastian.beischer <at> gmail.com>
Cc: 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Tue, 16 Feb 2021 19:28:43 +0200
>> "frame-local variables".
>
> Right, that would be the idea. Is there precedent for this? It seems to
> be the easiest way forward, but I don't know if this is considered
> acceptable.

Window-local tab-line.el uses window-parameter all over the place,
so for frame-local tab-bar.el it's natural to use frame-parameter.

>> Is this acceptable?

Yes, I agree with your reasoning in previous messages.

>> Proposed patch is attached.
>
> I made a mistake in that patch: In (frame-parameter ... ) the first
> argument should have been "frame" not "nil". Please find a fixed
> version attached.

Thanks, now your patch is pushed to master.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46299; Package emacs. (Wed, 24 Feb 2021 18:48:02 GMT) Full text and rfc822 format available.

Message #161 received at 46299 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Bastian Beranek <bastian.beischer <at> gmail.com>
Cc: 46299 <at> debbugs.gnu.org
Subject: Re: bug#46299: 28.0.50; Value of tab-bar-show not respected in new
 frames.
Date: Wed, 24 Feb 2021 20:46:36 +0200
tags 46299 fixed
close 46299 28.0.50
thanks

> Thanks, now your patch is pushed to master.

It seems now everything works well, so I''m closing this bug report.
Please reopen if you have more ideas.




Added tag(s) fixed. Request was from Juri Linkov <juri <at> linkov.net> to control <at> debbugs.gnu.org. (Wed, 24 Feb 2021 18:48:02 GMT) Full text and rfc822 format available.

bug marked as fixed in version 28.0.50, send any further explanations to 46299 <at> debbugs.gnu.org and Bastian Beischer <bastian.beischer <at> gmail.com> Request was from Juri Linkov <juri <at> linkov.net> to control <at> debbugs.gnu.org. (Wed, 24 Feb 2021 18:48:02 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Thu, 25 Mar 2021 11:24:12 GMT) Full text and rfc822 format available.

This bug report was last modified 3 years and 235 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.