* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. @ 2021-02-04 16:14 Bastian Beischer 2021-02-05 8:54 ` Juri Linkov 0 siblings, 1 reply; 53+ messages in thread From: Bastian Beischer @ 2021-02-04 16:14 UTC (permalink / raw) To: 46299 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)) ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-04 16:14 bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames Bastian Beischer @ 2021-02-05 8:54 ` Juri Linkov 2021-02-05 10:10 ` Bastian Beranek 0 siblings, 1 reply; 53+ messages in thread From: Juri Linkov @ 2021-02-05 8:54 UTC (permalink / raw) To: Bastian Beischer; +Cc: 46299 [-- Attachment #1: Type: text/plain, Size: 735 bytes --] > 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: [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: tab-bar-show-default-frame-alist.patch --] [-- Type: text/x-diff, Size: 666 bytes --] 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 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-05 8:54 ` Juri Linkov @ 2021-02-05 10:10 ` Bastian Beranek 2021-02-05 14:11 ` Bastian Beranek 0 siblings, 1 reply; 53+ messages in thread From: Bastian Beranek @ 2021-02-05 10:10 UTC (permalink / raw) To: Juri Linkov; +Cc: 46299 Hey Juri, On Fri, Feb 5, 2021 at 10:22 AM Juri Linkov <juri@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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-05 10:10 ` Bastian Beranek @ 2021-02-05 14:11 ` Bastian Beranek 2021-02-06 12:16 ` Bastian Beranek 0 siblings, 1 reply; 53+ messages in thread From: Bastian Beranek @ 2021-02-05 14:11 UTC (permalink / raw) To: Juri Linkov; +Cc: 46299 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-05 14:11 ` Bastian Beranek @ 2021-02-06 12:16 ` Bastian Beranek 2021-02-07 19:05 ` Juri Linkov 0 siblings, 1 reply; 53+ messages in thread From: Bastian Beranek @ 2021-02-06 12:16 UTC (permalink / raw) To: Juri Linkov; +Cc: 46299 [-- Attachment #1: Type: text/plain, Size: 1361 bytes --] 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@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 [-- Attachment #2: tab-bar.patch --] [-- Type: text/x-patch, Size: 2183 bytes --] diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el index 6720d82b47..1741173bbe 100644 --- a/lisp/tab-bar.el +++ b/lisp/tab-bar.el @@ -139,16 +139,21 @@ Possible modifier keys are `control', `meta', `shift', `hyper', `super' and :global t ;; It's defined in C/cus-start, this stops the d-m-m macro defining it again. :variable tab-bar-mode - (let ((val (if tab-bar-mode 1 0))) + + ;; Set frame parameters. + (let ((defaultval (if (and tab-bar-mode tab-bar-show) 1 0))) (dolist (frame (frame-list)) - (set-frame-parameter frame 'tab-bar-lines val)) - ;; 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))))) + (let ((frameval (if (natnump tab-bar-show) + (if (and defaultval + (> (length (funcall tab-bar-tabs-function frame)) tab-bar-show)) + 1 0) + defaultval))) + (set-frame-parameter frame 'tab-bar-lines frameval))) + (let ((newframeval (if (and defaultval (eq tab-bar-show t)) 1 0))) + (setq default-frame-alist + (cons (cons 'tab-bar-lines newframeval) + (assq-delete-all 'tab-bar-lines default-frame-alist))))) + (when tab-bar-mode (tab-bar--load-buttons)) (if tab-bar-mode @@ -251,16 +256,7 @@ you can use the command `toggle-frame-tab-bar'." :set (lambda (sym val) (set-default sym val) ;; Preload button images - (tab-bar-mode 1) - ;; Then handle each frame individually - (dolist (frame (frame-list)) - (set-frame-parameter - frame 'tab-bar-lines - (if (or (eq val t) - (and (natnump val) - (> (length (funcall tab-bar-tabs-function frame)) - val))) - 1 0)))) + (tab-bar-mode 1)) :group 'tab-bar :version "27.1") ^ permalink raw reply related [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-06 12:16 ` Bastian Beranek @ 2021-02-07 19:05 ` Juri Linkov 2021-02-07 23:03 ` Bastian Beranek 0 siblings, 1 reply; 53+ messages in thread From: Juri Linkov @ 2021-02-07 19:05 UTC (permalink / raw) To: Bastian Beranek; +Cc: 46299 > 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. ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-07 19:05 ` Juri Linkov @ 2021-02-07 23:03 ` Bastian Beranek 2021-02-08 17:50 ` Bastian Beranek 0 siblings, 1 reply; 53+ messages in thread From: Bastian Beranek @ 2021-02-07 23:03 UTC (permalink / raw) To: Juri Linkov; +Cc: 46299 [-- Attachment #1: Type: text/plain, Size: 999 bytes --] Hello Juri, On Sun, Feb 7, 2021 at 8:48 PM Juri Linkov <juri@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 [-- Attachment #2: tab-bar_v2.patch --] [-- Type: text/x-patch, Size: 4561 bytes --] diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el index 6720d82b47..03ddd75881 100644 --- a/lisp/tab-bar.el +++ b/lisp/tab-bar.el @@ -134,21 +134,30 @@ Possible modifier keys are `control', `meta', `shift', `hyper', `super' and :ascent center)) tab-bar-close-button))) +(defun tab-bar--update-tab-bar-lines () + ;; Set frame parameters. + (let ((defaultval (if (and tab-bar-mode tab-bar-show) 1 0))) + (dolist (frame (frame-list)) + (let ((frameval (if (natnump tab-bar-show) + (if (and defaultval + (> (length (funcall tab-bar-tabs-function frame)) tab-bar-show)) + 1 0) + defaultval))) + (set-frame-parameter frame 'tab-bar-lines frameval))) + (let ((newframeval (if (and defaultval (eq tab-bar-show t)) 1 0))) + (setq default-frame-alist + (cons (cons 'tab-bar-lines newframeval) + (assq-delete-all 'tab-bar-lines default-frame-alist)))))) + (define-minor-mode tab-bar-mode "Toggle the tab bar in all graphical frames (Tab Bar mode)." :global t ;; It's defined in C/cus-start, this stops the d-m-m macro defining it again. :variable tab-bar-mode - (let ((val (if tab-bar-mode 1 0))) - (dolist (frame (frame-list)) - (set-frame-parameter frame 'tab-bar-lines val)) - ;; 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))))) + + ;; Recalculate tab-bar-lines for all frames + (tab-bar--update-tab-bar-lines) + (when tab-bar-mode (tab-bar--load-buttons)) (if tab-bar-mode @@ -252,15 +261,7 @@ you can use the command `toggle-frame-tab-bar'." (set-default sym val) ;; Preload button images (tab-bar-mode 1) - ;; Then handle each frame individually - (dolist (frame (frame-list)) - (set-frame-parameter - frame 'tab-bar-lines - (if (or (eq val t) - (and (natnump val) - (> (length (funcall tab-bar-tabs-function frame)) - val))) - 1 0)))) + (tab-bar--update-tab-bar-lines)) :group 'tab-bar :version "27.1") @@ -852,16 +853,11 @@ After the tab is created, the hooks in (run-hook-with-args 'tab-bar-tab-post-open-functions (nth to-index tabs))) - (cond - ((eq tab-bar-show t) - (tab-bar-mode 1)) - ((and (natnump tab-bar-show) - (> (length (funcall tab-bar-tabs-function)) tab-bar-show) - (zerop (frame-parameter nil 'tab-bar-lines))) - (progn - (tab-bar--load-buttons) - (tab-bar--define-keys) - (set-frame-parameter nil 'tab-bar-lines 1)))) + ;; Recalculate tab-bar-lines and update frames + (tab-bar--update-tab-bar-lines) + (when tab-bar-mode + (tab-bar--load-buttons) + (tab-bar--define-keys)) (force-mode-line-update) (unless tab-bar-mode @@ -996,11 +992,8 @@ for the last tab on a frame is determined by tab-bar-closed-tabs) (set-frame-parameter nil 'tabs (delq close-tab tabs))) - (when (and (not (zerop (frame-parameter nil 'tab-bar-lines))) - (natnump tab-bar-show) - (<= (length (funcall tab-bar-tabs-function)) - tab-bar-show)) - (set-frame-parameter nil 'tab-bar-lines 0)) + ;; Recalculate tab-bar-lines and update frames + (tab-bar--update-tab-bar-lines) (force-mode-line-update) (unless tab-bar-mode @@ -1036,11 +1029,8 @@ for the last tab on a frame is determined by (run-hook-with-args 'tab-bar-tab-pre-close-functions (nth index tabs) nil))) (set-frame-parameter nil 'tabs (list (nth current-index tabs))) - (when (and (not (zerop (frame-parameter nil 'tab-bar-lines))) - (natnump tab-bar-show) - (<= (length (funcall tab-bar-tabs-function)) - tab-bar-show)) - (set-frame-parameter nil 'tab-bar-lines 0)) + ;; Recalculate tab-bar-lines and update frames + (tab-bar--update-tab-bar-lines) (force-mode-line-update) (unless tab-bar-mode ^ permalink raw reply related [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-07 23:03 ` Bastian Beranek @ 2021-02-08 17:50 ` Bastian Beranek 2021-02-08 18:19 ` Juri Linkov 0 siblings, 1 reply; 53+ messages in thread From: Bastian Beranek @ 2021-02-08 17:50 UTC (permalink / raw) To: Juri Linkov; +Cc: 46299 [-- Attachment #1: Type: text/plain, Size: 108 bytes --] Hello Juri, I have tried to clean up my patch a bit more, please see the attached version 3. Best Bastian [-- Attachment #2: tab-bar_v3.patch --] [-- Type: text/x-patch, Size: 5064 bytes --] diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el index 6720d82b47..8b94c242bc 100644 --- a/lisp/tab-bar.el +++ b/lisp/tab-bar.el @@ -134,21 +134,41 @@ Possible modifier keys are `control', `meta', `shift', `hyper', `super' and :ascent center)) 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." + (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)))) + +(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." + (if frame + ;; Set frame parameters for the given frame + (set-frame-parameter frame 'tab-bar-lines (tab-bar--tab-bar-lines-for-frame frame)) + (progn + ;; Loop over all frames and update default-frame-alist + (dolist (frame (frame-list)) + (set-frame-parameter frame 'tab-bar-lines (tab-bar--tab-bar-lines-for-frame frame))) + (setq default-frame-alist + (cons (cons 'tab-bar-lines (if (and tab-bar-mode (eq tab-bar-show t)) 1 0)) + (assq-delete-all 'tab-bar-lines default-frame-alist)))))) + (define-minor-mode tab-bar-mode "Toggle the tab bar in all graphical frames (Tab Bar mode)." :global t ;; It's defined in C/cus-start, this stops the d-m-m macro defining it again. :variable tab-bar-mode - (let ((val (if tab-bar-mode 1 0))) - (dolist (frame (frame-list)) - (set-frame-parameter frame 'tab-bar-lines val)) - ;; 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))))) + + ;; Recalculate tab-bar-lines for all frames + (tab-bar--update-tab-bar-lines) + (when tab-bar-mode (tab-bar--load-buttons)) (if tab-bar-mode @@ -251,16 +271,8 @@ you can use the command `toggle-frame-tab-bar'." :set (lambda (sym val) (set-default sym val) ;; Preload button images - (tab-bar-mode 1) - ;; Then handle each frame individually - (dolist (frame (frame-list)) - (set-frame-parameter - frame 'tab-bar-lines - (if (or (eq val t) - (and (natnump val) - (> (length (funcall tab-bar-tabs-function frame)) - val))) - 1 0)))) + ;; Note: tab-bar-mode updates tab-bar-lines as well. + (tab-bar-mode 1)) :group 'tab-bar :version "27.1") @@ -852,16 +864,11 @@ After the tab is created, the hooks in (run-hook-with-args 'tab-bar-tab-post-open-functions (nth to-index tabs))) - (cond - ((eq tab-bar-show t) - (tab-bar-mode 1)) - ((and (natnump tab-bar-show) - (> (length (funcall tab-bar-tabs-function)) tab-bar-show) - (zerop (frame-parameter nil 'tab-bar-lines))) - (progn - (tab-bar--load-buttons) - (tab-bar--define-keys) - (set-frame-parameter nil 'tab-bar-lines 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)) (force-mode-line-update) (unless tab-bar-mode @@ -996,11 +1003,8 @@ for the last tab on a frame is determined by tab-bar-closed-tabs) (set-frame-parameter nil 'tabs (delq close-tab tabs))) - (when (and (not (zerop (frame-parameter nil 'tab-bar-lines))) - (natnump tab-bar-show) - (<= (length (funcall tab-bar-tabs-function)) - tab-bar-show)) - (set-frame-parameter nil 'tab-bar-lines 0)) + ;; Recalculate tab-bar-lines and update frames + (tab-bar--update-tab-bar-lines (selected-frame)) (force-mode-line-update) (unless tab-bar-mode @@ -1036,11 +1040,8 @@ for the last tab on a frame is determined by (run-hook-with-args 'tab-bar-tab-pre-close-functions (nth index tabs) nil))) (set-frame-parameter nil 'tabs (list (nth current-index tabs))) - (when (and (not (zerop (frame-parameter nil 'tab-bar-lines))) - (natnump tab-bar-show) - (<= (length (funcall tab-bar-tabs-function)) - tab-bar-show)) - (set-frame-parameter nil 'tab-bar-lines 0)) + ;; Recalculate tab-bar-lines and update frames + (tab-bar--update-tab-bar-lines) (force-mode-line-update) (unless tab-bar-mode ^ permalink raw reply related [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-08 17:50 ` Bastian Beranek @ 2021-02-08 18:19 ` Juri Linkov 2021-02-08 19:04 ` Bastian Beranek 0 siblings, 1 reply; 53+ messages in thread From: Juri Linkov @ 2021-02-08 18:19 UTC (permalink / raw) To: Bastian Beranek; +Cc: 46299 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)) ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-08 18:19 ` Juri Linkov @ 2021-02-08 19:04 ` Bastian Beranek 2021-02-09 8:00 ` martin rudalics 0 siblings, 1 reply; 53+ messages in thread From: Bastian Beranek @ 2021-02-08 19:04 UTC (permalink / raw) To: Juri Linkov; +Cc: 46299 [-- Attachment #1: Type: text/plain, Size: 1336 bytes --] Hello Juri, On Mon, Feb 8, 2021 at 7:23 PM Juri Linkov <juri@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 [-- Attachment #2: tab-bar_v4.patch --] [-- Type: text/x-patch, Size: 5343 bytes --] commit 2673c365a878e7461ec1a4cba173738c2e6d754b Author: Bastian Beranek <bastian.beischer@rwth-aachen.de> Date: Mon Feb 8 18:12:33 2021 +0100 Fix issues with tab bar show. diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el index 6720d82b47..9666fb2460 100644 --- a/lisp/tab-bar.el +++ b/lisp/tab-bar.el @@ -134,21 +134,41 @@ Possible modifier keys are `control', `meta', `shift', `hyper', `super' and :ascent center)) 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." + (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)))) + +(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." + (if frame + ;; Set frame parameters for the given frame + (set-frame-parameter frame 'tab-bar-lines (tab-bar--tab-bar-lines-for-frame frame)) + (progn + ;; Loop over all frames and update default-frame-alist + (dolist (frame (frame-list)) + (set-frame-parameter frame 'tab-bar-lines (tab-bar--tab-bar-lines-for-frame frame))) + (setq default-frame-alist + (cons (cons 'tab-bar-lines (if (and tab-bar-mode (eq tab-bar-show t)) 1 0)) + (assq-delete-all 'tab-bar-lines default-frame-alist)))))) + (define-minor-mode tab-bar-mode "Toggle the tab bar in all graphical frames (Tab Bar mode)." :global t ;; It's defined in C/cus-start, this stops the d-m-m macro defining it again. :variable tab-bar-mode - (let ((val (if tab-bar-mode 1 0))) - (dolist (frame (frame-list)) - (set-frame-parameter frame 'tab-bar-lines val)) - ;; 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))))) + + ;; Recalculate tab-bar-lines for all frames + (tab-bar--update-tab-bar-lines) + (when tab-bar-mode (tab-bar--load-buttons)) (if tab-bar-mode @@ -251,16 +271,8 @@ you can use the command `toggle-frame-tab-bar'." :set (lambda (sym val) (set-default sym val) ;; Preload button images - (tab-bar-mode 1) - ;; Then handle each frame individually - (dolist (frame (frame-list)) - (set-frame-parameter - frame 'tab-bar-lines - (if (or (eq val t) - (and (natnump val) - (> (length (funcall tab-bar-tabs-function frame)) - val))) - 1 0)))) + ;; Note: tab-bar-mode updates tab-bar-lines as well. + (tab-bar-mode 1)) :group 'tab-bar :version "27.1") @@ -852,16 +864,15 @@ After the tab is created, the hooks in (run-hook-with-args 'tab-bar-tab-post-open-functions (nth to-index tabs))) - (cond - ((eq tab-bar-show t) + ;; Switch on tab-bar-mode, since a tab was created + (when tab-bar-show (tab-bar-mode 1)) - ((and (natnump tab-bar-show) - (> (length (funcall tab-bar-tabs-function)) tab-bar-show) - (zerop (frame-parameter nil 'tab-bar-lines))) - (progn - (tab-bar--load-buttons) - (tab-bar--define-keys) - (set-frame-parameter nil 'tab-bar-lines 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)) (force-mode-line-update) (unless tab-bar-mode @@ -996,11 +1007,8 @@ for the last tab on a frame is determined by tab-bar-closed-tabs) (set-frame-parameter nil 'tabs (delq close-tab tabs))) - (when (and (not (zerop (frame-parameter nil 'tab-bar-lines))) - (natnump tab-bar-show) - (<= (length (funcall tab-bar-tabs-function)) - tab-bar-show)) - (set-frame-parameter nil 'tab-bar-lines 0)) + ;; Recalculate tab-bar-lines and update frames + (tab-bar--update-tab-bar-lines (selected-frame)) (force-mode-line-update) (unless tab-bar-mode @@ -1036,11 +1044,8 @@ for the last tab on a frame is determined by (run-hook-with-args 'tab-bar-tab-pre-close-functions (nth index tabs) nil))) (set-frame-parameter nil 'tabs (list (nth current-index tabs))) - (when (and (not (zerop (frame-parameter nil 'tab-bar-lines))) - (natnump tab-bar-show) - (<= (length (funcall tab-bar-tabs-function)) - tab-bar-show)) - (set-frame-parameter nil 'tab-bar-lines 0)) + ;; Recalculate tab-bar-lines and update frames + (tab-bar--update-tab-bar-lines (selected-frame)) (force-mode-line-update) (unless tab-bar-mode ^ permalink raw reply related [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-08 19:04 ` Bastian Beranek @ 2021-02-09 8:00 ` martin rudalics 2021-02-09 8:15 ` Bastian Beranek 0 siblings, 1 reply; 53+ messages in thread From: martin rudalics @ 2021-02-09 8:00 UTC (permalink / raw) To: Bastian Beranek, Juri Linkov; +Cc: 46299 > + "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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-09 8:00 ` martin rudalics @ 2021-02-09 8:15 ` Bastian Beranek 2021-02-09 8:58 ` martin rudalics 2021-02-10 18:20 ` Juri Linkov 0 siblings, 2 replies; 53+ messages in thread From: Bastian Beranek @ 2021-02-09 8:15 UTC (permalink / raw) To: martin rudalics; +Cc: 46299, Juri Linkov Hey Martin, On Tue, Feb 9, 2021 at 9:00 AM martin rudalics <rudalics@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 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-09 8:15 ` Bastian Beranek @ 2021-02-09 8:58 ` martin rudalics 2021-02-09 9:23 ` Bastian Beranek 2021-02-10 18:20 ` Juri Linkov 1 sibling, 1 reply; 53+ messages in thread From: martin rudalics @ 2021-02-09 8:58 UTC (permalink / raw) To: Bastian Beranek; +Cc: 46299, Juri Linkov > 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-09 8:58 ` martin rudalics @ 2021-02-09 9:23 ` Bastian Beranek 2021-02-09 9:45 ` martin rudalics 0 siblings, 1 reply; 53+ messages in thread From: Bastian Beranek @ 2021-02-09 9:23 UTC (permalink / raw) To: martin rudalics; +Cc: 46299, Juri Linkov Hello Martin, On Tue, Feb 9, 2021 at 9:58 AM martin rudalics <rudalics@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 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-09 9:23 ` Bastian Beranek @ 2021-02-09 9:45 ` martin rudalics 2021-02-09 11:44 ` Bastian Beranek 0 siblings, 1 reply; 53+ messages in thread From: martin rudalics @ 2021-02-09 9:45 UTC (permalink / raw) To: Bastian Beranek; +Cc: 46299, Juri Linkov > + "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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-09 9:45 ` martin rudalics @ 2021-02-09 11:44 ` Bastian Beranek 2021-02-09 17:32 ` martin rudalics ` (2 more replies) 0 siblings, 3 replies; 53+ messages in thread From: Bastian Beranek @ 2021-02-09 11:44 UTC (permalink / raw) To: martin rudalics; +Cc: 46299, Juri Linkov [-- Attachment #1: Type: text/plain, Size: 1972 bytes --] On Tue, Feb 9, 2021 at 10:45 AM martin rudalics <rudalics@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 [-- Attachment #2: tab-bar_v5.patch --] [-- Type: text/x-patch, Size: 5499 bytes --] commit c642812d035dddb0dce0ed8c8ba895dd3559bf4c Author: Bastian Beranek <bastian.beischer@rwth-aachen.de> Date: Mon Feb 8 18:12:33 2021 +0100 Fix issues with tab bar show. diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el index 6720d82b47..37ca6dcff4 100644 --- a/lisp/tab-bar.el +++ b/lisp/tab-bar.el @@ -134,21 +134,44 @@ Possible modifier keys are `control', `meta', `shift', `hyper', `super' and :ascent center)) tab-bar-close-button))) +(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." + (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)))) + +(defun tab-bar--update-tab-bar-lines (&optional frame) + "Update the `tab-bar-lines' 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)) + (progn + ;; Loop over all frames and update default-frame-alist + (dolist (frame (frame-list)) + (set-frame-parameter frame 'tab-bar-lines (tab-bar--tab-bar-lines-for-frame frame))) + (setq default-frame-alist + (cons (cons 'tab-bar-lines (if (and tab-bar-mode (eq tab-bar-show t)) 1 0)) + (assq-delete-all 'tab-bar-lines default-frame-alist)))))) + (define-minor-mode tab-bar-mode "Toggle the tab bar in all graphical frames (Tab Bar mode)." :global t ;; It's defined in C/cus-start, this stops the d-m-m macro defining it again. :variable tab-bar-mode - (let ((val (if tab-bar-mode 1 0))) - (dolist (frame (frame-list)) - (set-frame-parameter frame 'tab-bar-lines val)) - ;; 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))))) + + ;; Recalculate tab-bar-lines for all frames + (tab-bar--update-tab-bar-lines) + (when tab-bar-mode (tab-bar--load-buttons)) (if tab-bar-mode @@ -251,16 +274,8 @@ you can use the command `toggle-frame-tab-bar'." :set (lambda (sym val) (set-default sym val) ;; Preload button images - (tab-bar-mode 1) - ;; Then handle each frame individually - (dolist (frame (frame-list)) - (set-frame-parameter - frame 'tab-bar-lines - (if (or (eq val t) - (and (natnump val) - (> (length (funcall tab-bar-tabs-function frame)) - val))) - 1 0)))) + ;; Note: tab-bar-mode updates tab-bar-lines as well. + (tab-bar-mode 1)) :group 'tab-bar :version "27.1") @@ -852,16 +867,15 @@ After the tab is created, the hooks in (run-hook-with-args 'tab-bar-tab-post-open-functions (nth to-index tabs))) - (cond - ((eq tab-bar-show t) + ;; Switch on tab-bar-mode, since a tab was created + (when tab-bar-show (tab-bar-mode 1)) - ((and (natnump tab-bar-show) - (> (length (funcall tab-bar-tabs-function)) tab-bar-show) - (zerop (frame-parameter nil 'tab-bar-lines))) - (progn - (tab-bar--load-buttons) - (tab-bar--define-keys) - (set-frame-parameter nil 'tab-bar-lines 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)) (force-mode-line-update) (unless tab-bar-mode @@ -996,11 +1010,8 @@ for the last tab on a frame is determined by tab-bar-closed-tabs) (set-frame-parameter nil 'tabs (delq close-tab tabs))) - (when (and (not (zerop (frame-parameter nil 'tab-bar-lines))) - (natnump tab-bar-show) - (<= (length (funcall tab-bar-tabs-function)) - tab-bar-show)) - (set-frame-parameter nil 'tab-bar-lines 0)) + ;; Recalculate tab-bar-lines and update frames + (tab-bar--update-tab-bar-lines (selected-frame)) (force-mode-line-update) (unless tab-bar-mode @@ -1036,11 +1047,8 @@ for the last tab on a frame is determined by (run-hook-with-args 'tab-bar-tab-pre-close-functions (nth index tabs) nil))) (set-frame-parameter nil 'tabs (list (nth current-index tabs))) - (when (and (not (zerop (frame-parameter nil 'tab-bar-lines))) - (natnump tab-bar-show) - (<= (length (funcall tab-bar-tabs-function)) - tab-bar-show)) - (set-frame-parameter nil 'tab-bar-lines 0)) + ;; Recalculate tab-bar-lines and update frames + (tab-bar--update-tab-bar-lines (selected-frame)) (force-mode-line-update) (unless tab-bar-mode ^ permalink raw reply related [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-09 11:44 ` Bastian Beranek @ 2021-02-09 17:32 ` martin rudalics 2021-02-09 18:06 ` Juri Linkov 2021-02-10 18:24 ` Juri Linkov 2 siblings, 0 replies; 53+ messages in thread From: martin rudalics @ 2021-02-09 17:32 UTC (permalink / raw) To: Bastian Beranek; +Cc: 46299, Juri Linkov > (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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-09 11:44 ` Bastian Beranek 2021-02-09 17:32 ` martin rudalics @ 2021-02-09 18:06 ` Juri Linkov 2021-02-09 18:46 ` Bastian Beranek 2021-02-09 19:01 ` Eli Zaretskii 2021-02-10 18:24 ` Juri Linkov 2 siblings, 2 replies; 53+ messages in thread From: Juri Linkov @ 2021-02-09 18:06 UTC (permalink / raw) To: Bastian Beranek; +Cc: 46299 > 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-09 18:06 ` Juri Linkov @ 2021-02-09 18:46 ` Bastian Beranek 2021-02-09 19:08 ` Eli Zaretskii 2021-02-09 19:01 ` Eli Zaretskii 1 sibling, 1 reply; 53+ messages in thread From: Bastian Beranek @ 2021-02-09 18:46 UTC (permalink / raw) To: Juri Linkov, Eli Zaretskii, Lars Ingebrigtsen; +Cc: 46299 On Tue, Feb 9, 2021 at 7:15 PM Juri Linkov <juri@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? ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-09 18:46 ` Bastian Beranek @ 2021-02-09 19:08 ` Eli Zaretskii 0 siblings, 0 replies; 53+ messages in thread From: Eli Zaretskii @ 2021-02-09 19:08 UTC (permalink / raw) To: Bastian Beranek; +Cc: larsi, juri, 46299 > From: Bastian Beranek <bastian.beischer@gmail.com> > Date: Tue, 9 Feb 2021 19:46:14 +0100 > Cc: martin rudalics <rudalics@gmx.at>, 46299@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. ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-09 18:06 ` Juri Linkov 2021-02-09 18:46 ` Bastian Beranek @ 2021-02-09 19:01 ` Eli Zaretskii 1 sibling, 0 replies; 53+ messages in thread From: Eli Zaretskii @ 2021-02-09 19:01 UTC (permalink / raw) To: Juri Linkov; +Cc: 46299, bastian.beischer > From: Juri Linkov <juri@linkov.net> > Date: Tue, 09 Feb 2021 20:06:07 +0200 > Cc: 46299@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. ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-09 11:44 ` Bastian Beranek 2021-02-09 17:32 ` martin rudalics 2021-02-09 18:06 ` Juri Linkov @ 2021-02-10 18:24 ` Juri Linkov 2021-02-11 12:14 ` Bastian Beranek 2 siblings, 1 reply; 53+ messages in thread From: Juri Linkov @ 2021-02-10 18:24 UTC (permalink / raw) To: Bastian Beranek; +Cc: 46299 > 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)) ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-10 18:24 ` Juri Linkov @ 2021-02-11 12:14 ` Bastian Beranek 2021-02-11 17:34 ` Bastian Beranek 2021-02-12 9:31 ` Juri Linkov 0 siblings, 2 replies; 53+ messages in thread From: Bastian Beranek @ 2021-02-11 12:14 UTC (permalink / raw) To: Juri Linkov; +Cc: 46299 [-- Attachment #1: Type: text/plain, Size: 4422 bytes --] 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@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@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. [-- Attachment #2: tab-bar_v6.patch --] [-- Type: text/x-patch, Size: 5449 bytes --] diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el index 6720d82b47..7afbb96212 100644 --- a/lisp/tab-bar.el +++ b/lisp/tab-bar.el @@ -134,21 +134,47 @@ Possible modifier keys are `control', `meta', `shift', `hyper', `super' and :ascent center)) tab-bar-close-button))) +(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." + (cond + ((not tab-bar-mode) 0) + ((not tab-bar-show) 0) + ((eq tab-bar-show t) 1) + ((natnump tab-bar-show) + (if (> (length (funcall tab-bar-tabs-function frame)) tab-bar-show) 1 0)))) + +(defun tab-bar--update-tab-bar-lines (&optional frames) + "Update the `tab-bar-lines' parameter in frames. +Update the tab-bar-lines frame parameter. If the optional +parameter FRAMES is omitted, update only the currently selected +frame. If it is `t', update all frames as well as the default +for new frames. Otherwise FRAMES should be a list of frames to +update." + (let ((frame-lst (cond ((null frames) + (list (selected-frame))) + ((eq frames t) + (frame-list)) + (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)))) + (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)) + (assq-delete-all 'tab-bar-lines default-frame-alist))))) + (define-minor-mode tab-bar-mode "Toggle the tab bar in all graphical frames (Tab Bar mode)." :global t ;; It's defined in C/cus-start, this stops the d-m-m macro defining it again. :variable tab-bar-mode - (let ((val (if tab-bar-mode 1 0))) - (dolist (frame (frame-list)) - (set-frame-parameter frame 'tab-bar-lines val)) - ;; 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))))) + + ;; Recalculate tab-bar-lines for all frames + (tab-bar--update-tab-bar-lines t) + (when tab-bar-mode (tab-bar--load-buttons)) (if tab-bar-mode @@ -250,17 +276,8 @@ you can use the command `toggle-frame-tab-bar'." :initialize 'custom-initialize-default :set (lambda (sym val) (set-default sym val) - ;; Preload button images - (tab-bar-mode 1) - ;; Then handle each frame individually - (dolist (frame (frame-list)) - (set-frame-parameter - frame 'tab-bar-lines - (if (or (eq val t) - (and (natnump val) - (> (length (funcall tab-bar-tabs-function frame)) - val))) - 1 0)))) + ;; Recalculate tab-bar-lines for all frames + (tab-bar--update-tab-bar-lines t)) :group 'tab-bar :version "27.1") @@ -852,16 +869,14 @@ After the tab is created, the hooks in (run-hook-with-args 'tab-bar-tab-post-open-functions (nth to-index tabs))) - (cond - ((eq tab-bar-show t) + ;; Switch on tab-bar-mode, since a tab was created + ;; Note: This also updates tab-bar-lines + (when tab-bar-show (tab-bar-mode 1)) - ((and (natnump tab-bar-show) - (> (length (funcall tab-bar-tabs-function)) tab-bar-show) - (zerop (frame-parameter nil 'tab-bar-lines))) - (progn - (tab-bar--load-buttons) - (tab-bar--define-keys) - (set-frame-parameter nil 'tab-bar-lines 1)))) + + (when tab-bar-mode + (tab-bar--load-buttons) + (tab-bar--define-keys)) (force-mode-line-update) (unless tab-bar-mode @@ -996,11 +1011,8 @@ for the last tab on a frame is determined by tab-bar-closed-tabs) (set-frame-parameter nil 'tabs (delq close-tab tabs))) - (when (and (not (zerop (frame-parameter nil 'tab-bar-lines))) - (natnump tab-bar-show) - (<= (length (funcall tab-bar-tabs-function)) - tab-bar-show)) - (set-frame-parameter nil 'tab-bar-lines 0)) + ;; Recalculate tab-bar-lines and update frames + (tab-bar--update-tab-bar-lines) (force-mode-line-update) (unless tab-bar-mode @@ -1036,11 +1048,8 @@ for the last tab on a frame is determined by (run-hook-with-args 'tab-bar-tab-pre-close-functions (nth index tabs) nil))) (set-frame-parameter nil 'tabs (list (nth current-index tabs))) - (when (and (not (zerop (frame-parameter nil 'tab-bar-lines))) - (natnump tab-bar-show) - (<= (length (funcall tab-bar-tabs-function)) - tab-bar-show)) - (set-frame-parameter nil 'tab-bar-lines 0)) + ;; Recalculate tab-bar-lines and update frames + (tab-bar--update-tab-bar-lines) (force-mode-line-update) (unless tab-bar-mode ^ permalink raw reply related [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-11 12:14 ` Bastian Beranek @ 2021-02-11 17:34 ` Bastian Beranek 2021-02-12 9:31 ` Juri Linkov 1 sibling, 0 replies; 53+ messages in thread From: Bastian Beranek @ 2021-02-11 17:34 UTC (permalink / raw) To: Juri Linkov; +Cc: 46299 [-- Attachment #1: Type: text/plain, Size: 5363 bytes --] On Thu, Feb 11, 2021 at 1:14 PM Bastian Beranek <bastian.beischer@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@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@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. [-- Attachment #2: tab-bar_v7.patch --] [-- Type: text/x-patch, Size: 5290 bytes --] diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el index 6720d82b47..93573d8a75 100644 --- a/lisp/tab-bar.el +++ b/lisp/tab-bar.el @@ -134,21 +134,47 @@ tab-bar--load-buttons :ascent center)) tab-bar-close-button))) +(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." + (cond + ((not tab-bar-mode) 0) + ((not tab-bar-show) 0) + ((eq tab-bar-show t) 1) + ((natnump tab-bar-show) + (if (> (length (funcall tab-bar-tabs-function frame)) tab-bar-show) 1 0)))) + +(defun tab-bar--update-tab-bar-lines (&optional frames) + "Update the `tab-bar-lines' parameter in frames. +Update the tab-bar-lines frame parameter. If the optional +parameter FRAMES is omitted, update only the currently selected +frame. If it is `t', update all frames as well as the default +for new frames. Otherwise FRAMES should be a list of frames to +update." + (let ((frame-lst (cond ((null frames) + (list (selected-frame))) + ((eq frames t) + (frame-list)) + (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)))) + (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)) + (assq-delete-all 'tab-bar-lines default-frame-alist))))) + (define-minor-mode tab-bar-mode "Toggle the tab bar in all graphical frames (Tab Bar mode)." :global t ;; It's defined in C/cus-start, this stops the d-m-m macro defining it again. :variable tab-bar-mode - (let ((val (if tab-bar-mode 1 0))) - (dolist (frame (frame-list)) - (set-frame-parameter frame 'tab-bar-lines val)) - ;; 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))))) + + ;; Recalculate tab-bar-lines for all frames + (tab-bar--update-tab-bar-lines t) + (when tab-bar-mode (tab-bar--load-buttons)) (if tab-bar-mode @@ -250,17 +276,9 @@ tab-bar-show :initialize 'custom-initialize-default :set (lambda (sym val) (set-default sym val) - ;; Preload button images - (tab-bar-mode 1) - ;; Then handle each frame individually - (dolist (frame (frame-list)) - (set-frame-parameter - frame 'tab-bar-lines - (if (or (eq val t) - (and (natnump val) - (> (length (funcall tab-bar-tabs-function frame)) - val))) - 1 0)))) + (if val + (tab-bar-mode 1) + (tab-bar--update-tab-bar-lines t))) :group 'tab-bar :version "27.1") @@ -852,16 +870,14 @@ tab-bar-new-tab-to (run-hook-with-args 'tab-bar-tab-post-open-functions (nth to-index tabs))) - (cond - ((eq tab-bar-show t) + ;; Switch on tab-bar-mode, since a tab was created + ;; Note: This also updates tab-bar-lines + (when tab-bar-show (tab-bar-mode 1)) - ((and (natnump tab-bar-show) - (> (length (funcall tab-bar-tabs-function)) tab-bar-show) - (zerop (frame-parameter nil 'tab-bar-lines))) - (progn - (tab-bar--load-buttons) - (tab-bar--define-keys) - (set-frame-parameter nil 'tab-bar-lines 1)))) + + (when tab-bar-mode + (tab-bar--load-buttons) + (tab-bar--define-keys)) (force-mode-line-update) (unless tab-bar-mode @@ -996,11 +1012,8 @@ tab-bar-close-tab tab-bar-closed-tabs) (set-frame-parameter nil 'tabs (delq close-tab tabs))) - (when (and (not (zerop (frame-parameter nil 'tab-bar-lines))) - (natnump tab-bar-show) - (<= (length (funcall tab-bar-tabs-function)) - tab-bar-show)) - (set-frame-parameter nil 'tab-bar-lines 0)) + ;; Recalculate tab-bar-lines and update frames + (tab-bar--update-tab-bar-lines) (force-mode-line-update) (unless tab-bar-mode @@ -1036,11 +1049,8 @@ tab-bar-close-other-tabs (run-hook-with-args 'tab-bar-tab-pre-close-functions (nth index tabs) nil))) (set-frame-parameter nil 'tabs (list (nth current-index tabs))) - (when (and (not (zerop (frame-parameter nil 'tab-bar-lines))) - (natnump tab-bar-show) - (<= (length (funcall tab-bar-tabs-function)) - tab-bar-show)) - (set-frame-parameter nil 'tab-bar-lines 0)) + ;; Recalculate tab-bar-lines and update frames + (tab-bar--update-tab-bar-lines) (force-mode-line-update) (unless tab-bar-mode ^ permalink raw reply related [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-11 12:14 ` Bastian Beranek 2021-02-11 17:34 ` Bastian Beranek @ 2021-02-12 9:31 ` Juri Linkov 2021-02-12 10:24 ` Bastian Beranek 1 sibling, 1 reply; 53+ messages in thread From: Juri Linkov @ 2021-02-12 9:31 UTC (permalink / raw) To: Bastian Beranek; +Cc: 46299 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. ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-12 9:31 ` Juri Linkov @ 2021-02-12 10:24 ` Bastian Beranek 2021-02-12 14:47 ` Bastian Beranek 0 siblings, 1 reply; 53+ messages in thread From: Bastian Beranek @ 2021-02-12 10:24 UTC (permalink / raw) To: Juri Linkov; +Cc: 46299 [-- Attachment #1: Type: text/plain, Size: 2861 bytes --] Hey Juri, On Fri, Feb 12, 2021 at 10:57 AM Juri Linkov <juri@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 [-- Attachment #2: tab-bar_v8.patch --] [-- Type: text/x-patch, Size: 5315 bytes --] diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el index 6720d82b47..87c9fd719d 100644 --- a/lisp/tab-bar.el +++ b/lisp/tab-bar.el @@ -134,21 +134,47 @@ tab-bar--load-buttons :ascent center)) tab-bar-close-button))) +(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." + (cond + ((not tab-bar-mode) 0) + ((not tab-bar-show) 0) + ((eq tab-bar-show t) 1) + ((natnump tab-bar-show) + (if (> (length (funcall tab-bar-tabs-function frame)) tab-bar-show) 1 0)))) + +(defun tab-bar--update-tab-bar-lines (&optional frames) + "Update the `tab-bar-lines' parameter in frames. +Update the tab-bar-lines frame parameter. If the optional +parameter FRAMES is omitted, update only the currently selected +frame. If it is `t', update all frames as well as the default +for new frames. Otherwise FRAMES should be a list of frames to +update." + (let ((frame-lst (cond ((null frames) + (list (selected-frame))) + ((eq frames t) + (frame-list)) + (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)))) + (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)) + (assq-delete-all 'tab-bar-lines default-frame-alist))))) + (define-minor-mode tab-bar-mode "Toggle the tab bar in all graphical frames (Tab Bar mode)." :global t ;; It's defined in C/cus-start, this stops the d-m-m macro defining it again. :variable tab-bar-mode - (let ((val (if tab-bar-mode 1 0))) - (dolist (frame (frame-list)) - (set-frame-parameter frame 'tab-bar-lines val)) - ;; 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))))) + + ;; Recalculate tab-bar-lines for all frames + (tab-bar--update-tab-bar-lines t) + (when tab-bar-mode (tab-bar--load-buttons)) (if tab-bar-mode @@ -250,17 +276,9 @@ tab-bar-show :initialize 'custom-initialize-default :set (lambda (sym val) (set-default sym val) - ;; Preload button images - (tab-bar-mode 1) - ;; Then handle each frame individually - (dolist (frame (frame-list)) - (set-frame-parameter - frame 'tab-bar-lines - (if (or (eq val t) - (and (natnump val) - (> (length (funcall tab-bar-tabs-function frame)) - val))) - 1 0)))) + (if val + (tab-bar-mode 1) + (tab-bar--update-tab-bar-lines t))) :group 'tab-bar :version "27.1") @@ -852,16 +870,12 @@ tab-bar-new-tab-to (run-hook-with-args 'tab-bar-tab-post-open-functions (nth to-index tabs))) - (cond - ((eq tab-bar-show t) - (tab-bar-mode 1)) - ((and (natnump tab-bar-show) - (> (length (funcall tab-bar-tabs-function)) tab-bar-show) - (zerop (frame-parameter nil 'tab-bar-lines))) - (progn - (tab-bar--load-buttons) - (tab-bar--define-keys) - (set-frame-parameter nil 'tab-bar-lines 1)))) + (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))) (force-mode-line-update) (unless tab-bar-mode @@ -996,11 +1010,8 @@ tab-bar-close-tab tab-bar-closed-tabs) (set-frame-parameter nil 'tabs (delq close-tab tabs))) - (when (and (not (zerop (frame-parameter nil 'tab-bar-lines))) - (natnump tab-bar-show) - (<= (length (funcall tab-bar-tabs-function)) - tab-bar-show)) - (set-frame-parameter nil 'tab-bar-lines 0)) + ;; Recalculate tab-bar-lines and update frames + (tab-bar--update-tab-bar-lines) (force-mode-line-update) (unless tab-bar-mode @@ -1036,11 +1047,8 @@ tab-bar-close-other-tabs (run-hook-with-args 'tab-bar-tab-pre-close-functions (nth index tabs) nil))) (set-frame-parameter nil 'tabs (list (nth current-index tabs))) - (when (and (not (zerop (frame-parameter nil 'tab-bar-lines))) - (natnump tab-bar-show) - (<= (length (funcall tab-bar-tabs-function)) - tab-bar-show)) - (set-frame-parameter nil 'tab-bar-lines 0)) + ;; Recalculate tab-bar-lines and update frames + (tab-bar--update-tab-bar-lines) (force-mode-line-update) (unless tab-bar-mode ^ permalink raw reply related [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-12 10:24 ` Bastian Beranek @ 2021-02-12 14:47 ` Bastian Beranek 2021-02-12 19:23 ` Bastian Beranek 0 siblings, 1 reply; 53+ messages in thread From: Bastian Beranek @ 2021-02-12 14:47 UTC (permalink / raw) To: Juri Linkov; +Cc: 46299 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-12 14:47 ` Bastian Beranek @ 2021-02-12 19:23 ` Bastian Beranek 2021-02-13 18:23 ` Juri Linkov 0 siblings, 1 reply; 53+ messages in thread From: Bastian Beranek @ 2021-02-12 19:23 UTC (permalink / raw) To: Juri Linkov; +Cc: 46299 [-- Attachment #1: Type: text/plain, Size: 1661 bytes --] On Fri, Feb 12, 2021 at 3:47 PM Bastian Beranek <bastian.beischer@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 [-- Attachment #2: tab-bar_v9.patch --] [-- Type: text/x-patch, Size: 5662 bytes --] diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el index 6720d82b47..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") @@ -134,21 +135,47 @@ tab-bar--load-buttons :ascent center)) tab-bar-close-button))) +(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." + (cond + ((not tab-bar-mode) 0) + ((not tab-bar-show) 0) + ((eq tab-bar-show t) 1) + ((natnump tab-bar-show) + (if (> (length (funcall tab-bar-tabs-function frame)) tab-bar-show) 1 0)))) + +(defun tab-bar--update-tab-bar-lines (&optional frames) + "Update the `tab-bar-lines' parameter in frames. +Update the tab-bar-lines frame parameter. If the optional +parameter FRAMES is omitted, update only the currently selected +frame. If it is `t', update all frames as well as the default +for new frames. Otherwise FRAMES should be a list of frames to +update." + (let ((frame-lst (cond ((null frames) + (list (selected-frame))) + ((eq frames t) + (frame-list)) + (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)))) + (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)) + (assq-delete-all 'tab-bar-lines default-frame-alist))))) + (define-minor-mode tab-bar-mode "Toggle the tab bar in all graphical frames (Tab Bar mode)." :global t ;; It's defined in C/cus-start, this stops the d-m-m macro defining it again. :variable tab-bar-mode - (let ((val (if tab-bar-mode 1 0))) - (dolist (frame (frame-list)) - (set-frame-parameter frame 'tab-bar-lines val)) - ;; 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))))) + + ;; Recalculate tab-bar-lines for all frames + (tab-bar--update-tab-bar-lines t) + (when tab-bar-mode (tab-bar--load-buttons)) (if tab-bar-mode @@ -250,17 +277,9 @@ tab-bar-show :initialize 'custom-initialize-default :set (lambda (sym val) (set-default sym val) - ;; Preload button images - (tab-bar-mode 1) - ;; Then handle each frame individually - (dolist (frame (frame-list)) - (set-frame-parameter - frame 'tab-bar-lines - (if (or (eq val t) - (and (natnump val) - (> (length (funcall tab-bar-tabs-function frame)) - val))) - 1 0)))) + (if val + (tab-bar-mode 1) + (tab-bar--update-tab-bar-lines t))) :group 'tab-bar :version "27.1") @@ -852,16 +871,12 @@ tab-bar-new-tab-to (run-hook-with-args 'tab-bar-tab-post-open-functions (nth to-index tabs))) - (cond - ((eq tab-bar-show t) - (tab-bar-mode 1)) - ((and (natnump tab-bar-show) - (> (length (funcall tab-bar-tabs-function)) tab-bar-show) - (zerop (frame-parameter nil 'tab-bar-lines))) - (progn - (tab-bar--load-buttons) - (tab-bar--define-keys) - (set-frame-parameter nil 'tab-bar-lines 1)))) + (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))) (force-mode-line-update) (unless tab-bar-mode @@ -996,11 +1011,8 @@ tab-bar-close-tab tab-bar-closed-tabs) (set-frame-parameter nil 'tabs (delq close-tab tabs))) - (when (and (not (zerop (frame-parameter nil 'tab-bar-lines))) - (natnump tab-bar-show) - (<= (length (funcall tab-bar-tabs-function)) - tab-bar-show)) - (set-frame-parameter nil 'tab-bar-lines 0)) + ;; Recalculate tab-bar-lines and update frames + (tab-bar--update-tab-bar-lines) (force-mode-line-update) (unless tab-bar-mode @@ -1036,11 +1048,8 @@ tab-bar-close-other-tabs (run-hook-with-args 'tab-bar-tab-pre-close-functions (nth index tabs) nil))) (set-frame-parameter nil 'tabs (list (nth current-index tabs))) - (when (and (not (zerop (frame-parameter nil 'tab-bar-lines))) - (natnump tab-bar-show) - (<= (length (funcall tab-bar-tabs-function)) - tab-bar-show)) - (set-frame-parameter nil 'tab-bar-lines 0)) + ;; Recalculate tab-bar-lines and update frames + (tab-bar--update-tab-bar-lines) (force-mode-line-update) (unless tab-bar-mode ^ permalink raw reply related [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-12 19:23 ` Bastian Beranek @ 2021-02-13 18:23 ` Juri Linkov 2021-02-13 19:02 ` Bastian Beranek 2021-02-14 13:08 ` Bastian Beranek 0 siblings, 2 replies; 53+ messages in thread From: Juri Linkov @ 2021-02-13 18:23 UTC (permalink / raw) To: Bastian Beranek; +Cc: 46299 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. ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-13 18:23 ` Juri Linkov @ 2021-02-13 19:02 ` Bastian Beranek 2021-02-13 19:46 ` Juri Linkov 2021-02-14 13:08 ` Bastian Beranek 1 sibling, 1 reply; 53+ messages in thread From: Bastian Beranek @ 2021-02-13 19:02 UTC (permalink / raw) To: Juri Linkov; +Cc: 46299 On Sat, Feb 13, 2021 at 7:33 PM Juri Linkov <juri@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? ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-13 19:02 ` Bastian Beranek @ 2021-02-13 19:46 ` Juri Linkov 2021-02-14 18:44 ` Juri Linkov 0 siblings, 1 reply; 53+ messages in thread From: Juri Linkov @ 2021-02-13 19:46 UTC (permalink / raw) To: Bastian Beranek; +Cc: 46299 > 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) ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-13 19:46 ` Juri Linkov @ 2021-02-14 18:44 ` Juri Linkov 2021-02-15 10:05 ` Bastian Beranek 2021-02-15 15:26 ` Eli Zaretskii 0 siblings, 2 replies; 53+ messages in thread From: Juri Linkov @ 2021-02-14 18:44 UTC (permalink / raw) To: Bastian Beranek; +Cc: 46299 >> 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. ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-14 18:44 ` Juri Linkov @ 2021-02-15 10:05 ` Bastian Beranek 2021-02-15 15:26 ` Eli Zaretskii 1 sibling, 0 replies; 53+ messages in thread From: Bastian Beranek @ 2021-02-15 10:05 UTC (permalink / raw) To: Juri Linkov; +Cc: 46299 Juri Linkov <juri@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? ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-14 18:44 ` Juri Linkov 2021-02-15 10:05 ` Bastian Beranek @ 2021-02-15 15:26 ` Eli Zaretskii 2021-02-15 15:32 ` Bastian Beranek 1 sibling, 1 reply; 53+ messages in thread From: Eli Zaretskii @ 2021-02-15 15:26 UTC (permalink / raw) To: Juri Linkov; +Cc: 46299, bastian.beischer > From: Juri Linkov <juri@linkov.net> > Date: Sun, 14 Feb 2021 20:44:20 +0200 > Cc: 46299@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? ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-15 15:26 ` Eli Zaretskii @ 2021-02-15 15:32 ` Bastian Beranek 2021-02-15 15:53 ` Eli Zaretskii 0 siblings, 1 reply; 53+ messages in thread From: Bastian Beranek @ 2021-02-15 15:32 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 46299, Juri Linkov On Mon, Feb 15, 2021 at 4:26 PM Eli Zaretskii <eliz@gnu.org> wrote: > > > From: Juri Linkov <juri@linkov.net> > > Date: Sun, 14 Feb 2021 20:44:20 +0200 > > Cc: 46299@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. ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-15 15:32 ` Bastian Beranek @ 2021-02-15 15:53 ` Eli Zaretskii 2021-02-16 10:40 ` Bastian Beranek 0 siblings, 1 reply; 53+ messages in thread From: Eli Zaretskii @ 2021-02-15 15:53 UTC (permalink / raw) To: Bastian Beranek; +Cc: 46299, juri > From: Bastian Beranek <bastian.beischer@gmail.com> > Date: Mon, 15 Feb 2021 16:32:24 +0100 > Cc: Juri Linkov <juri@linkov.net>, 46299@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. ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-15 15:53 ` Eli Zaretskii @ 2021-02-16 10:40 ` Bastian Beranek 0 siblings, 0 replies; 53+ messages in thread From: Bastian Beranek @ 2021-02-16 10:40 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 46299, juri Eli Zaretskii <eliz@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. ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-13 18:23 ` Juri Linkov 2021-02-13 19:02 ` Bastian Beranek @ 2021-02-14 13:08 ` Bastian Beranek 2021-02-14 18:50 ` Juri Linkov 1 sibling, 1 reply; 53+ messages in thread From: Bastian Beranek @ 2021-02-14 13:08 UTC (permalink / raw) To: Juri Linkov; +Cc: 46299 [-- Attachment #1: Type: text/plain, Size: 330 bytes --] > 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 [-- Attachment #2: 0001-Fix-showing-and-hiding-of-tab-bar-on-new-frames-bug-.patch --] [-- Type: text/x-patch, Size: 6370 bytes --] From 21f51c67ff3b979d9d910d23ba533d9a716f4c74 Mon Sep 17 00:00:00 2001 From: Bastian Beranek <bastian.beischer@rwth-aachen.de> Date: Mon, 8 Feb 2021 18:12:33 +0100 Subject: [PATCH] Fix showing and hiding of tab-bar on new frames (bug#46299) * lisp/tab-bar.el (tab-bar--update-tab-bar-lines) (tab-bar--tab-bar-lines-for-frame): New functions to update value of tab-bar-lines in frames. (tab-bar-mode, tab-bar-new-tab-to, tab-bar-close-tab) (tab-bar-close-other-tab, tab-bar-show :set): Use new function. (tab-bar-select-tab-modifiers :set): Work around visual glitch. --- lisp/tab-bar.el | 95 +++++++++++++++++++++++++++---------------------- 1 file changed, 52 insertions(+), 43 deletions(-) diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el index 6720d82b47..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") @@ -134,21 +135,47 @@ tab-bar--load-buttons :ascent center)) tab-bar-close-button))) +(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." + (cond + ((not tab-bar-mode) 0) + ((not tab-bar-show) 0) + ((eq tab-bar-show t) 1) + ((natnump tab-bar-show) + (if (> (length (funcall tab-bar-tabs-function frame)) tab-bar-show) 1 0)))) + +(defun tab-bar--update-tab-bar-lines (&optional frames) + "Update the `tab-bar-lines' parameter in frames. +Update the tab-bar-lines frame parameter. If the optional +parameter FRAMES is omitted, update only the currently selected +frame. If it is `t', update all frames as well as the default +for new frames. Otherwise FRAMES should be a list of frames to +update." + (let ((frame-lst (cond ((null frames) + (list (selected-frame))) + ((eq frames t) + (frame-list)) + (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)))) + (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)) + (assq-delete-all 'tab-bar-lines default-frame-alist))))) + (define-minor-mode tab-bar-mode "Toggle the tab bar in all graphical frames (Tab Bar mode)." :global t ;; It's defined in C/cus-start, this stops the d-m-m macro defining it again. :variable tab-bar-mode - (let ((val (if tab-bar-mode 1 0))) - (dolist (frame (frame-list)) - (set-frame-parameter frame 'tab-bar-lines val)) - ;; 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))))) + + ;; Recalculate tab-bar-lines for all frames + (tab-bar--update-tab-bar-lines t) + (when tab-bar-mode (tab-bar--load-buttons)) (if tab-bar-mode @@ -250,17 +277,9 @@ tab-bar-show :initialize 'custom-initialize-default :set (lambda (sym val) (set-default sym val) - ;; Preload button images - (tab-bar-mode 1) - ;; Then handle each frame individually - (dolist (frame (frame-list)) - (set-frame-parameter - frame 'tab-bar-lines - (if (or (eq val t) - (and (natnump val) - (> (length (funcall tab-bar-tabs-function frame)) - val))) - 1 0)))) + (if val + (tab-bar-mode 1) + (tab-bar--update-tab-bar-lines t))) :group 'tab-bar :version "27.1") @@ -852,16 +871,12 @@ tab-bar-new-tab-to (run-hook-with-args 'tab-bar-tab-post-open-functions (nth to-index tabs))) - (cond - ((eq tab-bar-show t) - (tab-bar-mode 1)) - ((and (natnump tab-bar-show) - (> (length (funcall tab-bar-tabs-function)) tab-bar-show) - (zerop (frame-parameter nil 'tab-bar-lines))) - (progn - (tab-bar--load-buttons) - (tab-bar--define-keys) - (set-frame-parameter nil 'tab-bar-lines 1)))) + (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))) (force-mode-line-update) (unless tab-bar-mode @@ -996,11 +1011,8 @@ tab-bar-close-tab tab-bar-closed-tabs) (set-frame-parameter nil 'tabs (delq close-tab tabs))) - (when (and (not (zerop (frame-parameter nil 'tab-bar-lines))) - (natnump tab-bar-show) - (<= (length (funcall tab-bar-tabs-function)) - tab-bar-show)) - (set-frame-parameter nil 'tab-bar-lines 0)) + ;; Recalculate tab-bar-lines and update frames + (tab-bar--update-tab-bar-lines) (force-mode-line-update) (unless tab-bar-mode @@ -1036,11 +1048,8 @@ tab-bar-close-other-tabs (run-hook-with-args 'tab-bar-tab-pre-close-functions (nth index tabs) nil))) (set-frame-parameter nil 'tabs (list (nth current-index tabs))) - (when (and (not (zerop (frame-parameter nil 'tab-bar-lines))) - (natnump tab-bar-show) - (<= (length (funcall tab-bar-tabs-function)) - tab-bar-show)) - (set-frame-parameter nil 'tab-bar-lines 0)) + ;; Recalculate tab-bar-lines and update frames + (tab-bar--update-tab-bar-lines) (force-mode-line-update) (unless tab-bar-mode -- 2.30.1 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-14 13:08 ` Bastian Beranek @ 2021-02-14 18:50 ` Juri Linkov 2021-02-14 19:28 ` Juri Linkov ` (2 more replies) 0 siblings, 3 replies; 53+ messages in thread From: Juri Linkov @ 2021-02-14 18:50 UTC (permalink / raw) To: Bastian Beranek; +Cc: 46299 >> 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. ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-14 18:50 ` Juri Linkov @ 2021-02-14 19:28 ` Juri Linkov 2021-02-15 8:16 ` martin rudalics 2021-02-15 10:09 ` Bastian Beranek 2 siblings, 0 replies; 53+ messages in thread From: Juri Linkov @ 2021-02-14 19:28 UTC (permalink / raw) To: Bastian Beranek; +Cc: 46299 > 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'. ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-14 18:50 ` Juri Linkov 2021-02-14 19:28 ` Juri Linkov @ 2021-02-15 8:16 ` martin rudalics 2021-02-15 9:07 ` Juri Linkov 2021-02-15 10:12 ` Bastian Beranek 2021-02-15 10:09 ` Bastian Beranek 2 siblings, 2 replies; 53+ messages in thread From: martin rudalics @ 2021-02-15 8:16 UTC (permalink / raw) To: Juri Linkov, Bastian Beranek; +Cc: 46299 > 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-15 8:16 ` martin rudalics @ 2021-02-15 9:07 ` Juri Linkov 2021-02-15 10:08 ` martin rudalics 2021-02-15 10:12 ` Bastian Beranek 1 sibling, 1 reply; 53+ messages in thread From: Juri Linkov @ 2021-02-15 9:07 UTC (permalink / raw) To: martin rudalics; +Cc: 46299, Bastian Beranek >> 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. ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-15 9:07 ` Juri Linkov @ 2021-02-15 10:08 ` martin rudalics 0 siblings, 0 replies; 53+ messages in thread From: martin rudalics @ 2021-02-15 10:08 UTC (permalink / raw) To: Juri Linkov; +Cc: 46299, Bastian Beranek > '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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-15 8:16 ` martin rudalics 2021-02-15 9:07 ` Juri Linkov @ 2021-02-15 10:12 ` Bastian Beranek 1 sibling, 0 replies; 53+ messages in thread From: Bastian Beranek @ 2021-02-15 10:12 UTC (permalink / raw) To: martin rudalics; +Cc: 46299, Juri Linkov martin rudalics <rudalics@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. ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-14 18:50 ` Juri Linkov 2021-02-14 19:28 ` Juri Linkov 2021-02-15 8:16 ` martin rudalics @ 2021-02-15 10:09 ` Bastian Beranek 2021-02-15 17:01 ` Juri Linkov 2 siblings, 1 reply; 53+ messages in thread From: Bastian Beranek @ 2021-02-15 10:09 UTC (permalink / raw) To: Juri Linkov; +Cc: 46299 Juri Linkov <juri@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. ^ permalink raw reply related [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-15 10:09 ` Bastian Beranek @ 2021-02-15 17:01 ` Juri Linkov 2021-02-15 22:10 ` Bastian Beranek 0 siblings, 1 reply; 53+ messages in thread From: Juri Linkov @ 2021-02-15 17:01 UTC (permalink / raw) To: Bastian Beranek; +Cc: 46299 >> 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. ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-15 17:01 ` Juri Linkov @ 2021-02-15 22:10 ` Bastian Beranek 2021-02-16 2:08 ` bug#46299: [External] : " Drew Adams 2021-02-16 10:59 ` Bastian Beranek 0 siblings, 2 replies; 53+ messages in thread From: Bastian Beranek @ 2021-02-15 22:10 UTC (permalink / raw) To: Juri Linkov; +Cc: 46299 Juri Linkov <juri@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) ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: [External] : bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-15 22:10 ` Bastian Beranek @ 2021-02-16 2:08 ` Drew Adams 2021-02-16 10:59 ` Bastian Beranek 1 sibling, 0 replies; 53+ messages in thread From: Drew Adams @ 2021-02-16 2:08 UTC (permalink / raw) To: Bastian Beranek, Juri Linkov; +Cc: 46299@debbugs.gnu.org > > 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".) ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-15 22:10 ` Bastian Beranek 2021-02-16 2:08 ` bug#46299: [External] : " Drew Adams @ 2021-02-16 10:59 ` Bastian Beranek 2021-02-16 15:31 ` Bastian Beranek 1 sibling, 1 reply; 53+ messages in thread From: Bastian Beranek @ 2021-02-16 10:59 UTC (permalink / raw) To: Juri Linkov; +Cc: 46299 [-- Attachment #1: Type: text/plain, Size: 1781 bytes --] Bastian Beranek <bastian.beischer@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. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Fix-behavior-of-toggle-frame-tab-bar-bug-46299.patch --] [-- Type: text/x-patch, Size: 1725 bytes --] From 5f158962c1bc63c10ebaa49357a16e45664d5579 Mon Sep 17 00:00:00 2001 From: Bastian Beranek <bastian.beischer@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 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-16 10:59 ` Bastian Beranek @ 2021-02-16 15:31 ` Bastian Beranek 2021-02-16 17:28 ` Juri Linkov 0 siblings, 1 reply; 53+ messages in thread From: Bastian Beranek @ 2021-02-16 15:31 UTC (permalink / raw) To: Juri Linkov; +Cc: 46299 [-- Attachment #1: Type: text/plain, Size: 263 bytes --] Bastian Beranek <bastian.beischer@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. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-lisp-tab-bar.el-Fix-behavior-of-toggle-frame-tab-bar.patch --] [-- Type: text/x-patch, Size: 1708 bytes --] From d4d40915ad6537fdd11555dfed2273303c564fb9 Mon Sep 17 00:00:00 2001 From: Bastian Beranek <bastian.beischer@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 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-16 15:31 ` Bastian Beranek @ 2021-02-16 17:28 ` Juri Linkov 2021-02-24 18:46 ` Juri Linkov 0 siblings, 1 reply; 53+ messages in thread From: Juri Linkov @ 2021-02-16 17:28 UTC (permalink / raw) To: Bastian Beranek; +Cc: 46299 >> "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. ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-16 17:28 ` Juri Linkov @ 2021-02-24 18:46 ` Juri Linkov 0 siblings, 0 replies; 53+ messages in thread From: Juri Linkov @ 2021-02-24 18:46 UTC (permalink / raw) To: Bastian Beranek; +Cc: 46299 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. ^ permalink raw reply [flat|nested] 53+ messages in thread
* bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames. 2021-02-09 8:15 ` Bastian Beranek 2021-02-09 8:58 ` martin rudalics @ 2021-02-10 18:20 ` Juri Linkov 1 sibling, 0 replies; 53+ messages in thread From: Juri Linkov @ 2021-02-10 18:20 UTC (permalink / raw) To: Bastian Beranek; +Cc: 46299 >> > + "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) ^ permalink raw reply [flat|nested] 53+ messages in thread
end of thread, other threads:[~2021-02-24 18:46 UTC | newest] Thread overview: 53+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-02-04 16:14 bug#46299: 28.0.50; Value of tab-bar-show not respected in new frames Bastian Beischer 2021-02-05 8:54 ` Juri Linkov 2021-02-05 10:10 ` Bastian Beranek 2021-02-05 14:11 ` Bastian Beranek 2021-02-06 12:16 ` Bastian Beranek 2021-02-07 19:05 ` Juri Linkov 2021-02-07 23:03 ` Bastian Beranek 2021-02-08 17:50 ` Bastian Beranek 2021-02-08 18:19 ` Juri Linkov 2021-02-08 19:04 ` Bastian Beranek 2021-02-09 8:00 ` martin rudalics 2021-02-09 8:15 ` Bastian Beranek 2021-02-09 8:58 ` martin rudalics 2021-02-09 9:23 ` Bastian Beranek 2021-02-09 9:45 ` martin rudalics 2021-02-09 11:44 ` Bastian Beranek 2021-02-09 17:32 ` martin rudalics 2021-02-09 18:06 ` Juri Linkov 2021-02-09 18:46 ` Bastian Beranek 2021-02-09 19:08 ` Eli Zaretskii 2021-02-09 19:01 ` Eli Zaretskii 2021-02-10 18:24 ` Juri Linkov 2021-02-11 12:14 ` Bastian Beranek 2021-02-11 17:34 ` Bastian Beranek 2021-02-12 9:31 ` Juri Linkov 2021-02-12 10:24 ` Bastian Beranek 2021-02-12 14:47 ` Bastian Beranek 2021-02-12 19:23 ` Bastian Beranek 2021-02-13 18:23 ` Juri Linkov 2021-02-13 19:02 ` Bastian Beranek 2021-02-13 19:46 ` Juri Linkov 2021-02-14 18:44 ` Juri Linkov 2021-02-15 10:05 ` Bastian Beranek 2021-02-15 15:26 ` Eli Zaretskii 2021-02-15 15:32 ` Bastian Beranek 2021-02-15 15:53 ` Eli Zaretskii 2021-02-16 10:40 ` Bastian Beranek 2021-02-14 13:08 ` Bastian Beranek 2021-02-14 18:50 ` Juri Linkov 2021-02-14 19:28 ` Juri Linkov 2021-02-15 8:16 ` martin rudalics 2021-02-15 9:07 ` Juri Linkov 2021-02-15 10:08 ` martin rudalics 2021-02-15 10:12 ` Bastian Beranek 2021-02-15 10:09 ` Bastian Beranek 2021-02-15 17:01 ` Juri Linkov 2021-02-15 22:10 ` Bastian Beranek 2021-02-16 2:08 ` bug#46299: [External] : " Drew Adams 2021-02-16 10:59 ` Bastian Beranek 2021-02-16 15:31 ` Bastian Beranek 2021-02-16 17:28 ` Juri Linkov 2021-02-24 18:46 ` Juri Linkov 2021-02-10 18:20 ` Juri Linkov
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).