* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook @ 2019-03-05 22:57 Alexander Miller 2019-03-06 9:39 ` martin rudalics ` (3 more replies) 0 siblings, 4 replies; 72+ messages in thread From: Alexander Miller @ 2019-03-05 22:57 UTC (permalink / raw) To: 34765 To quote from the documentation of select-window: Selections that "really count" are those causing a visible change in the next redisplay of WINDOW’s frame and should be always recorded. So if you think of running a function each time a window gets selected put it on ‘buffer-list-update-hook’. A temporary buffer hardly fits these criteria. The problem is not just theoretical either. I have now run multiple times into situations where use of a temp buffer caused a feedback loop that makes buffer-list-update-hook fire permanently. For example a function called by buffer-list-update-hook uses with-selected-window, this causes a recalculation of the frame title, that calls format-spec, which uses a temp-buffer and we are back to step one. Granted this case is very specific to spacemacs and I am unable to reproduce it from emacs -q (with-selected-window does not recalculate the frame title here), but this is already the second time I've run into this (first time it was magit). So yeah, with-temp-buffer correction aside, if you have any advice how to avoid the issue on my end without going back to advising select-window that'd be great. In GNU Emacs 26.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.30) of 2018-07-05 built on juergen Windowing system distributor 'The X.Org Foundation', version 11.0.12003000 Recent messages: eval((spacemacs/title-prepare "%I@%SXXX")) redisplay_internal\ \(C\ function\)() spacemacs/title-prepare Mark set Mark saved where search started Quit [2 times] Mark saved where search started uncompressing format-spec.el.gz...done Note: file is write protected Configured using: 'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib --localstatedir=/var --with-x-toolkit=gtk3 --with-xft --with-modules 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong -fno-plt' CPPFLAGS=-D_FORTIFY_SOURCE=2 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now' Configured features: XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS NOTIFY ACL GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 MODULES THREADS LIBSYSTEMD LCMS2 Important settings: value of $LC_COLLATE: en_GB.UTF-8 value of $LANG: en_GB.UTF-8 locale-coding-system: utf-8-unix Major mode: Emacs-Lisp Minor modes in effect: global-mu4e-conversation-mode: t mu4e-conversation-mode: t global-magit-file-mode: t magit-file-mode: t global-git-commit-mode: t async-bytecomp-package-mode: t framey-mode: t helm-descbinds-mode: t helm-mode: t helm-flx-mode: t global-evil-surround-mode: t evil-surround-mode: t recentf-mode: t diff-auto-refine-mode: t treemacs-filewatch-mode: t treemacs-git-mode: deferred treemacs-fringe-indicator-mode: t evil-escape-mode: t global-display-line-numbers-mode: t display-line-numbers-mode: t global-git-gutter-mode: t git-gutter-mode: t company-flx-mode: t global-company-mode: t company-mode: t auto-compile-mode: t elisp-slime-nav-mode: t rainbow-mode: t goto-address-prog-mode: t bug-reference-prog-mode: t flycheck-pos-tip-mode: t global-flycheck-mode: t flycheck-mode: t yas-global-mode: t yas-minor-mode: t rainbow-delimiters-mode: t eros-mode: t global-subword-mode: t subword-mode: t eldoc-in-minibuffer-mode: t show-smartparens-global-mode: t show-smartparens-mode: t smartparens-mode: t winum-mode: t shackle-mode: t eyebrowse-mode: t evil-goggles-mode: t winner-mode: t save-place-mode: t savehist-mode: t which-key-mode: t override-global-mode: t global-undo-tree-mode: t undo-tree-mode: t shell-dirtrack-mode: t evil-mode: t evil-local-mode: t spacemacs-leader-override-mode: t global-spacemacs-leader-override-mode: t xterm-mouse-mode: t global-auto-revert-mode: t ido-vertical-mode: t global-page-break-lines-mode: t page-break-lines-mode: t global-eldoc-mode: t eldoc-mode: t electric-indent-mode: t mouse-wheel-mode: t prettify-symbols-mode: t file-name-shadow-mode: t global-font-lock-mode: t font-lock-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/a/.emacs.d/elpa/26.1/develop/lv-20181110.1740/lv hides /home/a/.emacs.d/elpa/26.1/develop/hydra-20181128.1716/lv /home/a/.emacs.d/elpa/26.1/develop/ht-20181216.1137/ht hides /home/a/.emacs.d/core/libs/ht /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-screen hides /usr/share/emacs/26.1/lisp/org/ob-screen /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-id hides /usr/share/emacs/26.1/lisp/org/org-id /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-sass hides /usr/share/emacs/26.1/lisp/org/ob-sass /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-R hides /usr/share/emacs/26.1/lisp/org/ob-R /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-inlinetask hides /usr/share/emacs/26.1/lisp/org/org-inlinetask /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-dot hides /usr/share/emacs/26.1/lisp/org/ob-dot /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ox-texinfo hides /usr/share/emacs/26.1/lisp/org/ox-texinfo /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-asymptote hides /usr/share/emacs/26.1/lisp/org/ob-asymptote /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-indent hides /usr/share/emacs/26.1/lisp/org/org-indent /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-shell hides /usr/share/emacs/26.1/lisp/org/ob-shell /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-C hides /usr/share/emacs/26.1/lisp/org/ob-C /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-lilypond hides /usr/share/emacs/26.1/lisp/org/ob-lilypond /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-eval hides /usr/share/emacs/26.1/lisp/org/ob-eval /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-comint hides /usr/share/emacs/26.1/lisp/org/ob-comint /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-perl hides /usr/share/emacs/26.1/lisp/org/ob-perl /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-capture hides /usr/share/emacs/26.1/lisp/org/org-capture /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-clojure hides /usr/share/emacs/26.1/lisp/org/ob-clojure /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-ebnf hides /usr/share/emacs/26.1/lisp/org/ob-ebnf /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-clock hides /usr/share/emacs/26.1/lisp/org/org-clock /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-latex hides /usr/share/emacs/26.1/lisp/org/ob-latex /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-calc hides /usr/share/emacs/26.1/lisp/org/ob-calc /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-pcomplete hides /usr/share/emacs/26.1/lisp/org/org-pcomplete /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-macro hides /usr/share/emacs/26.1/lisp/org/org-macro /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-footnote hides /usr/share/emacs/26.1/lisp/org/org-footnote /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-datetree hides /usr/share/emacs/26.1/lisp/org/org-datetree /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob hides /usr/share/emacs/26.1/lisp/org/ob /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ox-org hides /usr/share/emacs/26.1/lisp/org/ox-org /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ox-beamer hides /usr/share/emacs/26.1/lisp/org/ox-beamer /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-plot hides /usr/share/emacs/26.1/lisp/org/org-plot /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ox-md hides /usr/share/emacs/26.1/lisp/org/ox-md /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-timer hides /usr/share/emacs/26.1/lisp/org/org-timer /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-picolisp hides /usr/share/emacs/26.1/lisp/org/ob-picolisp /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-ditaa hides /usr/share/emacs/26.1/lisp/org/ob-ditaa /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-eshell hides /usr/share/emacs/26.1/lisp/org/org-eshell /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-tangle hides /usr/share/emacs/26.1/lisp/org/ob-tangle /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-gnus hides /usr/share/emacs/26.1/lisp/org/org-gnus /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ox-icalendar hides /usr/share/emacs/26.1/lisp/org/ox-icalendar /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-forth hides /usr/share/emacs/26.1/lisp/org/ob-forth /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-css hides /usr/share/emacs/26.1/lisp/org/ob-css /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-ref hides /usr/share/emacs/26.1/lisp/org/ob-ref /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-sed hides /usr/share/emacs/26.1/lisp/org/ob-sed /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-J hides /usr/share/emacs/26.1/lisp/org/ob-J /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-table hides /usr/share/emacs/26.1/lisp/org/org-table /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-plantuml hides /usr/share/emacs/26.1/lisp/org/ob-plantuml /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-compat hides /usr/share/emacs/26.1/lisp/org/org-compat /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org hides /usr/share/emacs/26.1/lisp/org/org /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-element hides /usr/share/emacs/26.1/lisp/org/org-element /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-bibtex hides /usr/share/emacs/26.1/lisp/org/org-bibtex /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-lisp hides /usr/share/emacs/26.1/lisp/org/ob-lisp /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-python hides /usr/share/emacs/26.1/lisp/org/ob-python /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-protocol hides /usr/share/emacs/26.1/lisp/org/org-protocol /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-java hides /usr/share/emacs/26.1/lisp/org/ob-java /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ox hides /usr/share/emacs/26.1/lisp/org/ox /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-entities hides /usr/share/emacs/26.1/lisp/org/org-entities /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-hledger hides /usr/share/emacs/26.1/lisp/org/ob-hledger /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-macs hides /usr/share/emacs/26.1/lisp/org/org-macs /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-sql hides /usr/share/emacs/26.1/lisp/org/ob-sql /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-irc hides /usr/share/emacs/26.1/lisp/org/org-irc /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-mouse hides /usr/share/emacs/26.1/lisp/org/org-mouse /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-core hides /usr/share/emacs/26.1/lisp/org/ob-core /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-matlab hides /usr/share/emacs/26.1/lisp/org/ob-matlab /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-crypt hides /usr/share/emacs/26.1/lisp/org/org-crypt /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-table hides /usr/share/emacs/26.1/lisp/org/ob-table /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-scheme hides /usr/share/emacs/26.1/lisp/org/ob-scheme /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-bbdb hides /usr/share/emacs/26.1/lisp/org/org-bbdb /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-habit hides /usr/share/emacs/26.1/lisp/org/org-habit /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-mhe hides /usr/share/emacs/26.1/lisp/org/org-mhe /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-octave hides /usr/share/emacs/26.1/lisp/org/ob-octave /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-org hides /usr/share/emacs/26.1/lisp/org/ob-org /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-rmail hides /usr/share/emacs/26.1/lisp/org/org-rmail /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-maxima hides /usr/share/emacs/26.1/lisp/org/ob-maxima /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ox-ascii hides /usr/share/emacs/26.1/lisp/org/ox-ascii /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-exp hides /usr/share/emacs/26.1/lisp/org/ob-exp /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-version hides /usr/share/emacs/26.1/lisp/org/org-version /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-io hides /usr/share/emacs/26.1/lisp/org/ob-io /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-agenda hides /usr/share/emacs/26.1/lisp/org/org-agenda /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-abc hides /usr/share/emacs/26.1/lisp/org/ob-abc /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-makefile hides /usr/share/emacs/26.1/lisp/org/ob-makefile /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-lint hides /usr/share/emacs/26.1/lisp/org/org-lint /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-js hides /usr/share/emacs/26.1/lisp/org/ob-js /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-loaddefs hides /usr/share/emacs/26.1/lisp/org/org-loaddefs /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ox-man hides /usr/share/emacs/26.1/lisp/org/ox-man /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-ruby hides /usr/share/emacs/26.1/lisp/org/ob-ruby /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-awk hides /usr/share/emacs/26.1/lisp/org/ob-awk /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-duration hides /usr/share/emacs/26.1/lisp/org/org-duration /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ox-odt hides /usr/share/emacs/26.1/lisp/org/ox-odt /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-mscgen hides /usr/share/emacs/26.1/lisp/org/ob-mscgen /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-keys hides /usr/share/emacs/26.1/lisp/org/ob-keys /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-archive hides /usr/share/emacs/26.1/lisp/org/org-archive /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-gnuplot hides /usr/share/emacs/26.1/lisp/org/ob-gnuplot /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-stan hides /usr/share/emacs/26.1/lisp/org/ob-stan /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-w3m hides /usr/share/emacs/26.1/lisp/org/org-w3m /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-colview hides /usr/share/emacs/26.1/lisp/org/org-colview /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ox-html hides /usr/share/emacs/26.1/lisp/org/ox-html /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-fortran hides /usr/share/emacs/26.1/lisp/org/ob-fortran /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-groovy hides /usr/share/emacs/26.1/lisp/org/ob-groovy /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-list hides /usr/share/emacs/26.1/lisp/org/org-list /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-faces hides /usr/share/emacs/26.1/lisp/org/org-faces /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-lob hides /usr/share/emacs/26.1/lisp/org/ob-lob /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-eww hides /usr/share/emacs/26.1/lisp/org/org-eww /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-lua hides /usr/share/emacs/26.1/lisp/org/ob-lua /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-feed hides /usr/share/emacs/26.1/lisp/org/org-feed /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-sqlite hides /usr/share/emacs/26.1/lisp/org/ob-sqlite /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-haskell hides /usr/share/emacs/26.1/lisp/org/ob-haskell /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-src hides /usr/share/emacs/26.1/lisp/org/org-src /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-install hides /usr/share/emacs/26.1/lisp/org/org-install /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-emacs-lisp hides /usr/share/emacs/26.1/lisp/org/ob-emacs-lisp /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-shen hides /usr/share/emacs/26.1/lisp/org/ob-shen /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ox-latex hides /usr/share/emacs/26.1/lisp/org/ox-latex /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-mobile hides /usr/share/emacs/26.1/lisp/org/org-mobile /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-processing hides /usr/share/emacs/26.1/lisp/org/ob-processing /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-attach hides /usr/share/emacs/26.1/lisp/org/org-attach /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-ledger hides /usr/share/emacs/26.1/lisp/org/ob-ledger /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-ctags hides /usr/share/emacs/26.1/lisp/org/org-ctags /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ox-publish hides /usr/share/emacs/26.1/lisp/org/ox-publish /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-ocaml hides /usr/share/emacs/26.1/lisp/org/ob-ocaml /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-vala hides /usr/share/emacs/26.1/lisp/org/ob-vala /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-info hides /usr/share/emacs/26.1/lisp/org/org-info /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/org-docview hides /usr/share/emacs/26.1/lisp/org/org-docview /home/a/.emacs.d/elpa/26.1/develop/org-plus-contrib-20181230/ob-coq hides /usr/share/emacs/26.1/lisp/org/ob-coq Features: (shadow sort mail-extr emacsbug helm-command evil-nerd-commenter evil-nerd-commenter-operator evil-nerd-commenter-sdk smartparens-html sgml-mode helm-xref semantic/symref/grep grep semantic/symref helm-ag helm-elisp helm-eval org-indent org-table company-shell fish-mode org-clock diary-lib diary-loaddefs cal-iso vc-mtn vc-hg org-eldoc ob-python ob-shell org-bullets org-download toc-org org-eww org-rmail org-mhe org-irc org-info org-gnus nnir gnus-sum gnus-group gnus-undo gnus-start gnus-cloud nnimap nnmail mail-source utf7 netrc nnoo gnus-spec gnus-int gnus-range gnus-win gnus nnheader org-docview org-bibtex bibtex org-bbdb org-w3m smartparens-org org-habit german-holidays org-agenda org-mu4e mu4e-conversation shr svg dom gnus-dired mu4e desktop frameset mu4e-speedbar mu4e-main mu4e-view mu4e-headers mu4e-compose mu4e-context mu4e-draft mu4e-actions rfc2368 smtpmail sendmail mu4e-mark mu4e-message flow-fill html2text mu4e-proc mu4e-utils doc-view jka-compr image-mode mu4e-lists mu4e-vars mu4e-meta orgit org-element avl-tree generator org ob ob-tangle ob-ref ob-lob ob-table ob-exp org-macro org-footnote org-src ob-comint ob-keys org-pcomplete org-list org-faces org-entities org-version ob-emacs-lisp ob-core ob-eval org-compat org-macs org-loaddefs cal-menu calendar cal-loaddefs swiper ivy delsel colir ivy-overlay tabify debug macrostep semantic/find helm-semantic helm-imenu semantic/util-modes semantic/util semantic semantic/tag semantic/lex semantic/fw mode-local cedet mwim eieio-opt speedbar sb-image ezimage dframe face-remap gravatar url-cache ffap magit-gitflow treemacs-magit evil-magit git-rebase forge-list forge-commands forge-semi forge-bitbucket buck forge-gogs gogs forge-gitea gtea forge-gitlab glab forge-github ghub-graphql treepy graphql ghub let-alist forge-notify forge-revnote forge-pullreq forge-issue forge-topic forge-post forge-repo forge forge-core forge-db closql emacsql-sqlite emacsql emacsql-compiler magit-bookmark magit-submodule magit-obsolete magit-popup magit-blame magit-stash 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 which-func magit-diff smerge-mode magit-core magit-autorevert magit-margin magit-transient magit-process magit-mode transient git-commit magit-git magit-section magit-utils crm log-edit message rfc822 mml mml-sec epa gnus-util rmail rmail-loaddefs mailabbrev mail-utils gmm-utils mailheader pcvs-util add-log with-editor async-bytecomp flx dired dired-loaddefs helm-x-files helm-for-files helm-bookmark helm-adaptive helm-info helm-external helm-net browse-url xml framey helm-descbinds helm-mode helm-files helm-buffers helm-tags helm-locate helm-grep helm-regexp helm-utils helm-help helm-types helm-flx helm helm-source helm-multi-match helm-lib async cl-print evil-surround edebug lsp-treemacs recentf vc-bzr vc-src vc-sccs vc-svn vc-cvs vc-rcs vc vc-dispatcher company-lsp importmagic epc ctable concurrent deferred flycheck-rust lsp-ui-flycheck lsp-ui lsp-ui-doc smartparens-markdown markdown-mode lsp-ui-imenu lsp-ui-peek lsp-ui-sideline view lsp-clients dash-functional lsp lsp-mode tree-widget spinner network-stream starttls em-glob esh-util flymake-proc flymake hi-lock evil-matchit evil-matchit-sdk smartparens-python python tramp-sh tramp tramp-compat tramp-loaddefs trampver ucs-normalize parse-time vc-git diff-mode treemacs-evil treemacs treemacs-compatibility treemacs-mode treemacs-interface treemacs-extensions treemacs-persistence treemacs-mouse-interface treemacs-tag-follow-mode treemacs-filewatch-mode treemacs-tags imenu treemacs-follow-mode treemacs-rendering treemacs-async treemacs-faces treemacs-icons treemacs-workspaces treemacs-dom treemacs-visuals treemacs-fringe-indicator treemacs-impl treemacs-macros pfuture ace-window avy treemacs-customization bookmark pp evil-escape display-line-numbers git-gutter-fringe fringe-helper git-gutter company-flx company-files company-keywords company-etags company-gtags company-template company-dabbrev-code company-dabbrev company-yasnippet company-capf company-quickhelp company overseer pkg-info url-http tls gnutls url url-proxy url-privacy url-expand url-methods url-history mailcap url-auth url-cookie url-domsuf url-util url-gw nsm rmc puny epl compile auto-compile packed elisp-slime-nav etags xref rainbow-mode goto-addr bug-reference flycheck-pos-tip pos-tip flycheck-ledger flycheck json map find-func hideshow yasnippet-snippets yasnippet rainbow-delimiters elec-pair evil-cleverparens evil-cleverparens-text-objects evil-cleverparens-util paredit eros cap-words superword subword doom-modeline doom-modeline-segments doom-modeline-env doom-modeline-core project shrink-path eldoc-eval all-the-icons all-the-icons-faces data-material data-weathericons data-octicons data-fileicons data-faicons data-alltheicons memoize inline powerline powerline-separators color powerline-themes smartparens-config smartparens-text smartparens winum shackle trace eyebrowse evil-goggles pulse f s dash server winner xterm-color saveplace savehist noutline outline gh-common marshal hybrid-mode evil-evilified-state which-key use-package use-package-ensure use-package-delight use-package-diminish use-package-bind-key bind-key use-package-core hydra lv cus-edit cus-start cus-load evil evil-integration undo-tree diff evil-maps evil-commands reveal flyspell ispell evil-jumps evil-command-window evil-types evil-search evil-ex shell pcomplete comint ansi-color evil-macros evil-repeat evil-states evil-core evil-common windmove thingatpt rect evil-digraphs evil-vars ring bind-map quelpa mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045% ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-03-05 22:57 bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook Alexander Miller @ 2019-03-06 9:39 ` martin rudalics 2019-03-06 11:29 ` Alexander Miller ` (2 subsequent siblings) 3 siblings, 0 replies; 72+ messages in thread From: martin rudalics @ 2019-03-06 9:39 UTC (permalink / raw) To: Alexander Miller, 34765 > To quote from the documentation of select-window: > > Selections that "really count" are those > causing a visible change in the next redisplay of WINDOW’s frame and > should be always recorded. So if you think of running a function each > time a window gets selected put it on ‘buffer-list-update-hook’. > > A temporary buffer hardly fits these criteria. You're probably right but there's not much we can do in this regard. When you show a temporary buffer and temporarily show another buffer in the same window, there must be a way to show the temporary buffer again. That's why we have to run 'buffer-list-update-hook' for them. > The problem is not just > theoretical either. I have now run multiple times into situations where > use of a temp buffer caused a feedback loop that makes > buffer-list-update-hook fire permanently. For example a function called > by buffer-list-update-hook uses with-selected-window, this causes a > recalculation of the frame title, that calls format-spec, which uses a > temp-buffer and we are back to step one. Granted this case is very > specific to spacemacs and I am unable to reproduce it from emacs -q > (with-selected-window does not recalculate the frame title here), but > this is already the second time I've run into this 'with-selected-window' calls 'select-window' with NORECORD non-nil, so this should not be the culprit. I'm afraid you have to dig further to find out how 'buffer-list-update-hook' precisely gets called here. > (first time it was > magit). What was the issue there? > So yeah, with-temp-buffer correction aside, if you have any advice how > to avoid the issue on my end without going back to advising > select-window that'd be great. Emacs 27 now has 'window-selection-change-functions', strongly tied to redisplay and triggering only when the window selection has changed since last redisplay. Maybe you could try that. martin ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-03-05 22:57 bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook Alexander Miller 2019-03-06 9:39 ` martin rudalics @ 2019-03-06 11:29 ` Alexander Miller 2019-03-06 14:13 ` martin rudalics 2019-03-07 6:18 ` Alexander Miller 2020-12-20 17:57 ` Basil L. Contovounesios 3 siblings, 1 reply; 72+ messages in thread From: Alexander Miller @ 2019-03-06 11:29 UTC (permalink / raw) To: rudalics; +Cc: 34765, alexanderm > When you show a temporary buffer and temporarily show another > buffer in the same window, there must be a way to show the temporary > buffer again. I do no understand the use-case you have in mind here. with-temp-buffer serves to create a short-lived temporary buffer that is quickly disposed of again. When would such a buffer be shown anywhere? > I'm afraid you have to dig further to find out how > 'buffer-list-update-hook' precisely gets called here. Did that, turns out it's down to mode-line packages, both powerline and doom-modeline are advising select-window. I'll create PRs for both. > What was the issue there? Same feedback loop as described above, except it would only happen when a region was active in magit's status buffer. I had tracked the cause to a temp buffer deep inside magit's internals. Here's the github link: https://github.com/magit/magit/issues/3738#issuecomment-464520582 > Emacs 27 now has 'window-selection-change-functions', strongly > tied to redisplay and triggering only when the window selection has > changed since last redisplay. Maybe you could try that. This problem occurs in my treemacs package, so I cannot use a solution provided by a bleeding edge release, my modus operandi is to support the last 2 versions of emacs, so 25 and 26 (since stable distros like debian still use emacs 25). At any rate I have found the culprit in those modeline packages, so that point is solved. ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-03-06 11:29 ` Alexander Miller @ 2019-03-06 14:13 ` martin rudalics 2019-03-06 15:41 ` Eli Zaretskii 0 siblings, 1 reply; 72+ messages in thread From: martin rudalics @ 2019-03-06 14:13 UTC (permalink / raw) To: Alexander Miller; +Cc: 34765 > I do no understand the use-case you have in mind here. with-temp-buffer > serves to create a short-lived temporary buffer that is quickly disposed > of again. When would such a buffer be shown anywhere? I confused this 'with-output-to-temp-buffer'. Maybe we should indeed rebind 'buffer-list-update-hook' around the 'generate-new-buffer' and 'kill-buffer' calls of 'with-temp-buffer'. > > I'm afraid you have to dig further to find out how > > 'buffer-list-update-hook' precisely gets called here. > > Did that, turns out it's down to mode-line packages, both powerline and > doom-modeline are advising select-window. And do not preserve NORECORD? > I'll create PRs for both. > > > What was the issue there? > > Same feedback loop as described above, except it would only happen > when a region was active in magit's status buffer. I had tracked > the cause to a temp buffer deep inside magit's internals. Here's the > github link: > https://github.com/magit/magit/issues/3738#issuecomment-464520582 Maybe magit should simply try to reuse the same temporary buffer instead of recreating it excessively. Creating/killing temporary buffers does not come without some overhead. > > Emacs 27 now has 'window-selection-change-functions', strongly > > tied to redisplay and triggering only when the window selection has > > changed since last redisplay. Maybe you could try that. > > This problem occurs in my treemacs package, so I cannot use a solution > provided by a bleeding edge release, my modus operandi is to support the > last 2 versions of emacs, so 25 and 26 (since stable distros like debian > still use emacs 25). At any rate I have found the culprit in those modeline > packages, so that point is solved. I see. But please try 'window-selection-change-functions' sooner or later so we know whether it fixes these problems. martin ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-03-06 14:13 ` martin rudalics @ 2019-03-06 15:41 ` Eli Zaretskii 2019-03-06 17:57 ` martin rudalics 0 siblings, 1 reply; 72+ messages in thread From: Eli Zaretskii @ 2019-03-06 15:41 UTC (permalink / raw) To: martin rudalics; +Cc: 34765, alexanderm > Date: Wed, 06 Mar 2019 15:13:37 +0100 > From: martin rudalics <rudalics@gmx.at> > Cc: 34765@debbugs.gnu.org > > Maybe we should indeed rebind 'buffer-list-update-hook' around the > 'generate-new-buffer' and 'kill-buffer' calls of 'with-temp-buffer'. Wouldn't that be a backward-incompatible change? How long did we call that hook for temporary buffers? Also, can generate-new-buffer and/or kill-buffer run some hooks which might modify other, non-temporary buffers? ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-03-06 15:41 ` Eli Zaretskii @ 2019-03-06 17:57 ` martin rudalics 2019-04-23 9:21 ` martin rudalics 0 siblings, 1 reply; 72+ messages in thread From: martin rudalics @ 2019-03-06 17:57 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 34765, alexanderm >> Maybe we should indeed rebind 'buffer-list-update-hook' around the >> 'generate-new-buffer' and 'kill-buffer' calls of 'with-temp-buffer'. > > Wouldn't that be a backward-incompatible change? It would be a backward-incompatible change. > How long did we call > that hook for temporary buffers? Ever since that hook existed. > Also, can generate-new-buffer and/or > kill-buffer run some hooks which might modify other, non-temporary > buffers? 'generate-new-buffer' calls 'get-buffer-create' and that one runs only 'buffer-list-update-hook'. 'kill-buffer' runs its usual hooks and if one of them runs 'kill-buffer' for another buffer we'd have a problem. An even more radical solution would be to never run 'buffer-list-update-hook' for buffers whose name starts with a space. Backward-incompatible as well but cleaner from a designer's POV. In either case it wouldn't help the OP since he probably (hopefully) won't need a solution for Emacs 27 (where he should be able to use 'window-selection-change-functions' instead) and we are certainly not going to change this for Emacs 26. martin ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-03-06 17:57 ` martin rudalics @ 2019-04-23 9:21 ` martin rudalics 2019-04-23 10:36 ` Eli Zaretskii 2019-04-25 13:01 ` Stefan Monnier 0 siblings, 2 replies; 72+ messages in thread From: martin rudalics @ 2019-04-23 9:21 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 34765, alexanderm, Stefan Monnier [-- Attachment #1: Type: text/plain, Size: 1296 bytes --] > >> Maybe we should indeed rebind 'buffer-list-update-hook' around the > >> 'generate-new-buffer' and 'kill-buffer' calls of 'with-temp-buffer'. > > > > Wouldn't that be a backward-incompatible change? > > It would be a backward-incompatible change. > > > How long did we call > > that hook for temporary buffers? > > Ever since that hook existed. > > > Also, can generate-new-buffer and/or > > kill-buffer run some hooks which might modify other, non-temporary > > buffers? > > 'generate-new-buffer' calls 'get-buffer-create' and that one runs only > 'buffer-list-update-hook'. 'kill-buffer' runs its usual hooks and if > one of them runs 'kill-buffer' for another buffer we'd have a problem. > > An even more radical solution would be to never run > 'buffer-list-update-hook' for buffers whose name starts with a space. > Backward-incompatible as well but cleaner from a designer's POV. > > In either case it wouldn't help the OP since he probably (hopefully) > won't need a solution for Emacs 27 (where he should be able to use > 'window-selection-change-functions' instead) and we are certainly not > going to change this for Emacs 26. Stefan asked me to add a variable 'inhibit-buffer-list-update-hook' and I came up with the attached. WDYT? martin [-- Attachment #2: inhibit-buffer-list-update-hook.diff --] [-- Type: text/plain, Size: 5654 bytes --] diff --git a/lisp/subr.el b/lisp/subr.el index f68f9dd419..388705b3de 100644 --- a/lisp/subr.el +++ b/lisp/subr.el @@ -3569,13 +3569,16 @@ with-temp-buffer See also `with-temp-file' and `with-output-to-string'." (declare (indent 0) (debug t)) (let ((temp-buffer (make-symbol "temp-buffer"))) - `(let ((,temp-buffer (generate-new-buffer " *temp*"))) + `(let ((,temp-buffer + (let ((inhibit-buffer-list-update-hook t)) + (generate-new-buffer " *temp*")))) ;; FIXME: kill-buffer can change current-buffer in some odd cases. (with-current-buffer ,temp-buffer (unwind-protect (progn ,@body) (and (buffer-name ,temp-buffer) - (kill-buffer ,temp-buffer))))))) + (let ((inhibit-buffer-list-update-hook t)) + (kill-buffer ,temp-buffer)))))))) (defmacro with-silent-modifications (&rest body) "Execute BODY, pretending it does not modify the buffer. diff --git a/src/buffer.c b/src/buffer.c index ab47748191..822b5262dc 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -500,6 +500,36 @@ get_truename_buffer (register Lisp_Object filename) return Qnil; } + +/** + * run_buffer_list_update_hook: + * + * Run any functions on 'buffer-list-update-hook'. Do not run the + * functions when BUFFER is a buffer and its inhibit_buffer_hooks + * structure element is set. Do not run any functions either when we + * are not allowed to run hooks or 'inhibit-buffer-list-update-hook' + * is non-nil. + * + * While running the functions, bind 'inhibit-buffer-list-update-hook' + * to t to avoid invoking ourselves recursively. + */ +static void +run_buffer_list_update_hook (Lisp_Object buffer) +{ + if (!NILP (Vrun_hooks) + && (!BUFFERP (buffer) || !XBUFFER (buffer)->inhibit_buffer_hooks) + && !inhibit_buffer_list_update_hook) + { + ptrdiff_t count = SPECPDL_INDEX (); + + record_unwind_current_buffer (); + specbind (Qinhibit_buffer_list_update_hook, Qt); + call1 (Vrun_hooks, Qbuffer_list_update_hook); + unbind_to (count, Qnil); + } +} + + DEFUN ("get-buffer-create", Fget_buffer_create, Sget_buffer_create, 1, 1, 0, doc: /* Return the buffer specified by BUFFER-OR-NAME, creating a new one if needed. If BUFFER-OR-NAME is a string and a live buffer with that name exists, @@ -600,9 +630,8 @@ even if it is dead. The return value is never nil. */) /* Put this in the alist of all live buffers. */ XSETBUFFER (buffer, b); Vbuffer_alist = nconc2 (Vbuffer_alist, list1 (Fcons (name, buffer))); - /* And run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !b->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + /* Run buffer-list-update-hook. */ + run_buffer_list_update_hook (buffer); return buffer; } @@ -871,8 +900,7 @@ CLONE nil means the indirect buffer's state is reset to default values. */) } /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks)) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (Qnil); return buf; } @@ -1499,8 +1527,7 @@ This does not change the name of the visited file (if any). */) call0 (intern ("rename-auto-save-file")); /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !current_buffer->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (buf); /* Refetch since that last call may have done GC. */ return BVAR (current_buffer, name); @@ -1938,8 +1965,7 @@ cleaning up all windows currently displaying the buffer to be killed. */) bset_undo_list (b, Qnil); /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !b->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (buffer); return Qt; } @@ -1980,8 +2006,7 @@ record_buffer (Lisp_Object buffer) fset_buried_buffer_list (f, Fdelq (buffer, f->buried_buffer_list)); /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !XBUFFER (buffer)->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (buffer); } @@ -2019,8 +2044,7 @@ DEFUN ("bury-buffer-internal", Fbury_buffer_internal, Sbury_buffer_internal, (f, Fcons (buffer, Fdelq (buffer, f->buried_buffer_list))); /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !XBUFFER (buffer)->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (buffer); return Qnil; } @@ -6268,12 +6292,16 @@ The function `kill-all-local-variables' runs this before doing anything else. * doc: /* Hook run when the buffer list changes. Functions (implicitly) running this hook are `get-buffer-create', `make-indirect-buffer', `rename-buffer', `kill-buffer', `bury-buffer' -and `select-window'. Functions run by this hook should avoid calling -`select-window' with a nil NORECORD argument or `with-temp-buffer' -since either may lead to infinite recursion. */); +and `select-window'. This hook is not run when +`inhibit-buffer-list-update-hook' is non-nil. */); Vbuffer_list_update_hook = Qnil; DEFSYM (Qbuffer_list_update_hook, "buffer-list-update-hook"); + DEFVAR_BOOL ("inhibit-buffer-list-update-hook", inhibit_buffer_list_update_hook, + doc: /* Non-nil means don't run `buffer-list-update-hook'. */); + inhibit_buffer_list_update_hook = false; + DEFSYM (Qinhibit_buffer_list_update_hook, "inhibit-buffer-list-update-hook"); + defsubr (&Sbuffer_live_p); defsubr (&Sbuffer_list); defsubr (&Sget_buffer); ^ permalink raw reply related [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-04-23 9:21 ` martin rudalics @ 2019-04-23 10:36 ` Eli Zaretskii 2019-04-24 7:27 ` martin rudalics 2019-04-25 13:01 ` Stefan Monnier 1 sibling, 1 reply; 72+ messages in thread From: Eli Zaretskii @ 2019-04-23 10:36 UTC (permalink / raw) To: martin rudalics; +Cc: 34765, alexanderm, monnier > From: martin rudalics <rudalics@gmx.at> > Cc: 34765@debbugs.gnu.org, alexanderm@web.de, > Stefan Monnier <monnier@IRO.UMontreal.CA> > Date: Tue, 23 Apr 2019 11:21:45 +0200 > > Stefan asked me to add a variable 'inhibit-buffer-list-update-hook' > and I came up with the attached. WDYT? Did he also ask you to remove the inhibit_buffer_hooks flag of the buffer object? I'd rather prefer that you set that flag for temporary buffers. In any case, removing the flag will get back the problem with hidden buffers used by coding.c, right? I don't want to lose that. More generally, I don't understand the need for this variable. If we just want to inhibit the hooks for temporary buffers, we don't need to provide a variable, we can do that internally and unconditionally. The variable assumes there are other legitimate use cases where a Lisp program would want to inhibit the hooks, and that we agree to let Lisp programs do that. What are those use cases? ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-04-23 10:36 ` Eli Zaretskii @ 2019-04-24 7:27 ` martin rudalics 2019-04-24 11:12 ` Eli Zaretskii 0 siblings, 1 reply; 72+ messages in thread From: martin rudalics @ 2019-04-24 7:27 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 34765, alexanderm, monnier >> Stefan asked me to add a variable 'inhibit-buffer-list-update-hook' >> and I came up with the attached. WDYT? > > Did he also ask you to remove the inhibit_buffer_hooks flag of the > buffer object? I'd rather prefer that you set that flag for temporary > buffers. In any case, removing the flag will get back the problem > with hidden buffers used by coding.c, right? I don't want to lose > that. FWIW handling of the the inhibit_buffer_hooks flag is unaffected by my patch. Note that I special cased the call from 'make-indirect-buffer' which is the only one that doesn't check that flag. > More generally, I don't understand the need for this variable. If we > just want to inhibit the hooks for temporary buffers, we don't need to > provide a variable, we can do that internally and unconditionally. > The variable assumes there are other legitimate use cases where a Lisp > program would want to inhibit the hooks, and that we agree to let Lisp > programs do that. What are those use cases? I don't know. The one we care about in the case at hand is that of 'with-temp-buffer' which seems innocuous enough for being used in the body of a function run by 'buffer-list-update-hook'. Since that macro is in subr.el and as such cannnot easily set the inhibit_buffer_hooks flag, the 'inhibit-buffer-list-update-hook' variable provides a simple workaround. But maybe Stefan himself can explain the rationale for such a variable. IIUC his main motivation was that by rebinding 'inhibit-buffer-list-update-hook' to nil one can from Lisp relax the restrictions provided by my patch. martin ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-04-24 7:27 ` martin rudalics @ 2019-04-24 11:12 ` Eli Zaretskii 2019-04-24 12:55 ` Stefan Monnier 2019-04-25 8:06 ` martin rudalics 0 siblings, 2 replies; 72+ messages in thread From: Eli Zaretskii @ 2019-04-24 11:12 UTC (permalink / raw) To: martin rudalics; +Cc: 34765, alexanderm, monnier > Cc: 34765@debbugs.gnu.org, alexanderm@web.de, monnier@IRO.UMontreal.CA > From: martin rudalics <rudalics@gmx.at> > Date: Wed, 24 Apr 2019 09:27:59 +0200 > > >> Stefan asked me to add a variable 'inhibit-buffer-list-update-hook' > >> and I came up with the attached. WDYT? > > > > Did he also ask you to remove the inhibit_buffer_hooks flag of the > > buffer object? I'd rather prefer that you set that flag for temporary > > buffers. In any case, removing the flag will get back the problem > > with hidden buffers used by coding.c, right? I don't want to lose > > that. > > FWIW handling of the the inhibit_buffer_hooks flag is unaffected by my > patch. You are right, sorry. > > More generally, I don't understand the need for this variable. If we > > just want to inhibit the hooks for temporary buffers, we don't need to > > provide a variable, we can do that internally and unconditionally. > > The variable assumes there are other legitimate use cases where a Lisp > > program would want to inhibit the hooks, and that we agree to let Lisp > > programs do that. What are those use cases? > > I don't know. The one we care about in the case at hand is that of > 'with-temp-buffer' which seems innocuous enough for being used in the > body of a function run by 'buffer-list-update-hook'. Since that macro > is in subr.el and as such cannnot easily set the inhibit_buffer_hooks > flag, the 'inhibit-buffer-list-update-hook' variable provides a simple > workaround. I meant to suggest that get-buffer-create sets this flag when the name of the buffer fits the template of temporary buffers. Exactly like it does for code-conversion buffers now. ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-04-24 11:12 ` Eli Zaretskii @ 2019-04-24 12:55 ` Stefan Monnier 2019-04-25 8:06 ` martin rudalics 1 sibling, 0 replies; 72+ messages in thread From: Stefan Monnier @ 2019-04-24 12:55 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 34765, alexanderm > I meant to suggest that get-buffer-create sets this flag when the name > of the buffer fits the template of temporary buffers. That's a more drastic change, but it might be worth a try. Stefan ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-04-24 11:12 ` Eli Zaretskii 2019-04-24 12:55 ` Stefan Monnier @ 2019-04-25 8:06 ` martin rudalics 2019-04-25 8:50 ` Eli Zaretskii 1 sibling, 1 reply; 72+ messages in thread From: martin rudalics @ 2019-04-25 8:06 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 34765, alexanderm, monnier > I meant to suggest that get-buffer-create sets this flag when the name > of the buffer fits the template of temporary buffers. Exactly like it > does for code-conversion buffers now. We don't have a "template of temporary buffers". For example, a buffer specified by the BUFNAME arg of 'with-output-to-temp-buffer' should not match such a template. But I'd favor a solution that skips all buffers whose name starts with a space as we do for 'other-buffer' or 'unbury-buffer'. We'd just have to document it, maybe provide an additional argument for 'buffer-list' to not return such buffers, decide what to do when we rename them, when they visit a file, get killed ... martin ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-04-25 8:06 ` martin rudalics @ 2019-04-25 8:50 ` Eli Zaretskii 2019-04-25 10:31 ` martin rudalics 0 siblings, 1 reply; 72+ messages in thread From: Eli Zaretskii @ 2019-04-25 8:50 UTC (permalink / raw) To: martin rudalics; +Cc: 34765, alexanderm, monnier > Cc: 34765@debbugs.gnu.org, alexanderm@web.de, monnier@IRO.UMontreal.CA > From: martin rudalics <rudalics@gmx.at> > Date: Thu, 25 Apr 2019 10:06:18 +0200 > > > I meant to suggest that get-buffer-create sets this flag when the name > > of the buffer fits the template of temporary buffers. Exactly like it > > does for code-conversion buffers now. > > We don't have a "template of temporary buffers". Of course we do: with-temp-buffer produces buffer names that follow such a template. > For example, a buffer specified by the BUFNAME arg of > 'with-output-to-temp-buffer' should not match such a template. Such buffers should not necessarily be exempt from running the hooks, AFAIU. In any case, one can always bind the hook locally to nil in the body of the macro, right? > But I'd favor a solution that skips all buffers whose name starts > with a space as we do for 'other-buffer' or 'unbury-buffer'. I think this is too drastic a measure. We should only disable the hooks in buffers where no one in their right minds will ever want to run them. ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-04-25 8:50 ` Eli Zaretskii @ 2019-04-25 10:31 ` martin rudalics 2019-04-25 10:49 ` Eli Zaretskii 0 siblings, 1 reply; 72+ messages in thread From: martin rudalics @ 2019-04-25 10:31 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 34765, alexanderm, monnier >> > I meant to suggest that get-buffer-create sets this flag when the name >> > of the buffer fits the template of temporary buffers. Exactly like it >> > does for code-conversion buffers now. >> >> We don't have a "template of temporary buffers". > > Of course we do: with-temp-buffer produces buffer names that follow > such a template. IIUC we'd need an expression similar to Vcode_conversion_workbuf_name to use in 'get-buffer-create' b->inhibit_buffer_hooks = (STRINGP (Vcode_conversion_workbuf_name) && strncmp (SSDATA (name), SSDATA (Vcode_conversion_workbuf_name), SBYTES (Vcode_conversion_workbuf_name)) == 0); So we could specify, for example, staticpro (&Vtemp_buffer_name); Vtemp_buffer_name = build_pure_c_string (" *temp*"); and rewrite the above assignment as b->inhibit_buffer_hooks = ((STRINGP (Vcode_conversion_workbuf_name) && strncmp (SSDATA (name), SSDATA (Vcode_conversion_workbuf_name), SBYTES (Vcode_conversion_workbuf_name)) == 0) || (STRINGP (Vtemp_buffer_name) && strncmp (SSDATA (name), SSDATA (Vtemp_buffer_name), SBYTES (Vtemp_buffer_name)) == 0)); >> For example, a buffer specified by the BUFNAME arg of >> 'with-output-to-temp-buffer' should not match such a template. > > Such buffers should not necessarily be exempt from running the hooks, > AFAIU. In any case, one can always bind the hook locally to nil in > the body of the macro, right? Unless the body changes the buffer list in some signifcant way, for example, by creating or deleting another buffer. >> But I'd favor a solution that skips all buffers whose name starts >> with a space as we do for 'other-buffer' or 'unbury-buffer'. > > I think this is too drastic a measure. We should only disable the > hooks in buffers where no one in their right minds will ever want to > run them. Then what about the buffers created by 'with-temp-file' or 'with-output-to-string'? martin ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-04-25 10:31 ` martin rudalics @ 2019-04-25 10:49 ` Eli Zaretskii 2019-04-26 7:40 ` martin rudalics 0 siblings, 1 reply; 72+ messages in thread From: Eli Zaretskii @ 2019-04-25 10:49 UTC (permalink / raw) To: martin rudalics; +Cc: 34765, alexanderm, monnier > Cc: 34765@debbugs.gnu.org, alexanderm@web.de, monnier@IRO.UMontreal.CA > From: martin rudalics <rudalics@gmx.at> > Date: Thu, 25 Apr 2019 12:31:20 +0200 > > >> > I meant to suggest that get-buffer-create sets this flag when the name > >> > of the buffer fits the template of temporary buffers. Exactly like it > >> > does for code-conversion buffers now. > >> > >> We don't have a "template of temporary buffers". > > > > Of course we do: with-temp-buffer produces buffer names that follow > > such a template. > > IIUC we'd need an expression similar to Vcode_conversion_workbuf_name > to use in 'get-buffer-create' That's what I meant, yes. > >> For example, a buffer specified by the BUFNAME arg of > >> 'with-output-to-temp-buffer' should not match such a template. > > > > Such buffers should not necessarily be exempt from running the hooks, > > AFAIU. In any case, one can always bind the hook locally to nil in > > the body of the macro, right? > > Unless the body changes the buffer list in some signifcant way, for > example, by creating or deleting another buffer. Sure, but that's why this should not be done unconditionally. > Then what about the buffers created by 'with-temp-file' or > 'with-output-to-string'? Fine with me. But the former already uses with-temp-buffer, so I don't think we need anything special for it. ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-04-25 10:49 ` Eli Zaretskii @ 2019-04-26 7:40 ` martin rudalics 2019-04-26 8:09 ` Eli Zaretskii 0 siblings, 1 reply; 72+ messages in thread From: martin rudalics @ 2019-04-26 7:40 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 34765, alexanderm, monnier >> Then what about the buffers created by 'with-temp-file' or >> 'with-output-to-string'? > > Fine with me. But the former already uses with-temp-buffer, so I > don't think we need anything special for it. Here 'with-temp-file' uses (get-buffer-create (generate-new-buffer-name " *temp file*")) and 'with-output-to-string' (get-buffer-create (generate-new-buffer-name " *string-output*")) martin ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-04-26 7:40 ` martin rudalics @ 2019-04-26 8:09 ` Eli Zaretskii 0 siblings, 0 replies; 72+ messages in thread From: Eli Zaretskii @ 2019-04-26 8:09 UTC (permalink / raw) To: martin rudalics; +Cc: 34765, alexanderm, monnier > Cc: 34765@debbugs.gnu.org, alexanderm@web.de, monnier@IRO.UMontreal.CA > From: martin rudalics <rudalics@gmx.at> > Date: Fri, 26 Apr 2019 09:40:48 +0200 > > >> Then what about the buffers created by 'with-temp-file' or > >> 'with-output-to-string'? > > > > Fine with me. But the former already uses with-temp-buffer, so I > > don't think we need anything special for it. > > Here 'with-temp-file' uses > > (get-buffer-create (generate-new-buffer-name " *temp file*")) > > and 'with-output-to-string' > > (get-buffer-create (generate-new-buffer-name " *string-output*")) Oops. ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-04-23 9:21 ` martin rudalics 2019-04-23 10:36 ` Eli Zaretskii @ 2019-04-25 13:01 ` Stefan Monnier 2019-04-25 14:34 ` Eli Zaretskii 2019-04-26 7:41 ` martin rudalics 1 sibling, 2 replies; 72+ messages in thread From: Stefan Monnier @ 2019-04-25 13:01 UTC (permalink / raw) To: martin rudalics; +Cc: 34765, alexanderm > - `(let ((,temp-buffer (generate-new-buffer " *temp*"))) > + `(let ((,temp-buffer > + (let ((inhibit-buffer-list-update-hook t)) > + (generate-new-buffer " *temp*")))) > ;; FIXME: kill-buffer can change current-buffer in some odd cases. > (with-current-buffer ,temp-buffer > (unwind-protect > (progn ,@body) > (and (buffer-name ,temp-buffer) > - (kill-buffer ,temp-buffer))))))) > + (let ((inhibit-buffer-list-update-hook t)) > + (kill-buffer ,temp-buffer)))))))) Hmm... I was thinking we could expose `inhibit_buffer_hooks` as a Lisp variable (so we can set it without having to match names, which I find ugly and brittle), but we'd want to `setq` it rather than let-bind it. But while it's easy to set it before running kill-buffer, we can't set it before calling generate-new-buffer :-( How 'bout adding an optional argument to `generate-new-buffer` to set `inhibit_buffer_hooks`? Stefan ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-04-25 13:01 ` Stefan Monnier @ 2019-04-25 14:34 ` Eli Zaretskii 2019-04-26 7:41 ` martin rudalics 2019-04-26 7:41 ` martin rudalics 1 sibling, 1 reply; 72+ messages in thread From: Eli Zaretskii @ 2019-04-25 14:34 UTC (permalink / raw) To: Stefan Monnier; +Cc: 34765, alexanderm > From: Stefan Monnier <monnier@IRO.UMontreal.CA> > Cc: Eli Zaretskii <eliz@gnu.org>, 34765@debbugs.gnu.org, alexanderm@web.de > Date: Thu, 25 Apr 2019 09:01:01 -0400 > > How 'bout adding an optional argument to `generate-new-buffer` to set > `inhibit_buffer_hooks`? Fine with me, but aren't you afraid people will start abusing this? ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-04-25 14:34 ` Eli Zaretskii @ 2019-04-26 7:41 ` martin rudalics 2019-04-26 8:10 ` Eli Zaretskii 0 siblings, 1 reply; 72+ messages in thread From: martin rudalics @ 2019-04-26 7:41 UTC (permalink / raw) To: Eli Zaretskii, Stefan Monnier; +Cc: 34765, alexanderm >> How 'bout adding an optional argument to `generate-new-buffer` to set >> `inhibit_buffer_hooks`? > > Fine with me, but aren't you afraid people will start abusing this? I wouldn't pass an argument but rather set the flag unconditionally when NAME starts with a space ... martin ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-04-26 7:41 ` martin rudalics @ 2019-04-26 8:10 ` Eli Zaretskii 0 siblings, 0 replies; 72+ messages in thread From: Eli Zaretskii @ 2019-04-26 8:10 UTC (permalink / raw) To: martin rudalics; +Cc: 34765, alexanderm, monnier > Cc: 34765@debbugs.gnu.org, alexanderm@web.de > From: martin rudalics <rudalics@gmx.at> > Date: Fri, 26 Apr 2019 09:41:11 +0200 > > >> How 'bout adding an optional argument to `generate-new-buffer` to set > >> `inhibit_buffer_hooks`? > > > > Fine with me, but aren't you afraid people will start abusing this? > > I wouldn't pass an argument but rather set the flag unconditionally > when NAME starts with a space ... Too radical, IMO. ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-04-25 13:01 ` Stefan Monnier 2019-04-25 14:34 ` Eli Zaretskii @ 2019-04-26 7:41 ` martin rudalics 2019-04-26 8:10 ` Eli Zaretskii 1 sibling, 1 reply; 72+ messages in thread From: martin rudalics @ 2019-04-26 7:41 UTC (permalink / raw) To: Stefan Monnier; +Cc: 34765, alexanderm > How 'bout adding an optional argument to `generate-new-buffer` to set > `inhibit_buffer_hooks`? How about moving 'generate-new-buffer' to C to set that flag without exposing it to Lisp (and to avoid things like ;; We can't use `generate-new-buffer' because files.el ;; is not yet loaded. in 'load-with-code-conversion')? martin ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-04-26 7:41 ` martin rudalics @ 2019-04-26 8:10 ` Eli Zaretskii 2019-04-26 11:00 ` martin rudalics 0 siblings, 1 reply; 72+ messages in thread From: Eli Zaretskii @ 2019-04-26 8:10 UTC (permalink / raw) To: martin rudalics; +Cc: 34765, alexanderm, monnier > Cc: Eli Zaretskii <eliz@gnu.org>, 34765@debbugs.gnu.org, alexanderm@web.de > From: martin rudalics <rudalics@gmx.at> > Date: Fri, 26 Apr 2019 09:41:02 +0200 > > > How 'bout adding an optional argument to `generate-new-buffer` to set > > `inhibit_buffer_hooks`? > > How about moving 'generate-new-buffer' to C to set that flag without > exposing it to Lisp (and to avoid things like > > ;; We can't use `generate-new-buffer' because files.el > ;; is not yet loaded. > > in 'load-with-code-conversion')? Fine with me. ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-04-26 8:10 ` Eli Zaretskii @ 2019-04-26 11:00 ` martin rudalics 2019-04-26 11:26 ` Eli Zaretskii ` (2 more replies) 0 siblings, 3 replies; 72+ messages in thread From: martin rudalics @ 2019-04-26 11:00 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 34765, alexanderm, monnier [-- Attachment #1: Type: text/plain, Size: 462 bytes --] >> > How 'bout adding an optional argument to `generate-new-buffer` to set >> > `inhibit_buffer_hooks`? >> >> How about moving 'generate-new-buffer' to C to set that flag without >> exposing it to Lisp (and to avoid things like >> >> ;; We can't use `generate-new-buffer' because files.el >> ;; is not yet loaded. >> >> in 'load-with-code-conversion')? > > Fine with me. I attach a preliminary patch. martin [-- Attachment #2: inhibit-buffer-hooks.diff --] [-- Type: text/plain, Size: 11521 bytes --] diff --git a/lisp/files.el b/lisp/files.el index c05d70a00e..d919793251 100644 --- a/lisp/files.el +++ b/lisp/files.el @@ -1777,6 +1777,9 @@ kill-buffer-hook The buffer being killed is current while the hook is running. See `kill-buffer'. +This hook is not run for buffers created by 'generate-new-buffer' +with the second argument 'inhibit-buffer-hooks' non-nil. + Note: Be careful with let-binding this hook considering it is frequently used for cleanup.") @@ -1880,11 +1883,6 @@ create-file-buffer (concat "|" lastname) lastname)))) -(defun generate-new-buffer (name) - "Create and return a buffer with a name based on NAME. -Choose the buffer's name using `generate-new-buffer-name'." - (get-buffer-create (generate-new-buffer-name name))) - (defcustom automount-dir-prefix (purecopy "^/tmp_mnt/") "Regexp to match the automounter prefix in a directory name." :group 'files diff --git a/lisp/international/mule.el b/lisp/international/mule.el index ba30fee496..d168ee9a16 100644 --- a/lisp/international/mule.el +++ b/lisp/international/mule.el @@ -306,10 +306,7 @@ load-with-code-conversion (and (null noerror) (signal 'file-error (list "Cannot open load file" file))) ;; Read file with code conversion, and then eval. - (let* ((buffer - ;; We can't use `generate-new-buffer' because files.el - ;; is not yet loaded. - (get-buffer-create (generate-new-buffer-name " *load*"))) + (let* ((buffer (generate-new-buffer " *load*")) (load-in-progress t) (source (save-match-data (string-match "\\.el\\'" fullname)))) (unless nomessage diff --git a/lisp/subr.el b/lisp/subr.el index f68f9dd419..5f2c43c78b 100644 --- a/lisp/subr.el +++ b/lisp/subr.el @@ -3530,8 +3530,7 @@ with-temp-file (let ((temp-file (make-symbol "temp-file")) (temp-buffer (make-symbol "temp-buffer"))) `(let ((,temp-file ,file) - (,temp-buffer - (get-buffer-create (generate-new-buffer-name " *temp file*")))) + (,temp-buffer (generate-new-buffer " *temp file*" t))) (unwind-protect (prog1 (with-current-buffer ,temp-buffer @@ -3569,7 +3568,7 @@ with-temp-buffer See also `with-temp-file' and `with-output-to-string'." (declare (indent 0) (debug t)) (let ((temp-buffer (make-symbol "temp-buffer"))) - `(let ((,temp-buffer (generate-new-buffer " *temp*"))) + `(let ((,temp-buffer (generate-new-buffer " *temp*" t))) ;; FIXME: kill-buffer can change current-buffer in some odd cases. (with-current-buffer ,temp-buffer (unwind-protect @@ -3604,8 +3603,7 @@ with-silent-modifications (defmacro with-output-to-string (&rest body) "Execute BODY, return the text it sent to `standard-output', as a string." (declare (indent 0) (debug t)) - `(let ((standard-output - (get-buffer-create (generate-new-buffer-name " *string-output*")))) + `(let ((standard-output (generate-new-buffer " *string-output*" t))) (unwind-protect (progn (let ((standard-output standard-output)) diff --git a/src/buffer.c b/src/buffer.c index ab47748191..3e135148fb 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -500,19 +500,43 @@ get_truename_buffer (register Lisp_Object filename) return Qnil; } -DEFUN ("get-buffer-create", Fget_buffer_create, Sget_buffer_create, 1, 1, 0, - doc: /* Return the buffer specified by BUFFER-OR-NAME, creating a new one if needed. -If BUFFER-OR-NAME is a string and a live buffer with that name exists, -return that buffer. If no such buffer exists, create a new buffer with -that name and return it. If BUFFER-OR-NAME starts with a space, the new -buffer does not keep undo information. -If BUFFER-OR-NAME is a buffer instead of a string, return it as given, -even if it is dead. The return value is never nil. */) - (register Lisp_Object buffer_or_name) +/** + * run_buffer_list_update_hook: + * + * Run any functions on 'buffer-list-update-hook'. Do not run the + * functions when BUFFER is a buffer and its inhibit_buffer_hooks + * structure element is set. Do not run any functions either when we + * are not allowed to run hooks. + */ +static void +run_buffer_list_update_hook (Lisp_Object buffer) { - register Lisp_Object buffer, name; - register struct buffer *b; + if (!NILP (Vrun_hooks) + && (!BUFFERP (buffer) || !XBUFFER (buffer)->inhibit_buffer_hooks)) + { + ptrdiff_t count = SPECPDL_INDEX (); + + record_unwind_current_buffer (); + call1 (Vrun_hooks, Qbuffer_list_update_hook); + unbind_to (count, Qnil); + } +} + +/** + * get_buffer_create: + * + * Return buffer specified by BUFFER-OR-NAME, creating a new one if + * needed. See Fget_buffer_create below for more information. + * + * Second argument INHIBIT_BUFFER_HOOKS true means to not run + * 'buffer-list-update-hook'. + */ +static Lisp_Object +get_buffer_create (Lisp_Object buffer_or_name, bool inhibit_buffer_hooks) +{ + Lisp_Object buffer, name; + struct buffer *b; buffer = Fget_buffer (buffer_or_name); if (!NILP (buffer)) @@ -600,14 +624,29 @@ even if it is dead. The return value is never nil. */) /* Put this in the alist of all live buffers. */ XSETBUFFER (buffer, b); Vbuffer_alist = nconc2 (Vbuffer_alist, list1 (Fcons (name, buffer))); - /* And run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !b->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + if (!inhibit_buffer_hooks) + /* Run buffer-list-update-hook. */ + run_buffer_list_update_hook (buffer); return buffer; } +DEFUN ("get-buffer-create", Fget_buffer_create, Sget_buffer_create, 1, 1, 0, + doc: /* Return the buffer specified by BUFFER-OR-NAME, creating a new one if needed. +If BUFFER-OR-NAME is a string and a live buffer with that name exists, +return that buffer. If no such buffer exists, create a new buffer with +that name and return it. If BUFFER-OR-NAME starts with a space, the new +buffer does not keep undo information. + +If BUFFER-OR-NAME is a buffer instead of a string, return it as given, +even if it is dead. The return value is never nil. */) + (Lisp_Object buffer_or_name) +{ + return get_buffer_create (buffer_or_name, false); +} + + /* Return a list of overlays which is a copy of the overlay list LIST, but for buffer B. */ @@ -871,8 +910,7 @@ CLONE nil means the indirect buffer's state is reset to default values. */) } /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks)) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (Qnil); return buf; } @@ -1135,6 +1173,31 @@ is first appended to NAME, to speed up finding a non-existent buffer. */) } } +DEFUN ("generate-new-buffer", Fgenerate_new_buffer, Sgenerate_new_buffer, + 1, 2, 0, + doc: /* Create and return a buffer with a name based on NAME. +Choose the buffer's name using `generate-new-buffer-name'. + +Optional second argument INHIBIT-BUFFER-HOOKS non-nil means to not run +any buffer hooks ('kill-buffer-hook', 'buffer-list-update-hook' or +'kill-buffer-query-functions') for this buffer. This argument should +be set only for internal buffers that are never presented to users or +passed on to other applications. */) + (Lisp_Object name, Lisp_Object inhibit_buffer_hooks) +{ + Lisp_Object buffer_name = Fgenerate_new_buffer_name (name, Qnil); + Lisp_Object buffer = get_buffer_create (buffer_name, + !NILP (inhibit_buffer_hooks)); + + if (!NILP (inhibit_buffer_hooks)) + { + struct buffer *b = XBUFFER (buffer); + + b->inhibit_buffer_hooks = true; + } + + return buffer; +} \f DEFUN ("buffer-name", Fbuffer_name, Sbuffer_name, 0, 1, 0, doc: /* Return the name of BUFFER, as a string. @@ -1499,8 +1562,7 @@ This does not change the name of the visited file (if any). */) call0 (intern ("rename-auto-save-file")); /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !current_buffer->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (buf); /* Refetch since that last call may have done GC. */ return BVAR (current_buffer, name); @@ -1677,6 +1739,10 @@ buffer is actually killed. The buffer being killed will be current while the hook is running. Functions called by any of these hooks are supposed to not change the current buffer. +Neither 'kill-buffer-query-functions' nor 'kill-buffer-hook' are run +for buffers created by 'generate-new-buffer' with the second argument +'inhibit-buffer-hooks' non-nil. + Any processes that have this buffer as the `process-buffer' are killed with SIGHUP. This function calls `replace-buffer-in-windows' for cleaning up all windows currently displaying the buffer to be killed. */) @@ -1938,8 +2004,7 @@ cleaning up all windows currently displaying the buffer to be killed. */) bset_undo_list (b, Qnil); /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !b->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (buffer); return Qt; } @@ -1980,8 +2045,7 @@ record_buffer (Lisp_Object buffer) fset_buried_buffer_list (f, Fdelq (buffer, f->buried_buffer_list)); /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !XBUFFER (buffer)->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (buffer); } @@ -2019,8 +2083,7 @@ DEFUN ("bury-buffer-internal", Fbury_buffer_internal, Sbury_buffer_internal, (f, Fcons (buffer, Fdelq (buffer, f->buried_buffer_list))); /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !XBUFFER (buffer)->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (buffer); return Qnil; } @@ -6255,7 +6318,10 @@ Use Custom to set this variable and update the display. */); The buffer being killed will be current while the functions are running. If any of them returns nil, the buffer is not killed. Functions run by -this hook are supposed to not change the current buffer. */); +this hook are supposed to not change the current buffer. + +This hook is not run for buffers created by 'generate-new-buffer' with +the second argument 'inhibit-buffer-hooks' non-nil. */); Vkill_buffer_query_functions = Qnil; DEFVAR_LISP ("change-major-mode-hook", Vchange_major_mode_hook, @@ -6268,9 +6334,9 @@ The function `kill-all-local-variables' runs this before doing anything else. * doc: /* Hook run when the buffer list changes. Functions (implicitly) running this hook are `get-buffer-create', `make-indirect-buffer', `rename-buffer', `kill-buffer', `bury-buffer' -and `select-window'. Functions run by this hook should avoid calling -`select-window' with a nil NORECORD argument or `with-temp-buffer' -since either may lead to infinite recursion. */); +and `select-window'. This hook is not run for buffers created by +'generate-new-buffer' with the second argument 'inhibit-buffer-hooks' +non-nil. */); Vbuffer_list_update_hook = Qnil; DEFSYM (Qbuffer_list_update_hook, "buffer-list-update-hook"); @@ -6281,6 +6347,7 @@ since either may lead to infinite recursion. */); defsubr (&Sget_buffer_create); defsubr (&Smake_indirect_buffer); defsubr (&Sgenerate_new_buffer_name); + defsubr (&Sgenerate_new_buffer); defsubr (&Sbuffer_name); defsubr (&Sbuffer_file_name); defsubr (&Sbuffer_base_buffer); ^ permalink raw reply related [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-04-26 11:00 ` martin rudalics @ 2019-04-26 11:26 ` Eli Zaretskii 2019-04-27 8:30 ` martin rudalics 2019-04-26 17:14 ` Basil L. Contovounesios 2020-08-26 11:06 ` Lars Ingebrigtsen 2 siblings, 1 reply; 72+ messages in thread From: Eli Zaretskii @ 2019-04-26 11:26 UTC (permalink / raw) To: martin rudalics; +Cc: 34765, alexanderm, monnier > Cc: monnier@IRO.UMontreal.CA, 34765@debbugs.gnu.org, alexanderm@web.de > From: martin rudalics <rudalics@gmx.at> > Date: Fri, 26 Apr 2019 13:00:06 +0200 > > > I attach a preliminary patch. Thanks. This will need documentation changes when pushed. > +/** > + * run_buffer_list_update_hook: > + * > + * Run any functions on 'buffer-list-update-hook'. Do not run the > + * functions when BUFFER is a buffer and its inhibit_buffer_hooks > + * structure element is set. Do not run any functions either when we > + * are not allowed to run hooks. > + */ Can we please use our style in commentary? We don't use bock comments in Emacs, so I'd like not to proliferate them. > + if (!inhibit_buffer_hooks) > + /* Run buffer-list-update-hook. */ > + run_buffer_list_update_hook (buffer); It is somewhat strange that part of the reasons for not running hooks are tested inside run_buffer_list_update_hook, and others explicitly here. Any special reasons for this inconsistency? > +DEFUN ("get-buffer-create", Fget_buffer_create, Sget_buffer_create, 1, 1, 0, > + doc: /* Return the buffer specified by BUFFER-OR-NAME, creating a new one if needed. > +If BUFFER-OR-NAME is a string and a live buffer with that name exists, > +return that buffer. If no such buffer exists, create a new buffer with > +that name and return it. If BUFFER-OR-NAME starts with a space, the new > +buffer does not keep undo information. > + > +If BUFFER-OR-NAME is a buffer instead of a string, return it as given, > +even if it is dead. The return value is never nil. */) > + (Lisp_Object buffer_or_name) > +{ > + return get_buffer_create (buffer_or_name, false); > +} Should this function also acquire an additional optional argument? If not, why not? > +Optional second argument INHIBIT-BUFFER-HOOKS non-nil means to not run > +any buffer hooks ('kill-buffer-hook', 'buffer-list-update-hook' or > +'kill-buffer-query-functions') for this buffer. This argument should The hooks should be quoted `like this', right? We do want them to become hyperlinks. > +be set only for internal buffers that are never presented to users or > +passed on to other applications. */) > + (Lisp_Object name, Lisp_Object inhibit_buffer_hooks) > +{ > + Lisp_Object buffer_name = Fgenerate_new_buffer_name (name, Qnil); > + Lisp_Object buffer = get_buffer_create (buffer_name, > + !NILP (inhibit_buffer_hooks)); > + > + if (!NILP (inhibit_buffer_hooks)) > + { > + struct buffer *b = XBUFFER (buffer); > + > + b->inhibit_buffer_hooks = true; > + } Should this flag be set inside get_buffer_create? > +Neither 'kill-buffer-query-functions' nor 'kill-buffer-hook' are run > +for buffers created by 'generate-new-buffer' with the second argument > +'inhibit-buffer-hooks' non-nil. Quoting again. > @@ -6268,9 +6334,9 @@ The function `kill-all-local-variables' runs this before doing anything else. * > doc: /* Hook run when the buffer list changes. > Functions (implicitly) running this hook are `get-buffer-create', > `make-indirect-buffer', `rename-buffer', `kill-buffer', `bury-buffer' > -and `select-window'. Functions run by this hook should avoid calling > -`select-window' with a nil NORECORD argument or `with-temp-buffer' > -since either may lead to infinite recursion. */); > +and `select-window'. This hook is not run for buffers created by > +'generate-new-buffer' with the second argument 'inhibit-buffer-hooks' > +non-nil. */); And here. ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-04-26 11:26 ` Eli Zaretskii @ 2019-04-27 8:30 ` martin rudalics 0 siblings, 0 replies; 72+ messages in thread From: martin rudalics @ 2019-04-27 8:30 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 34765, alexanderm, monnier > Can we please use our style in commentary? We don't use bock comments > in Emacs, so I'd like not to proliferate them. Actually it's not a comment but a doc-string and if you customize 'font-lock-doc-face' you will notice. These were part of a project of mine to improve handling and readability of Emacs primitives' doc-strings (including font locking, Lisp syntax with completion and eldoc support, correct filling, exporting/importing from C to Lisp to texinfo and the like). But since I never managed to write some of these components, the project has stalled. I'll stop proliferating that style now. >> + if (!inhibit_buffer_hooks) >> + /* Run buffer-list-update-hook. */ >> + run_buffer_list_update_hook (buffer); > > It is somewhat strange that part of the reasons for not running hooks > are tested inside run_buffer_list_update_hook, and others explicitly > here. Any special reasons for this inconsistency? See below. >> +DEFUN ("get-buffer-create", Fget_buffer_create, Sget_buffer_create, 1, 1, 0, >> + doc: /* Return the buffer specified by BUFFER-OR-NAME, creating a new one if needed. >> +If BUFFER-OR-NAME is a string and a live buffer with that name exists, >> +return that buffer. If no such buffer exists, create a new buffer with >> +that name and return it. If BUFFER-OR-NAME starts with a space, the new >> +buffer does not keep undo information. >> + >> +If BUFFER-OR-NAME is a buffer instead of a string, return it as given, >> +even if it is dead. The return value is never nil. */) >> + (Lisp_Object buffer_or_name) >> +{ >> + return get_buffer_create (buffer_or_name, false); >> +} > > Should this function also acquire an additional optional argument? If > not, why not? You mean 'get-buffer-create'? I had that in a first version. But since you earlier expressed concerns about people abusing the extra argument of 'generate-new-buffer' I thought 'get-buffer-create' would be even more critical in this regard. In either case I'll do what you (or others) consider best. >> +Optional second argument INHIBIT-BUFFER-HOOKS non-nil means to not run >> +any buffer hooks ('kill-buffer-hook', 'buffer-list-update-hook' or >> +'kill-buffer-query-functions') for this buffer. This argument should > > The hooks should be quoted `like this', right? We do want them to > become hyperlinks. Right. >> +be set only for internal buffers that are never presented to users or >> +passed on to other applications. */) >> + (Lisp_Object name, Lisp_Object inhibit_buffer_hooks) >> +{ >> + Lisp_Object buffer_name = Fgenerate_new_buffer_name (name, Qnil); >> + Lisp_Object buffer = get_buffer_create (buffer_name, >> + !NILP (inhibit_buffer_hooks)); >> + >> + if (!NILP (inhibit_buffer_hooks)) >> + { >> + struct buffer *b = XBUFFER (buffer); >> + >> + b->inhibit_buffer_hooks = true; >> + } > > Should this flag be set inside get_buffer_create? If we do that, it would also resolve the "inconsistency" you bemoan above. So the answer is probably yes. > Quoting again. [...] > And here. Right. At least I show signs of consistency when adopting silly style. Thanks, martin ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-04-26 11:00 ` martin rudalics 2019-04-26 11:26 ` Eli Zaretskii @ 2019-04-26 17:14 ` Basil L. Contovounesios 2019-04-27 8:31 ` martin rudalics 2020-08-26 11:06 ` Lars Ingebrigtsen 2 siblings, 1 reply; 72+ messages in thread From: Basil L. Contovounesios @ 2019-04-26 17:14 UTC (permalink / raw) To: martin rudalics; +Cc: 34765, alexanderm, monnier [-- Attachment #1: Type: text/plain, Size: 135 bytes --] martin rudalics <rudalics@gmx.at> writes: > I attach a preliminary patch. Your patch allows the following additional change, right? [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: coding.diff --] [-- Type: text/x-diff, Size: 474 bytes --] diff --git a/src/coding.c b/src/coding.c index 2c6b2c4d05..3dd5f3b1c1 100644 --- a/src/coding.c +++ b/src/coding.c @@ -7820,9 +7820,7 @@ code_conversion_save (bool with_work_buf, bool multibyte) { if (reused_workbuf_in_use) { - Lisp_Object name - = Fgenerate_new_buffer_name (Vcode_conversion_workbuf_name, Qnil); - workbuf = Fget_buffer_create (name); + workbuf = Fgenerate_new_buffer (Vcode_conversion_workbuf_name, Qt); } else { [-- Attachment #3: Type: text/plain, Size: 20 bytes --] Thanks, -- Basil ^ permalink raw reply related [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-04-26 17:14 ` Basil L. Contovounesios @ 2019-04-27 8:31 ` martin rudalics 2019-05-20 13:42 ` Basil L. Contovounesios 0 siblings, 1 reply; 72+ messages in thread From: martin rudalics @ 2019-04-27 8:31 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: 34765, alexanderm, monnier > Your patch allows the following additional change, right? I suppose so. I'm not sure about the Fget_buffer_create in the else branch of that if, though. It would be nice to get rid of the entire b->inhibit_buffer_hooks = (STRINGP (Vcode_conversion_workbuf_name) && strncmp (SSDATA (name), SSDATA (Vcode_conversion_workbuf_name), SBYTES (Vcode_conversion_workbuf_name)) == 0); form in 'get-buffer-create'. Any ideas? Thanks, martin ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-04-27 8:31 ` martin rudalics @ 2019-05-20 13:42 ` Basil L. Contovounesios 2019-05-21 7:32 ` martin rudalics 0 siblings, 1 reply; 72+ messages in thread From: Basil L. Contovounesios @ 2019-05-20 13:42 UTC (permalink / raw) To: martin rudalics; +Cc: 34765, alexanderm, monnier martin rudalics <rudalics@gmx.at> writes: >> Your patch allows the following additional change, right? > > I suppose so. I'm not sure about the Fget_buffer_create in the else > branch of that if, though. WDYM? > It would be nice to get rid of the entire > > b->inhibit_buffer_hooks > = (STRINGP (Vcode_conversion_workbuf_name) > && strncmp (SSDATA (name), SSDATA (Vcode_conversion_workbuf_name), > SBYTES (Vcode_conversion_workbuf_name)) == 0); > > form in 'get-buffer-create'. Any ideas? Doesn't your patch in https://debbugs.gnu.org/34765#86 already eliminate the need for it? Thanks, -- Basil ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-05-20 13:42 ` Basil L. Contovounesios @ 2019-05-21 7:32 ` martin rudalics 2019-05-21 7:58 ` Basil L. Contovounesios 0 siblings, 1 reply; 72+ messages in thread From: martin rudalics @ 2019-05-21 7:32 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: 34765, alexanderm, monnier >> It would be nice to get rid of the entire >> >> b->inhibit_buffer_hooks >> = (STRINGP (Vcode_conversion_workbuf_name) >> && strncmp (SSDATA (name), SSDATA (Vcode_conversion_workbuf_name), >> SBYTES (Vcode_conversion_workbuf_name)) == 0); >> >> form in 'get-buffer-create'. Any ideas? > > Doesn't your patch in https://debbugs.gnu.org/34765#86 already eliminate > the need for it? FWIW no, it leaves that form in place. And 'get-buffer-create' should not have to worry about delving any deeper into the name than down to its first character to check for ' ' or '*'. But I'm not happy with my patch anyway. Something more elegant is needed. martin ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-05-21 7:32 ` martin rudalics @ 2019-05-21 7:58 ` Basil L. Contovounesios 2019-05-21 10:04 ` martin rudalics 2019-05-22 7:21 ` Eli Zaretskii 0 siblings, 2 replies; 72+ messages in thread From: Basil L. Contovounesios @ 2019-05-21 7:58 UTC (permalink / raw) To: martin rudalics; +Cc: 34765, alexanderm, monnier martin rudalics <rudalics@gmx.at> writes: >>> It would be nice to get rid of the entire >>> >>> b->inhibit_buffer_hooks >>> = (STRINGP (Vcode_conversion_workbuf_name) >>> && strncmp (SSDATA (name), SSDATA (Vcode_conversion_workbuf_name), >>> SBYTES (Vcode_conversion_workbuf_name)) == 0); >>> >>> form in 'get-buffer-create'. Any ideas? >> >> Doesn't your patch in https://debbugs.gnu.org/34765#86 already eliminate >> the need for it? > > FWIW no, it leaves that form in place. I know, my emphasis was on eliminating the *need* for it. > And 'get-buffer-create' should not have to worry about delving any > deeper into the name than down to its first character to check for ' ' > or '*'. > > But I'm not happy with my patch anyway. Something more elegant is > needed. The possibilities for the buffer creation subroutine are either to act specially on certain buffer name prefixes, or to accept an extra argument indicating what to do, no? Are there any others? There was mention of exposing a buffer-local variable to Elisp, but IIRC setting that after creating the buffer would already be too late. Buffer names starting with spaces are already special in some contexts, so extending that idea for inhibiting buffer hooks doesn't sound too bad, but the extra flag seems equally elegant and more backward-compatible. Am I missing something? Thanks, -- Basil ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-05-21 7:58 ` Basil L. Contovounesios @ 2019-05-21 10:04 ` martin rudalics 2019-05-22 7:29 ` Eli Zaretskii 2019-05-22 7:21 ` Eli Zaretskii 1 sibling, 1 reply; 72+ messages in thread From: martin rudalics @ 2019-05-21 10:04 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: 34765, alexanderm, monnier >>>> It would be nice to get rid of the entire >>>> >>>> b->inhibit_buffer_hooks >>>> = (STRINGP (Vcode_conversion_workbuf_name) >>>> && strncmp (SSDATA (name), SSDATA (Vcode_conversion_workbuf_name), >>>> SBYTES (Vcode_conversion_workbuf_name)) == 0); >>>> >>>> form in 'get-buffer-create'. Any ideas? >>> >>> Doesn't your patch in https://debbugs.gnu.org/34765#86 already eliminate >>> the need for it? >> >> FWIW no, it leaves that form in place. > > I know, my emphasis was on eliminating the *need* for it. That's what I meant with the above. In code_conversion_save we should get rid of _both_ Fget_buffer_create instances but but I have not managed to understand the Vcode_conversion_workbuf_name vs Vcode_conversion_reused_workbuf rigmarole yet. > The possibilities for the buffer creation subroutine are either to act > specially on certain buffer name prefixes, or to accept an extra > argument indicating what to do, no? Are there any others? There was > mention of exposing a buffer-local variable to Elisp, but IIRC setting > that after creating the buffer would already be too late. So far there is no extra argument, the entire analysis is based on examining the proposed name argument. > Buffer names starting with spaces are already special in some contexts, > so extending that idea for inhibiting buffer hooks doesn't sound too > bad, Eli thinks that "this is too drastic a measure". > but the extra flag seems equally elegant and more > backward-compatible. Am I missing something? The piece we are discussing here namely how to get rid of the stuff at the top of this mail. And issues Eli raised earlier - whether 'get-buffer-create' should accept an extra argument or whether it should set b->inhibit_buffer_hooks = true. martin ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-05-21 10:04 ` martin rudalics @ 2019-05-22 7:29 ` Eli Zaretskii 2019-05-22 8:32 ` martin rudalics 0 siblings, 1 reply; 72+ messages in thread From: Eli Zaretskii @ 2019-05-22 7:29 UTC (permalink / raw) To: martin rudalics; +Cc: contovob, 34765, monnier, alexanderm > From: martin rudalics <rudalics@gmx.at> > Cc: Eli Zaretskii <eliz@gnu.org>, 34765@debbugs.gnu.org, alexanderm@web.de, > monnier@IRO.UMontreal.CA > Date: Tue, 21 May 2019 12:04:46 +0200 > > I have not managed to understand the Vcode_conversion_workbuf_name > vs Vcode_conversion_reused_workbuf rigmarole yet. The latter is a fixed buffer, created once and never killed. So the buffer hooks never run for it, and never affect Emacs. By contrast, the former is a buffer created when the reused buffer is busy and cannot be reused. We then kill Vcode_conversion_workbuf_name when we no longer need it. Thus, buffer hooks run for these work buffers all the time, and for a code-conversion intensive code they could slow down Emacs, specially if the list of buffer hooks is long. > > The possibilities for the buffer creation subroutine are either to act > > specially on certain buffer name prefixes, or to accept an extra > > argument indicating what to do, no? Are there any others? There was > > mention of exposing a buffer-local variable to Elisp, but IIRC setting > > that after creating the buffer would already be too late. > > So far there is no extra argument, the entire analysis is based on > examining the proposed name argument. Since we currently keep this special treatment limited to a small number of buffer names, I'm not sure there's a need to expose this facility to Lisp. > > Buffer names starting with spaces are already special in some contexts, > > so extending that idea for inhibiting buffer hooks doesn't sound too > > bad, > > Eli thinks that "this is too drastic a measure". Yes. No one said buffer hooks must _never_ run for temporary buffers. There could be legitimate Lisp programs which want those hooks to run in that case. ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-05-22 7:29 ` Eli Zaretskii @ 2019-05-22 8:32 ` martin rudalics 2019-05-22 10:06 ` Eli Zaretskii 2019-05-22 14:12 ` Stefan Monnier 0 siblings, 2 replies; 72+ messages in thread From: martin rudalics @ 2019-05-22 8:32 UTC (permalink / raw) To: Eli Zaretskii; +Cc: contovob, 34765, monnier, alexanderm >> I have not managed to understand the Vcode_conversion_workbuf_name >> vs Vcode_conversion_reused_workbuf rigmarole yet. > > The latter is a fixed buffer, created once and never killed. So the > buffer hooks never run for it, and never affect Emacs. By contrast, > the former is a buffer created when the reused buffer is busy and > cannot be reused. We then kill Vcode_conversion_workbuf_name when we > no longer need it. Thus, buffer hooks run for these work buffers all > the time, and for a code-conversion intensive code they could slow > down Emacs, specially if the list of buffer hooks is long. Thanks, I hope I understand it now. Could you pretty please add the (instructive for me) "a buffer created when the reused buffer is busy and cannot be reused" somewhere to the comments maybe together with a reference to reused_workbuf_in_use. Hopefully this also means that we can use _something like_ Fgenerate_new_buffer to replace both instances of Fget_buffer_create in code_conversion_restore. > Since we currently keep this special treatment limited to a small > number of buffer names, I'm not sure there's a need to expose this > facility to Lisp. I'm not sure either. >>> Buffer names starting with spaces are already special in some contexts, >>> so extending that idea for inhibiting buffer hooks doesn't sound too >>> bad, >> >> Eli thinks that "this is too drastic a measure". > > Yes. No one said buffer hooks must _never_ run for temporary buffers. > There could be legitimate Lisp programs which want those hooks to run > in that case. I have to agree. martin ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-05-22 8:32 ` martin rudalics @ 2019-05-22 10:06 ` Eli Zaretskii 2019-05-23 8:38 ` martin rudalics 2019-05-22 14:12 ` Stefan Monnier 1 sibling, 1 reply; 72+ messages in thread From: Eli Zaretskii @ 2019-05-22 10:06 UTC (permalink / raw) To: martin rudalics; +Cc: contovob, 34765, monnier, alexanderm > Cc: contovob@tcd.ie, 34765@debbugs.gnu.org, alexanderm@web.de, > monnier@IRO.UMontreal.CA > From: martin rudalics <rudalics@gmx.at> > Date: Wed, 22 May 2019 10:32:39 +0200 > > >> I have not managed to understand the Vcode_conversion_workbuf_name > >> vs Vcode_conversion_reused_workbuf rigmarole yet. > > > > The latter is a fixed buffer, created once and never killed. So the > > buffer hooks never run for it, and never affect Emacs. By contrast, > > the former is a buffer created when the reused buffer is busy and > > cannot be reused. We then kill Vcode_conversion_workbuf_name when we > > no longer need it. Thus, buffer hooks run for these work buffers all > > the time, and for a code-conversion intensive code they could slow > > down Emacs, specially if the list of buffer hooks is long. > > Thanks, I hope I understand it now. Could you pretty please add the > (instructive for me) "a buffer created when the reused buffer is busy > and cannot be reused" somewhere to the comments maybe together with a > reference to reused_workbuf_in_use. Not sure where you want to add this. coding.c already says: /* Name (or base name) of work buffer for code conversion. */ Lisp_Object Vcode_conversion_workbuf_name; /* A working buffer used by the top level conversion. Once it is created, it is never destroyed. It has the name Vcode_conversion_workbuf_name. The other working buffers are destroyed after the use is finished, and their names are modified versions of Vcode_conversion_workbuf_name. */ static Lisp_Object Vcode_conversion_reused_workbuf; /* True iff Vcode_conversion_reused_workbuf is already in use. */ static bool reused_workbuf_in_use; is that what you wanted to see? > Hopefully this also means that we can use _something like_ > Fgenerate_new_buffer to replace both instances of Fget_buffer_create > in code_conversion_restore. You mean code_conversion_save, I presume. One of those calls to Fget_buffer_create shouldn't generate a new name, though. It should always use a fixed name. ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-05-22 10:06 ` Eli Zaretskii @ 2019-05-23 8:38 ` martin rudalics 2019-05-23 14:37 ` Eli Zaretskii 0 siblings, 1 reply; 72+ messages in thread From: martin rudalics @ 2019-05-23 8:38 UTC (permalink / raw) To: Eli Zaretskii; +Cc: contovob, 34765, monnier, alexanderm >> Thanks, I hope I understand it now. Could you pretty please add the >> (instructive for me) "a buffer created when the reused buffer is busy >> and cannot be reused" somewhere to the comments maybe together with a >> reference to reused_workbuf_in_use. > > Not sure where you want to add this. coding.c already says: That's what I've read and did not understand. It's a bit too concise. > /* Name (or base name) of work buffer for code conversion. */ > Lisp_Object Vcode_conversion_workbuf_name; This comment insinuates that there is only one such buffer. > /* A working buffer used by the top level conversion. What is the "top level conversion"? > Once it is > created, it is never destroyed. It has the name > Vcode_conversion_workbuf_name. The other working buffers are > destroyed after the use is finished, and their names are modified > versions of Vcode_conversion_workbuf_name. */ > static Lisp_Object Vcode_conversion_reused_workbuf; > > /* True iff Vcode_conversion_reused_workbuf is already in use. */ > static bool reused_workbuf_in_use; > > is that what you wanted to see? I'd prefer something like a common comment for all these variables going as /* The internal work buffers for code conversion are created lazily on demand. The name of the first buffer created that way is specified by Vcode_conversion_workbuf_name. Once created, this buffer is no more deleted in the current Emacs session. While this buffer is in use, the boolean variable reused_workbuf_in_use is true. This buffer is reused for new conversions whenever reused_workbuf_in_use is false. When reused_workbuf_in_use is true and more code conversion work has to be done, a new buffer is created. The name of that new buffer is generated by Fgenerate_new_buffer_name, using Vcode_conversion_workbuf_name as base name. Any such buffer is destroyed immediately as soon as it is no more used. */ >> Hopefully this also means that we can use _something like_ >> Fgenerate_new_buffer to replace both instances of Fget_buffer_create >> in code_conversion_restore. > > You mean code_conversion_save, I presume. You presume correctly. > One of those calls to Fget_buffer_create shouldn't generate a new > name, though. It should always use a fixed name. Fgenerate_new_buffer_name should do that: "If there is no live buffer named NAME, then return NAME.". Concludingly, what we need in the present context is a function - to create a buffer whose name is generated from a base name argument, - to set the new buffer's inhibit_buffer_hooks flag according to a corresponding argument and to not run any hooks when that flag should be set and the buffer is created, - that is as internal as possible to avoid that applications misuse it (it can't be entirely internal since macros like 'with-temp-buffer' will need it). martin ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-05-23 8:38 ` martin rudalics @ 2019-05-23 14:37 ` Eli Zaretskii 2019-05-24 8:01 ` martin rudalics 0 siblings, 1 reply; 72+ messages in thread From: Eli Zaretskii @ 2019-05-23 14:37 UTC (permalink / raw) To: martin rudalics; +Cc: contovob, 34765, monnier, alexanderm > Cc: contovob@tcd.ie, 34765@debbugs.gnu.org, alexanderm@web.de, > monnier@IRO.UMontreal.CA > From: martin rudalics <rudalics@gmx.at> > Date: Thu, 23 May 2019 10:38:38 +0200 > > I'd prefer something like a common comment for all these variables > going as Your wish is my command. Done. ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-05-23 14:37 ` Eli Zaretskii @ 2019-05-24 8:01 ` martin rudalics 0 siblings, 0 replies; 72+ messages in thread From: martin rudalics @ 2019-05-24 8:01 UTC (permalink / raw) To: Eli Zaretskii; +Cc: contovob, 34765, monnier, alexanderm > Your wish is my command. Done. Perfect! Many thanks, martin ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-05-22 8:32 ` martin rudalics 2019-05-22 10:06 ` Eli Zaretskii @ 2019-05-22 14:12 ` Stefan Monnier 2019-05-22 15:50 ` Eli Zaretskii 1 sibling, 1 reply; 72+ messages in thread From: Stefan Monnier @ 2019-05-22 14:12 UTC (permalink / raw) To: martin rudalics; +Cc: contovob, 34765, alexanderm >> Since we currently keep this special treatment limited to a small >> number of buffer names, I'm not sure there's a need to expose this >> facility to Lisp. > I'm not sure either. How 'bout this: additionally to "hidden buffers" which start with a SPC character, we introduce the notion of "internal buffers" whose name starts with another special first char or with two SPC chars and disable those buffers's hooks. Stefan ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-05-22 14:12 ` Stefan Monnier @ 2019-05-22 15:50 ` Eli Zaretskii 0 siblings, 0 replies; 72+ messages in thread From: Eli Zaretskii @ 2019-05-22 15:50 UTC (permalink / raw) To: Stefan Monnier; +Cc: contovob, 34765, alexanderm > From: Stefan Monnier <monnier@iro.umontreal.ca> > Cc: Eli Zaretskii <eliz@gnu.org>, contovob@tcd.ie, 34765@debbugs.gnu.org, alexanderm@web.de > Date: Wed, 22 May 2019 10:12:36 -0400 > > How 'bout this: additionally to "hidden buffers" which start with a SPC > character, we introduce the notion of "internal buffers" whose name > starts with another special first char or with two SPC chars and disable > those buffers's hooks. It's possible, if there's a frequent enough use case for such automatic disabling. Is there? In general, since the hook can be disabled in each buffer individually, we'd need a very good reason to provide another facility. ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-05-21 7:58 ` Basil L. Contovounesios 2019-05-21 10:04 ` martin rudalics @ 2019-05-22 7:21 ` Eli Zaretskii 1 sibling, 0 replies; 72+ messages in thread From: Eli Zaretskii @ 2019-05-22 7:21 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: 34765, monnier, alexanderm > From: "Basil L. Contovounesios" <contovob@tcd.ie> > Cc: Eli Zaretskii <eliz@gnu.org>, 34765@debbugs.gnu.org, alexanderm@web.de, monnier@IRO.UMontreal.CA > Date: Tue, 21 May 2019 08:58:17 +0100 > > The possibilities for the buffer creation subroutine are either to act > specially on certain buffer name prefixes, or to accept an extra > argument indicating what to do, no? Are there any others? There was > mention of exposing a buffer-local variable to Elisp, but IIRC setting > that after creating the buffer would already be too late. > > Buffer names starting with spaces are already special in some contexts, > so extending that idea for inhibiting buffer hooks doesn't sound too > bad, but the extra flag seems equally elegant and more > backward-compatible. Am I missing something? The flag in the C structure has the advantage that it can be easily set and reset from C without the need to temporarily bind Lisp variables, which then involves unwind-protect forms etc., and as a side effect makes the entire operation slower. Also, a flag can be set/reset regardless of whether the buffer already finished being created, whereas buffer-local variables must have a live buffer. ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-04-26 11:00 ` martin rudalics 2019-04-26 11:26 ` Eli Zaretskii 2019-04-26 17:14 ` Basil L. Contovounesios @ 2020-08-26 11:06 ` Lars Ingebrigtsen 2020-09-05 7:05 ` Eli Zaretskii 2 siblings, 1 reply; 72+ messages in thread From: Lars Ingebrigtsen @ 2020-08-26 11:06 UTC (permalink / raw) To: martin rudalics; +Cc: 34765, alexanderm, monnier martin rudalics <rudalics@gmx.at> writes: > I attach a preliminary patch. [...] > +DEFUN ("generate-new-buffer", Fgenerate_new_buffer, Sgenerate_new_buffer, > + 1, 2, 0, > + doc: /* Create and return a buffer with a name based on NAME. > +Choose the buffer's name using `generate-new-buffer-name'. > + > +Optional second argument INHIBIT-BUFFER-HOOKS non-nil means to not run > +any buffer hooks ('kill-buffer-hook', 'buffer-list-update-hook' or > +'kill-buffer-query-functions') for this buffer. This argument should > +be set only for internal buffers that are never presented to users or > +passed on to other applications. */) Reading this thread, it seemed like everybody agreed that this patch (in itself) was a good change, and then the discussion turned to other ways of specifying "internal" Emacs buffers (that shouldn't have the hooks run as well, as double-spacing " *temp*", etc). The latter may be a nice addition, but the patch here is pretty much necessary for implementing something like that, so was there any reason the patch wasn't applied? -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-08-26 11:06 ` Lars Ingebrigtsen @ 2020-09-05 7:05 ` Eli Zaretskii 2020-10-07 3:27 ` Lars Ingebrigtsen 0 siblings, 1 reply; 72+ messages in thread From: Eli Zaretskii @ 2020-09-05 7:05 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: 34765, monnier, alexanderm > From: Lars Ingebrigtsen <larsi@gnus.org> > Cc: Eli Zaretskii <eliz@gnu.org>, 34765@debbugs.gnu.org, > alexanderm@web.de, monnier@IRO.UMontreal.CA > Date: Wed, 26 Aug 2020 13:06:13 +0200 > > martin rudalics <rudalics@gmx.at> writes: > > > I attach a preliminary patch. > > [...] > > > +DEFUN ("generate-new-buffer", Fgenerate_new_buffer, Sgenerate_new_buffer, > > + 1, 2, 0, > > + doc: /* Create and return a buffer with a name based on NAME. > > +Choose the buffer's name using `generate-new-buffer-name'. > > + > > +Optional second argument INHIBIT-BUFFER-HOOKS non-nil means to not run > > +any buffer hooks ('kill-buffer-hook', 'buffer-list-update-hook' or > > +'kill-buffer-query-functions') for this buffer. This argument should > > +be set only for internal buffers that are never presented to users or > > +passed on to other applications. */) > > Reading this thread, it seemed like everybody agreed that this patch (in > itself) was a good change, and then the discussion turned to other ways > of specifying "internal" Emacs buffers (that shouldn't have the hooks > run as well, as double-spacing " *temp*", etc). > > The latter may be a nice addition, but the patch here is pretty much > necessary for implementing something like that, so was there any reason > the patch wasn't applied? AFAIU, the patch was never finalized after the review comments. ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-09-05 7:05 ` Eli Zaretskii @ 2020-10-07 3:27 ` Lars Ingebrigtsen 2020-10-07 7:58 ` martin rudalics 0 siblings, 1 reply; 72+ messages in thread From: Lars Ingebrigtsen @ 2020-10-07 3:27 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 34765, monnier, alexanderm Eli Zaretskii <eliz@gnu.org> writes: >> The latter may be a nice addition, but the patch here is pretty much >> necessary for implementing something like that, so was there any reason >> the patch wasn't applied? > > AFAIU, the patch was never finalized after the review comments. Right. Martin, did you finish up this patch? -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-10-07 3:27 ` Lars Ingebrigtsen @ 2020-10-07 7:58 ` martin rudalics 2020-11-29 21:03 ` Basil L. Contovounesios 0 siblings, 1 reply; 72+ messages in thread From: martin rudalics @ 2020-10-07 7:58 UTC (permalink / raw) To: Lars Ingebrigtsen, Eli Zaretskii; +Cc: 34765, alexanderm, monnier >>> The latter may be a nice addition, but the patch here is pretty much >>> necessary for implementing something like that, so was there any reason >>> the patch wasn't applied? >> >> AFAIU, the patch was never finalized after the review comments. > > Right. Martin, did you finish up this patch? No. IIRC some issues were never resolved and Basil also wanted to add something. If you can make heads or tails of it ... martin ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-10-07 7:58 ` martin rudalics @ 2020-11-29 21:03 ` Basil L. Contovounesios 2020-11-30 9:05 ` martin rudalics 0 siblings, 1 reply; 72+ messages in thread From: Basil L. Contovounesios @ 2020-11-29 21:03 UTC (permalink / raw) To: martin rudalics; +Cc: 34765, alexanderm, Lars Ingebrigtsen, monnier martin rudalics <rudalics@gmx.at> writes: >>>> The latter may be a nice addition, but the patch here is pretty much >>>> necessary for implementing something like that, so was there any reason >>>> the patch wasn't applied? >>> >>> AFAIU, the patch was never finalized after the review comments. >> >> Right. Martin, did you finish up this patch? > > No. IIRC some issues were never resolved and Basil also wanted to add > something. Did I? AFAICT I was only pointing out the trivial simplification of Fgenerate_new_buffer_name + Fget_buffer_create = Fgenerate_new_buffer once Fgenerate_new_buffer can be called from C. I was otherwise happy with the patch and either of the proposed approaches to setting inhibit_buffer_hooks. > If you can make heads or tails of it ... Depends: is it an animal or a coin? -- Basil ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-11-29 21:03 ` Basil L. Contovounesios @ 2020-11-30 9:05 ` martin rudalics 2020-11-30 18:07 ` Basil L. Contovounesios 0 siblings, 1 reply; 72+ messages in thread From: martin rudalics @ 2020-11-30 9:05 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: 34765, alexanderm, Lars Ingebrigtsen, monnier >>>>> The latter may be a nice addition, but the patch here is pretty much >>>>> necessary for implementing something like that, so was there any reason >>>>> the patch wasn't applied? >>>> >>>> AFAIU, the patch was never finalized after the review comments. >>> >>> Right. Martin, did you finish up this patch? >> >> No. IIRC some issues were never resolved and Basil also wanted to add >> something. > > Did I? AFAICT I was only pointing out the trivial simplification of > Fgenerate_new_buffer_name + Fget_buffer_create = Fgenerate_new_buffer > once Fgenerate_new_buffer can be called from C. > > I was otherwise happy with the patch and either of the proposed > approaches to setting inhibit_buffer_hooks. > >> If you can make heads or tails of it ... > > Depends: is it an animal or a coin? Both, probably. If you think that patch is of some use, please try to polish it up and push it. Honestly, I've forgotten all about it. And also tell us why 'buffer-list-update-hook' would be still needed now that we have 'window-selection-change-functions'. Thanks, martin ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-11-30 9:05 ` martin rudalics @ 2020-11-30 18:07 ` Basil L. Contovounesios 2020-11-30 19:01 ` martin rudalics 2020-11-30 19:42 ` Eli Zaretskii 0 siblings, 2 replies; 72+ messages in thread From: Basil L. Contovounesios @ 2020-11-30 18:07 UTC (permalink / raw) To: martin rudalics; +Cc: 34765, alexanderm, Lars Ingebrigtsen, monnier martin rudalics <rudalics@gmx.at> writes: > If you think that patch is of some use, please try to > polish it up and push it. Honestly, I've forgotten all about it. Sure, I'll give it a go. I'm slightly confused about the conclusion of this thread, though. You and Eli said it was too radical to set inhibit_buffer_hooks based on whether a buffer name starts with a space, as there may be legitimate cases where those hooks should run in a temporary buffer. I accept this, but I don't see how setting inhibit_buffer_hooks based on a new optional argument to Fgenerate_new_buffer (and/or Fget_buffer_create) solves the problem entirely. If I create a temporary buffer with the proposed (generate-new-buffer "foo" t) then how do I later tell Emacs that this buffer's hooks should run? In other words, can we be sure that the buffers we choose to create with inhibit_buffer_hooks set will *never* need to later unset it? Should we expose a getter or setter for this buffer member, or is that opening ourselves up to abuse? I suppose if someone *really* wanted to un-temporarify a buffer with inhibit_buffer_hooks set, they could create a new, non-temporary buffer and swap buffer contents, or something to that effect? Am I wildly misunderstanding something here? > And also tell us why 'buffer-list-update-hook' would be still needed now > that we have 'window-selection-change-functions'. I've yet to look into these hooks and their precise semantics, but are you suggesting that one might obsolete the other, in which case we should deprecate the old one? Thanks, -- Basil ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-11-30 18:07 ` Basil L. Contovounesios @ 2020-11-30 19:01 ` martin rudalics 2020-11-30 20:33 ` Basil L. Contovounesios 2020-11-30 19:42 ` Eli Zaretskii 1 sibling, 1 reply; 72+ messages in thread From: martin rudalics @ 2020-11-30 19:01 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: 34765, alexanderm, Lars Ingebrigtsen, monnier > I'm slightly confused about the conclusion of this thread, though. > You and Eli said it was too radical to set inhibit_buffer_hooks based > on whether a buffer name starts with a space, as there may be legitimate > cases where those hooks should run in a temporary buffer. Yes. > I accept this, but I don't see how setting inhibit_buffer_hooks based on > a new optional argument to Fgenerate_new_buffer (and/or > Fget_buffer_create) solves the problem entirely. > > If I create a temporary buffer with the proposed > > (generate-new-buffer "foo" t) > > then how do I later tell Emacs that this buffer's hooks should run? > > In other words, can we be sure that the buffers we choose to create with > inhibit_buffer_hooks set will *never* need to later unset it? > > Should we expose a getter or setter for this buffer member, or is that > opening ourselves up to abuse? I'm not sure. But if we do that, we can get rid of the rest: We'd just generate the new buffer and set/reset its hook inhibitor afterwards. > I suppose if someone *really* wanted to un-temporarify a buffer with > inhibit_buffer_hooks set, they could create a new, non-temporary buffer > and swap buffer contents, or something to that effect? Admittedly clumsy. > Am I wildly misunderstanding something here? I don't think so. >> And also tell us why 'buffer-list-update-hook' would be still needed now >> that we have 'window-selection-change-functions'. > > I've yet to look into these hooks and their precise semantics, but are > you suggesting that one might obsolete the other, in which case we > should deprecate the old one? People usually don't care about the buffer lists, they always wanted to run a hook when a window got selected in a more permanent fashion. At the time of adding 'buffer-list-update-hook', I thought that it fulfilled some of the criteria wanted for a 'select-window-hook' and added text in that sense to the manual. So if you care about the buffer lists, 'buffer-list-update-hook' is the one to use. If you care about window selection, 'window-selection-change-functions' is the one. martin ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-11-30 19:01 ` martin rudalics @ 2020-11-30 20:33 ` Basil L. Contovounesios 2020-12-01 9:34 ` martin rudalics 0 siblings, 1 reply; 72+ messages in thread From: Basil L. Contovounesios @ 2020-11-30 20:33 UTC (permalink / raw) To: martin rudalics; +Cc: 34765, alexanderm, Lars Ingebrigtsen, monnier martin rudalics <rudalics@gmx.at> writes: >> Should we expose a getter or setter for this buffer member, or is that >> opening ourselves up to abuse? > > I'm not sure. But if we do that, we can get rid of the rest: We'd just > generate the new buffer and set/reset its hook inhibitor afterwards. Not necessarily, because Fget_buffer_create still has to run the hook before returning the buffer. >>> And also tell us why 'buffer-list-update-hook' would be still needed now >>> that we have 'window-selection-change-functions'. >> >> I've yet to look into these hooks and their precise semantics, but are >> you suggesting that one might obsolete the other, in which case we >> should deprecate the old one? > > People usually don't care about the buffer lists, they always wanted to > run a hook when a window got selected in a more permanent fashion. At > the time of adding 'buffer-list-update-hook', I thought that it > fulfilled some of the criteria wanted for a 'select-window-hook' and > added text in that sense to the manual. So if you care about the buffer > lists, 'buffer-list-update-hook' is the one to use. If you care about > window selection, 'window-selection-change-functions' is the one. Thanks, then it sounds to me like there's nothing to be done, other than recommend the right hook for the right job. Right? -- Basil ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-11-30 20:33 ` Basil L. Contovounesios @ 2020-12-01 9:34 ` martin rudalics 0 siblings, 0 replies; 72+ messages in thread From: martin rudalics @ 2020-12-01 9:34 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: 34765, alexanderm, Lars Ingebrigtsen, monnier > Thanks, then it sounds to me like there's nothing to be done, other than > recommend the right hook for the right job. Right? Right. martin ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-11-30 18:07 ` Basil L. Contovounesios 2020-11-30 19:01 ` martin rudalics @ 2020-11-30 19:42 ` Eli Zaretskii 2020-11-30 20:34 ` Basil L. Contovounesios 1 sibling, 1 reply; 72+ messages in thread From: Eli Zaretskii @ 2020-11-30 19:42 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: 34765, larsi, monnier, alexanderm > From: "Basil L. Contovounesios" <contovob@tcd.ie> > Cc: Lars Ingebrigtsen <larsi@gnus.org>, Eli Zaretskii <eliz@gnu.org>, > 34765@debbugs.gnu.org, alexanderm@web.de, monnier@IRO.UMontreal.CA > Date: Mon, 30 Nov 2020 18:07:12 +0000 > > I'm slightly confused about the conclusion of this thread, though. > You and Eli said it was too radical to set inhibit_buffer_hooks based > on whether a buffer name starts with a space, as there may be legitimate > cases where those hooks should run in a temporary buffer. > > I accept this, but I don't see how setting inhibit_buffer_hooks based on > a new optional argument to Fgenerate_new_buffer (and/or > Fget_buffer_create) solves the problem entirely. > > If I create a temporary buffer with the proposed > > (generate-new-buffer "foo" t) > > then how do I later tell Emacs that this buffer's hooks should run? The assumption is that you don't want to. Recall that the original idea was to turn the hooks off unconditionally based on the buffer's name. The above is a less drastic measure: it allows the code which creates the buffer to request that regardless of the name. But the original motivation, that there are buffers where we want to never run the hooks, is still valid. > In other words, can we be sure that the buffers we choose to create with > inhibit_buffer_hooks set will *never* need to later unset it? We didn't see examples of such buffers, so we pretend they don't exist. > Should we expose a getter or setter for this buffer member Not until someone comes up crying that this is needed, and presents a convincing use case, IMO. ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-11-30 19:42 ` Eli Zaretskii @ 2020-11-30 20:34 ` Basil L. Contovounesios 2020-12-07 22:16 ` Basil L. Contovounesios 0 siblings, 1 reply; 72+ messages in thread From: Basil L. Contovounesios @ 2020-11-30 20:34 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 34765, larsi, monnier, alexanderm Eli Zaretskii <eliz@gnu.org> writes: >> From: "Basil L. Contovounesios" <contovob@tcd.ie> >> Cc: Lars Ingebrigtsen <larsi@gnus.org>, Eli Zaretskii <eliz@gnu.org>, >> 34765@debbugs.gnu.org, alexanderm@web.de, monnier@IRO.UMontreal.CA >> Date: Mon, 30 Nov 2020 18:07:12 +0000 >> >> If I create a temporary buffer with the proposed >> >> (generate-new-buffer "foo" t) >> >> then how do I later tell Emacs that this buffer's hooks should run? > > The assumption is that you don't want to. Recall that the original > idea was to turn the hooks off unconditionally based on the buffer's > name. The above is a less drastic measure: it allows the code which > creates the buffer to request that regardless of the name. But the > original motivation, that there are buffers where we want to never run > the hooks, is still valid. > >> In other words, can we be sure that the buffers we choose to create with >> inhibit_buffer_hooks set will *never* need to later unset it? > > We didn't see examples of such buffers, so we pretend they don't > exist. > >> Should we expose a getter or setter for this buffer member > > Not until someone comes up crying that this is needed, and presents a > convincing use case, IMO. WFM. I'll finish up Martin's patch with some docs then. Thanks, -- Basil ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-11-30 20:34 ` Basil L. Contovounesios @ 2020-12-07 22:16 ` Basil L. Contovounesios 2020-12-07 22:37 ` Basil L. Contovounesios ` (2 more replies) 0 siblings, 3 replies; 72+ messages in thread From: Basil L. Contovounesios @ 2020-12-07 22:16 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 34765, larsi, monnier, alexanderm [-- Attachment #1: Type: text/plain, Size: 759 bytes --] "Basil L. Contovounesios" <contovob@tcd.ie> writes: > I'll finish up Martin's patch with some docs then. How's the attached? The gist of it is that get-buffer-create and generate-new-buffer gain a new argument for inhibiting buffer hooks, which is set in with-temp-buffer, with-temp-file, with-output-to-string, insert-file-contents, Vprin1_to_string_buffer, and code_conversion_save. Arising questions: 0. Why isn't kill-buffer-hook defined as a DEFVAR_LISP? Is it because it has to be defined in init_buffer_once? If so, why is that the case? 1. Is it okay for with-temp-buffer, with-temp-file, and with-output-to-string to unconditionally inhibit buffer hooks, or should this be controlled by e.g. a global variable? Thanks, -- Basil [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Inhibit-buffer-hooks-in-temporary-buffers.patch --] [-- Type: text/x-diff, Size: 45895 bytes --] From 6847a28e105094d6e024b1239520d45498ff8653 Mon Sep 17 00:00:00 2001 From: "Basil L. Contovounesios" <contovob@tcd.ie> Date: Tue, 1 Dec 2020 01:12:32 +0000 Subject: [PATCH] Inhibit buffer hooks in temporary buffers Give get-buffer-create an optional argument to inhibit buffer hooks in internal or temporary buffers for efficiency (bug#34765). * .dir-locals.el (c-mode): Enforce existing indent-tabs-mode policy. * etc/NEWS: Announce new parameter of get-buffer-create and generate-new-buffer, and that with-temp-buffer and with-temp-file now inhibit buffer hooks. * doc/lispref/buffers.texi (Buffer Names): Fix typo. (Creating Buffers): Document new parameter of get-buffer-create and generate-new-buffer. (Buffer List, Killing Buffers): Document when buffer hooks are inhibited. (Current Buffer): * doc/lispref/files.texi (Writing to Files): Document that with-temp-buffer and with-temp-file inhibit buffer hooks. * doc/lispref/internals.texi (Buffer Internals): Document inhibit_buffer_hooks flag. Remove stale comment. * doc/misc/gnus-faq.texi (FAQ 5-8): * lisp/simple.el (shell-command-on-region): Fix indentation. * lisp/files.el (kill-buffer-hook): Document when hook is inhibited. (create-file-buffer): * lisp/gnus/gnus-uu.el (gnus-uu-unshar-article): * lisp/international/mule.el (load-with-code-conversion): * lisp/mh-e/mh-xface.el (mh-x-image-url-fetch-image): * lisp/net/imap.el (imap-open): * lisp/net/mailcap.el (mailcap-maybe-eval): * lisp/progmodes/flymake-proc.el (flymake-proc--read-file-to-temp-buffer) (flymake-proc--copy-buffer-to-temp-buffer): Simplify. * lisp/subr.el (generate-new-buffer): Forward new optional argument to inhibit buffer hooks to get-buffer-create. (with-temp-buffer, with-temp-file, with-output-to-string): Inhibit buffer hooks in created buffer. * src/buffer.c (run_buffer_list_update_hook): New helper function. (Fget_buffer_create): Use it. Add optional argument to set inhibit_buffer_hooks flag instead of comparing the buffer name to Vcode_conversion_workbuf_name. All callers changed. (Fmake_indirect_buffer, Frename_buffer, Fbury_buffer_internal) (record_buffer): Use run_buffer_list_update_hook. (Fkill_buffer): Document when buffer hooks are inhibited. Use run_buffer_list_update_hook. (init_buffer_once): Inhibit buffer hooks in Vprin1_to_string_buffer. (Vkill_buffer_query_functions, Vbuffer_list_update_hook): Document when hooks are inhibited. * src/buffer.h (struct buffer): Update inhibit_buffer_hooks commentary. * src/coding.h (Vcode_conversion_workbuf_name): * src/coding.c (Vcode_conversion_workbuf_name): Make static again since it is no longer needed in src/buffer.c. (code_conversion_restore, code_conversion_save, syms_of_coding): Prefer boolean over integer constants. * src/fileio.c (Finsert_file_contents): Inhibit buffer hooks in " *code-converting-work*" buffer. * src/window.c (Fselect_window): Fix grammar. Mention window-selection-change-functions alongside buffer-list-update-hook. * test/src/buffer-tests.el: Fix requires. (buffer-tests-inhibit-buffer-hooks): New test. --- .dir-locals.el | 1 + doc/lispref/buffers.texi | 55 ++++++++++++++++------ doc/lispref/files.texi | 7 ++- doc/lispref/internals.texi | 11 ++++- doc/misc/gnus-faq.texi | 8 ++-- etc/NEWS | 16 +++++++ lisp/files.el | 6 ++- lisp/gnus/gnus-uu.el | 3 +- lisp/international/mule.el | 9 ++-- lisp/mh-e/mh-xface.el | 3 +- lisp/net/imap.el | 3 +- lisp/net/mailcap.el | 3 +- lisp/progmodes/flymake-proc.el | 13 +++-- lisp/simple.el | 3 +- lisp/subr.el | 54 ++++++++++----------- src/buffer.c | 86 +++++++++++++++++++--------------- src/buffer.h | 10 ++-- src/callproc.c | 5 +- src/coding.c | 12 ++--- src/coding.h | 3 -- src/fileio.c | 2 +- src/minibuf.c | 2 +- src/print.c | 2 +- src/process.c | 12 ++--- src/w32fns.c | 2 +- src/window.c | 7 +-- src/xdisp.c | 4 +- src/xfns.c | 2 +- src/xwidget.c | 3 +- test/src/buffer-tests.el | 33 +++++++++++-- 30 files changed, 228 insertions(+), 152 deletions(-) diff --git a/.dir-locals.el b/.dir-locals.el index 27d50c6069..b313945936 100644 --- a/.dir-locals.el +++ b/.dir-locals.el @@ -9,6 +9,7 @@ (c-noise-macro-names . ("INLINE" "ATTRIBUTE_NO_SANITIZE_UNDEFINED" "UNINIT" "CALLBACK" "ALIGN_STACK")) (electric-quote-comment . nil) (electric-quote-string . nil) + (indent-tabs-mode . t) (mode . bug-reference-prog))) (objc-mode . ((c-file-style . "GNU") (electric-quote-comment . nil) diff --git a/doc/lispref/buffers.texi b/doc/lispref/buffers.texi index 2860343628..c8c4852e30 100644 --- a/doc/lispref/buffers.texi +++ b/doc/lispref/buffers.texi @@ -225,13 +225,18 @@ Current Buffer @defmac with-temp-buffer body@dots{} @anchor{Definition of with-temp-buffer} -The @code{with-temp-buffer} macro evaluates the @var{body} forms -with a temporary buffer as the current buffer. It saves the identity of +The @code{with-temp-buffer} macro evaluates the @var{body} forms with +a temporary buffer as the current buffer. It saves the identity of the current buffer, creates a temporary buffer and makes it current, evaluates the @var{body} forms, and finally restores the previous -current buffer while killing the temporary buffer. By default, undo -information (@pxref{Undo}) is not recorded in the buffer created by -this macro (but @var{body} can enable that, if needed). +current buffer while killing the temporary buffer. + +By default, undo information (@pxref{Undo}) is not recorded in the +buffer created by this macro (but @var{body} can enable that, if +needed). The temporary buffer also does not run the hooks +@code{kill-buffer-hook}, @code{kill-buffer-query-functions} +(@pxref{Killing Buffers}), and @code{buffer-list-update-hook} +(@pxref{Buffer List}). The return value is the value of the last form in @var{body}. You can return the contents of the temporary buffer by using @@ -345,9 +350,9 @@ Buffer Names If the optional second argument @var{ignore} is non-@code{nil}, it should be a string, a potential buffer name. It means to consider -that potential buffer acceptable, if it is tried, even it is the name -of an existing buffer (which would normally be rejected). Thus, if -buffers named @samp{foo}, @samp{foo<2>}, @samp{foo<3>} and +that potential buffer acceptable, if it is tried, even if it is the +name of an existing buffer (which would normally be rejected). Thus, +if buffers named @samp{foo}, @samp{foo<2>}, @samp{foo<3>} and @samp{foo<4>} exist, @example @@ -932,13 +937,17 @@ Buffer List @defvar buffer-list-update-hook This is a normal hook run whenever the buffer list changes. Functions (implicitly) running this hook are @code{get-buffer-create} -(@pxref{Creating Buffers}), @code{rename-buffer} (@pxref{Buffer Names}), -@code{kill-buffer} (@pxref{Killing Buffers}), @code{bury-buffer} (see -above) and @code{select-window} (@pxref{Selecting Windows}). +(@pxref{Creating Buffers}), @code{rename-buffer} (@pxref{Buffer +Names}), @code{kill-buffer} (@pxref{Killing Buffers}), +@code{bury-buffer} (see above), and @code{select-window} +(@pxref{Selecting Windows}). This hook is not run for internal or +temporary buffers created by @code{get-buffer-create} or +@code{generate-new-buffer} with a non-@code{nil} argument +@var{inhibit-buffer-hooks}. Functions run by this hook should avoid calling @code{select-window} -with a nil @var{norecord} argument or @code{with-temp-buffer} since -either may lead to infinite recursion. +with a @code{nil} @var{norecord} argument since this may lead to +infinite recursion. @end defvar @node Creating Buffers @@ -951,12 +960,20 @@ Creating Buffers with the specified name; @code{generate-new-buffer} always creates a new buffer and gives it a unique name. + Both functions accept an optional argument @var{inhibit-buffer-hooks}. +If it is non-@code{nil}, the buffer they create does not run the hooks +@code{kill-buffer-hook}, @code{kill-buffer-query-functions} +(@pxref{Killing Buffers}), and @code{buffer-list-update-hook} +(@pxref{Buffer List}). This avoids slowing down internal or temporary +buffers that are never presented to users or passed on to other +applications. + Other functions you can use to create buffers include @code{with-output-to-temp-buffer} (@pxref{Temporary Displays}) and @code{create-file-buffer} (@pxref{Visiting Files}). Starting a subprocess can also create a buffer (@pxref{Processes}). -@defun get-buffer-create buffer-or-name +@defun get-buffer-create buffer-or-name &optional inhibit-buffer-hooks This function returns a buffer named @var{buffer-or-name}. The buffer returned does not become the current buffer---this function does not change which buffer is current. @@ -980,7 +997,7 @@ Creating Buffers buffer initially disables undo information recording (@pxref{Undo}). @end defun -@defun generate-new-buffer name +@defun generate-new-buffer name &optional inhibit-buffer-hooks This function returns a newly created, empty buffer, but does not make it current. The name of the buffer is generated by passing @var{name} to the function @code{generate-new-buffer-name} (@pxref{Buffer @@ -1092,6 +1109,10 @@ Killing Buffers they are called. The idea of this feature is that these functions will ask for confirmation from the user. If any of them returns @code{nil}, @code{kill-buffer} spares the buffer's life. + +This hook is not run for internal or temporary buffers created by +@code{get-buffer-create} or @code{generate-new-buffer} with a +non-@code{nil} argument @var{inhibit-buffer-hooks}. @end defvar @defvar kill-buffer-hook @@ -1100,6 +1121,10 @@ Killing Buffers The buffer to be killed is current when the hook functions run. @xref{Hooks}. This variable is a permanent local, so its local binding is not cleared by changing major modes. + +This hook is not run for internal or temporary buffers created by +@code{get-buffer-create} or @code{generate-new-buffer} with a +non-@code{nil} argument @var{inhibit-buffer-hooks}. @end defvar @defopt buffer-offer-save diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi index d49ac42bb4..6949ca29c6 100644 --- a/doc/lispref/files.texi +++ b/doc/lispref/files.texi @@ -701,8 +701,11 @@ Writing to Files The current buffer is restored even in case of an abnormal exit via @code{throw} or error (@pxref{Nonlocal Exits}). -See also @code{with-temp-buffer} in @ref{Definition of -with-temp-buffer,, The Current Buffer}. +Like @code{with-temp-buffer} (@pxref{Definition of with-temp-buffer,, +Current Buffer}), the temporary buffer used by this macro does not run +the hooks @code{kill-buffer-hook}, @code{kill-buffer-query-functions} +(@pxref{Killing Buffers}), and @code{buffer-list-update-hook} +(@pxref{Buffer List}). @end defmac @node File Locks diff --git a/doc/lispref/internals.texi b/doc/lispref/internals.texi index bb25983aa4..3ba59298f1 100644 --- a/doc/lispref/internals.texi +++ b/doc/lispref/internals.texi @@ -2375,6 +2375,15 @@ Buffer Internals This flag indicates that redisplay optimizations should not be used to display this buffer. +@item inhibit_buffer_hooks +This flag indicates that the buffer should not run the hooks +@code{kill-buffer-hook}, @code{kill-buffer-query-functions} +(@pxref{Killing Buffers}), and @code{buffer-list-update-hook} +(@pxref{Buffer List}). It is set at buffer creation (@pxref{Creating +Buffers}), and avoids slowing down internally used temporary buffers, +such as those created by @code{with-temp-buffer} (@pxref{Definition of +with-temp-buffer,, Current Buffer}). + @item overlay_center This field holds the current overlay center position. @xref{Managing Overlays}. @@ -2388,8 +2397,6 @@ Buffer Internals and @code{overlays_after} is sorted in order of increasing beginning position. -@c FIXME? the following are now all Lisp_Object BUFFER_INTERNAL_FIELD (foo). - @item name A Lisp string that names the buffer. It is guaranteed to be unique. @xref{Buffer Names}. This and the following fields have their names diff --git a/doc/misc/gnus-faq.texi b/doc/misc/gnus-faq.texi index adb812f572..c30e80ff56 100644 --- a/doc/misc/gnus-faq.texi +++ b/doc/misc/gnus-faq.texi @@ -1523,10 +1523,10 @@ FAQ 5-8 @example (setq message-default-headers - (with-temp-buffer - (insert "X-Face: ") - (insert-file-contents "~/.xface") - (buffer-string))) + (with-temp-buffer + (insert "X-Face: ") + (insert-file-contents "~/.xface") + (buffer-string))) @end example @noindent diff --git a/etc/NEWS b/etc/NEWS index d796a4cdbc..6e2b92fed2 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -1735,6 +1735,13 @@ modifies the string's text properties; instead, it uses and returns a copy of the string. This helps avoid trouble when strings are shared or constants. ++++ +** Temporary buffers no longer run certain buffer hooks. +The macros 'with-temp-buffer' and 'with-temp-file' no longer run the +hooks 'kill-buffer-hook', 'kill-buffer-query-functions', and +'buffer-list-update-hook' for the temporary buffers they create. This +avoids slowing them down when a lot of these hooks are defined. + --- ** The obsolete function 'thread-alive-p' has been removed. @@ -2077,6 +2084,15 @@ and 'play-sound-file'. If this variable is non-nil, character syntax is used for printing numbers when this makes sense, such as '?A' for 65. ++++ +** Buffers can now be created with certain hooks disabled. +The functions 'get-buffer-create' and 'generate-new-buffer' accept a +new optional argument 'inhibit-buffer-hooks'. If non-nil, the new +buffer does not run the hooks 'kill-buffer-hook', +'kill-buffer-query-functions', and 'buffer-list-update-hook'. This +avoids slowing down internal or temporary buffers that are never +presented to users or passed on to other applications. + \f * Changes in Emacs 28.1 on Non-Free Operating Systems diff --git a/lisp/files.el b/lisp/files.el index 2cf77d5d7e..97316f0f1e 100644 --- a/lisp/files.el +++ b/lisp/files.el @@ -1850,6 +1850,10 @@ kill-buffer-hook The buffer being killed is current while the hook is running. See `kill-buffer'. +This hook is not run for internal or temporary buffers created by +`get-buffer-create' or `generate-new-buffer' with argument +INHIBIT-BUFFER-HOOKS non-nil. + Note: Be careful with let-binding this hook considering it is frequently used for cleanup.") @@ -1951,7 +1955,7 @@ create-file-buffer (let ((lastname (file-name-nondirectory filename))) (if (string= lastname "") (setq lastname filename)) - (generate-new-buffer (if (string-match-p "\\` " lastname) + (generate-new-buffer (if (string-prefix-p " " lastname) (concat "|" lastname) lastname)))) diff --git a/lisp/gnus/gnus-uu.el b/lisp/gnus/gnus-uu.el index 70aeac00d7..00ca430cc5 100644 --- a/lisp/gnus/gnus-uu.el +++ b/lisp/gnus/gnus-uu.el @@ -1587,8 +1587,7 @@ gnus-uu-unshar-article (save-excursion (switch-to-buffer (current-buffer)) (delete-other-windows) - (let ((buffer (get-buffer-create (generate-new-buffer-name - "*Warning*")))) + (let ((buffer (generate-new-buffer "*Warning*"))) (unless (unwind-protect (with-current-buffer buffer diff --git a/lisp/international/mule.el b/lisp/international/mule.el index 212e7232b4..6571454dff 100644 --- a/lisp/international/mule.el +++ b/lisp/international/mule.el @@ -307,12 +307,9 @@ load-with-code-conversion (and (null noerror) (signal 'file-error (list "Cannot open load file" file))) ;; Read file with code conversion, and then eval. - (let* ((buffer - ;; We can't use `generate-new-buffer' because files.el - ;; is not yet loaded. - (get-buffer-create (generate-new-buffer-name " *load*"))) - (load-in-progress t) - (source (save-match-data (string-match "\\.el\\'" fullname)))) + (let ((buffer (generate-new-buffer " *load*")) + (load-in-progress t) + (source (string-suffix-p ".el" fullname))) (unless nomessage (if source (message "Loading %s (source)..." file) diff --git a/lisp/mh-e/mh-xface.el b/lisp/mh-e/mh-xface.el index 909f1fe95d..65039310e7 100644 --- a/lisp/mh-e/mh-xface.el +++ b/lisp/mh-e/mh-xface.el @@ -425,8 +425,7 @@ mh-x-image-url-fetch-image be displayed in a buffer and position specified by MARKER. The actual display is carried out by the SENTINEL function." (if mh-wget-executable - (let ((buffer (get-buffer-create (generate-new-buffer-name - mh-temp-fetch-buffer))) + (let ((buffer (generate-new-buffer mh-temp-fetch-buffer)) (filename (or (mh-funcall-if-exists make-temp-file "mhe-fetch") (expand-file-name (make-temp-name "~/mhe-fetch"))))) (with-current-buffer buffer diff --git a/lisp/net/imap.el b/lisp/net/imap.el index 0394f0efea..50b08d9612 100644 --- a/lisp/net/imap.el +++ b/lisp/net/imap.el @@ -1033,8 +1033,7 @@ imap-open (when (funcall (nth 1 (assq stream imap-stream-alist)) buffer) ;; Stream changed? (if (not (eq imap-default-stream stream)) - (with-current-buffer (get-buffer-create - (generate-new-buffer-name " *temp*")) + (with-current-buffer (generate-new-buffer " *temp*") (mapc 'make-local-variable imap-local-variables) (set-buffer-multibyte nil) (buffer-disable-undo) diff --git a/lisp/net/mailcap.el b/lisp/net/mailcap.el index d0f8c1272d..bc99f02fe3 100644 --- a/lisp/net/mailcap.el +++ b/lisp/net/mailcap.el @@ -386,8 +386,7 @@ mailcap-maybe-eval (when (save-window-excursion (delete-other-windows) - (let ((buffer (get-buffer-create (generate-new-buffer-name - "*Warning*")))) + (let ((buffer (generate-new-buffer "*Warning*"))) (unwind-protect (with-current-buffer buffer (insert (substitute-command-keys diff --git a/lisp/progmodes/flymake-proc.el b/lisp/progmodes/flymake-proc.el index 152dc725c7..e9d5018f30 100644 --- a/lisp/progmodes/flymake-proc.el +++ b/lisp/progmodes/flymake-proc.el @@ -429,16 +429,15 @@ flymake-proc--replace-region (defun flymake-proc--read-file-to-temp-buffer (file-name) "Insert contents of FILE-NAME into newly created temp buffer." - (let* ((temp-buffer (get-buffer-create (generate-new-buffer-name (concat "flymake:" (file-name-nondirectory file-name)))))) - (with-current-buffer temp-buffer - (insert-file-contents file-name)) - temp-buffer)) + (with-current-buffer (generate-new-buffer + (concat "flymake:" (file-name-nondirectory file-name))) + (insert-file-contents file-name) + (current-buffer))) (defun flymake-proc--copy-buffer-to-temp-buffer (buffer) "Copy contents of BUFFER into newly created temp buffer." - (with-current-buffer - (get-buffer-create (generate-new-buffer-name - (concat "flymake:" (buffer-name buffer)))) + (with-current-buffer (generate-new-buffer + (concat "flymake:" (buffer-name buffer))) (insert-buffer-substring buffer) (current-buffer))) diff --git a/lisp/simple.el b/lisp/simple.el index 3c4f756230..7781a1e91a 100644 --- a/lisp/simple.el +++ b/lisp/simple.el @@ -4301,8 +4301,7 @@ shell-command-on-region (defun shell-command-to-string (command) "Execute shell command COMMAND and return its output as a string." (with-output-to-string - (with-current-buffer - standard-output + (with-current-buffer standard-output (shell-command command t)))) (defun process-file (program &optional infile buffer display &rest args) diff --git a/lisp/subr.el b/lisp/subr.el index c28807f694..0daceb6869 100644 --- a/lisp/subr.el +++ b/lisp/subr.el @@ -3692,10 +3692,11 @@ internal--after-with-selected-window (when (window-live-p (nth 1 state)) (select-window (nth 1 state) 'norecord))) -(defun generate-new-buffer (name) +(defun generate-new-buffer (name &optional inhibit-buffer-hooks) "Create and return a buffer with a name based on NAME. -Choose the buffer's name using `generate-new-buffer-name'." - (get-buffer-create (generate-new-buffer-name name))) +Choose the buffer's name using `generate-new-buffer-name'. +See `get-buffer-create' for the meaning of INHIBIT-BUFFER-HOOKS." + (get-buffer-create (generate-new-buffer-name name) inhibit-buffer-hooks)) (defmacro with-selected-window (window &rest body) "Execute the forms in BODY with WINDOW as the selected window. @@ -3854,23 +3855,29 @@ with-output-to-temp-buffer (prog1 (progn ,@body) (internal-temp-output-buffer-show ,buf))))) +(defmacro with-temp-buffer (&rest body) + "Create a temporary buffer, and evaluate BODY there like `progn'. +The buffer does not run the hooks `kill-buffer-hook', +`kill-buffer-query-functions', and `buffer-list-update-hook'. +See also `with-temp-file' and `with-output-to-string'." + (declare (indent 0) (debug t)) + (let ((temp-buffer (make-symbol "temp-buffer"))) + `(let ((,temp-buffer (generate-new-buffer " *temp*" t))) + ;; `kill-buffer' can change current-buffer in some odd cases. + (with-current-buffer ,temp-buffer + (unwind-protect + (progn ,@body) + (and (buffer-name ,temp-buffer) + (kill-buffer ,temp-buffer))))))) + (defmacro with-temp-file (file &rest body) "Create a new buffer, evaluate BODY there, and write the buffer to FILE. The value returned is the value of the last form in BODY. -See also `with-temp-buffer'." +The buffer is created using `with-temp-buffer', which see." (declare (indent 1) (debug t)) - (let ((temp-file (make-symbol "temp-file")) - (temp-buffer (make-symbol "temp-buffer"))) - `(let ((,temp-file ,file) - (,temp-buffer (generate-new-buffer " *temp file*"))) - (unwind-protect - (prog1 - (with-current-buffer ,temp-buffer - ,@body) - (with-current-buffer ,temp-buffer - (write-region nil nil ,temp-file nil 0))) - (and (buffer-name ,temp-buffer) - (kill-buffer ,temp-buffer)))))) + `(with-temp-buffer + (prog1 (progn ,@body) + (write-region nil nil ,file nil 0)))) (defmacro with-temp-message (message &rest body) "Display MESSAGE temporarily if non-nil while BODY is evaluated. @@ -3895,19 +3902,6 @@ with-temp-message (message "%s" ,current-message) (message nil))))))) -(defmacro with-temp-buffer (&rest body) - "Create a temporary buffer, and evaluate BODY there like `progn'. -See also `with-temp-file' and `with-output-to-string'." - (declare (indent 0) (debug t)) - (let ((temp-buffer (make-symbol "temp-buffer"))) - `(let ((,temp-buffer (generate-new-buffer " *temp*"))) - ;; `kill-buffer' can change current-buffer in some odd cases. - (with-current-buffer ,temp-buffer - (unwind-protect - (progn ,@body) - (and (buffer-name ,temp-buffer) - (kill-buffer ,temp-buffer))))))) - (defmacro with-silent-modifications (&rest body) "Execute BODY, pretending it does not modify the buffer. This macro is typically used around modifications of @@ -3935,7 +3929,7 @@ with-silent-modifications (defmacro with-output-to-string (&rest body) "Execute BODY, return the text it sent to `standard-output', as a string." (declare (indent 0) (debug t)) - `(let ((standard-output (generate-new-buffer " *string-output*"))) + `(let ((standard-output (generate-new-buffer " *string-output*" t))) (unwind-protect (progn (let ((standard-output standard-output)) diff --git a/src/buffer.c b/src/buffer.c index 4215acbf1d..926fbc8f09 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -37,7 +37,6 @@ Copyright (C) 1985-1989, 1993-1995, 1997-2020 Free Software Foundation, #include "window.h" #include "commands.h" #include "character.h" -#include "coding.h" #include "buffer.h" #include "region-cache.h" #include "indent.h" @@ -514,16 +513,31 @@ get_truename_buffer (register Lisp_Object filename) return Qnil; } -DEFUN ("get-buffer-create", Fget_buffer_create, Sget_buffer_create, 1, 1, 0, +/* Run 'buffer-list-update-hook' if Vrun_hooks is non-nil, and BUF is + either NULL or does not have buffer hooks inhibited. */ + +static void +run_buffer_list_update_hook (struct buffer *buf) { + if (! (NILP (Vrun_hooks) || (buf && buf->inhibit_buffer_hooks))) + call1 (Vrun_hooks, Qbuffer_list_update_hook); +} + +DEFUN ("get-buffer-create", Fget_buffer_create, Sget_buffer_create, 1, 2, 0, doc: /* Return the buffer specified by BUFFER-OR-NAME, creating a new one if needed. If BUFFER-OR-NAME is a string and a live buffer with that name exists, return that buffer. If no such buffer exists, create a new buffer with -that name and return it. If BUFFER-OR-NAME starts with a space, the new -buffer does not keep undo information. +that name and return it. + +If BUFFER-OR-NAME starts with a space, the new buffer does not keep undo +information. If optional argument INHIBIT-BUFFER-HOOKS is non-nil, the +new buffer does not run the hooks `kill-buffer-hook', +`kill-buffer-query-functions', and `buffer-list-update-hook'. This +avoids slowing down internal or temporary buffers that are never +presented to users or passed on to other applications. If BUFFER-OR-NAME is a buffer instead of a string, return it as given, even if it is dead. The return value is never nil. */) - (register Lisp_Object buffer_or_name) + (register Lisp_Object buffer_or_name, Lisp_Object inhibit_buffer_hooks) { register Lisp_Object buffer, name; register struct buffer *b; @@ -598,11 +612,7 @@ DEFUN ("get-buffer-create", Fget_buffer_create, Sget_buffer_create, 1, 1, 0, set_string_intervals (name, NULL); bset_name (b, name); - b->inhibit_buffer_hooks - = (STRINGP (Vcode_conversion_workbuf_name) - && strncmp (SSDATA (name), SSDATA (Vcode_conversion_workbuf_name), - SBYTES (Vcode_conversion_workbuf_name)) == 0); - + b->inhibit_buffer_hooks = !NILP (inhibit_buffer_hooks); bset_undo_list (b, SREF (name, 0) != ' ' ? Qnil : Qt); reset_buffer (b); @@ -614,9 +624,8 @@ DEFUN ("get-buffer-create", Fget_buffer_create, Sget_buffer_create, 1, 1, 0, /* Put this in the alist of all live buffers. */ XSETBUFFER (buffer, b); Vbuffer_alist = nconc2 (Vbuffer_alist, list1 (Fcons (name, buffer))); - /* And run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !b->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + + run_buffer_list_update_hook (b); return buffer; } @@ -890,9 +899,7 @@ DEFUN ("make-indirect-buffer", Fmake_indirect_buffer, Smake_indirect_buffer, set_buffer_internal_1 (old_b); } - /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks)) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (NULL); return buf; } @@ -1536,9 +1543,7 @@ DEFUN ("rename-buffer", Frename_buffer, Srename_buffer, 1, 2, && !NILP (BVAR (current_buffer, auto_save_file_name))) call0 (intern ("rename-auto-save-file")); - /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !current_buffer->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (current_buffer); /* Refetch since that last call may have done GC. */ return BVAR (current_buffer, name); @@ -1612,7 +1617,7 @@ DEFUN ("other-buffer", Fother_buffer, Sother_buffer, 0, 3, 0, buf = Fget_buffer (scratch); if (NILP (buf)) { - buf = Fget_buffer_create (scratch); + buf = Fget_buffer_create (scratch, Qnil); Fset_buffer_major_mode (buf); } return buf; @@ -1636,7 +1641,7 @@ other_buffer_safely (Lisp_Object buffer) buf = Fget_buffer (scratch); if (NILP (buf)) { - buf = Fget_buffer_create (scratch); + buf = Fget_buffer_create (scratch, Qnil); Fset_buffer_major_mode (buf); } @@ -1713,7 +1718,9 @@ DEFUN ("kill-buffer", Fkill_buffer, Skill_buffer, 0, 1, "bKill buffer: ", the buffer is not killed. The hook `kill-buffer-hook' is run before the buffer is actually killed. The buffer being killed will be current while the hook is running. Functions called by any of these hooks are -supposed to not change the current buffer. +supposed to not change the current buffer. Neither hook is run for +internal or temporary buffers created by `get-buffer-create' or +`generate-new-buffer' with argument INHIBIT-BUFFER-HOOKS non-nil. Any processes that have this buffer as the `process-buffer' are killed with SIGHUP. This function calls `replace-buffer-in-windows' for @@ -1973,9 +1980,7 @@ DEFUN ("kill-buffer", Fkill_buffer, Skill_buffer, 0, 1, "bKill buffer: ", bset_width_table (b, Qnil); unblock_input (); - /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !b->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (b); return Qt; } @@ -2015,9 +2020,7 @@ record_buffer (Lisp_Object buffer) fset_buffer_list (f, Fcons (buffer, Fdelq (buffer, f->buffer_list))); fset_buried_buffer_list (f, Fdelq (buffer, f->buried_buffer_list)); - /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !XBUFFER (buffer)->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (XBUFFER (buffer)); } @@ -2054,9 +2057,7 @@ DEFUN ("bury-buffer-internal", Fbury_buffer_internal, Sbury_buffer_internal, fset_buried_buffer_list (f, Fcons (buffer, Fdelq (buffer, f->buried_buffer_list))); - /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !XBUFFER (buffer)->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (XBUFFER (buffer)); return Qnil; } @@ -5349,10 +5350,11 @@ init_buffer_once (void) Fput (Qkill_buffer_hook, Qpermanent_local, Qt); /* Super-magic invisible buffer. */ - Vprin1_to_string_buffer = Fget_buffer_create (build_pure_c_string (" prin1")); + Vprin1_to_string_buffer = + Fget_buffer_create (build_pure_c_string (" prin1"), Qt); Vbuffer_alist = Qnil; - Fset_buffer (Fget_buffer_create (build_pure_c_string ("*scratch*"))); + Fset_buffer (Fget_buffer_create (build_pure_c_string ("*scratch*"), Qnil)); inhibit_modification_hooks = 0; } @@ -5397,7 +5399,7 @@ init_buffer (void) #endif /* USE_MMAP_FOR_BUFFERS */ AUTO_STRING (scratch, "*scratch*"); - Fset_buffer (Fget_buffer_create (scratch)); + Fset_buffer (Fget_buffer_create (scratch, Qnil)); if (NILP (BVAR (&buffer_defaults, enable_multibyte_characters))) Fset_buffer_multibyte (Qnil); @@ -6300,9 +6302,14 @@ from (abs POSITION). If POSITION is positive, point was at the front DEFVAR_LISP ("kill-buffer-query-functions", Vkill_buffer_query_functions, doc: /* List of functions called with no args to query before killing a buffer. The buffer being killed will be current while the functions are running. +See `kill-buffer'. If any of them returns nil, the buffer is not killed. Functions run by -this hook are supposed to not change the current buffer. */); +this hook are supposed to not change the current buffer. + +This hook is not run for internal or temporary buffers created by +`get-buffer-create' or `generate-new-buffer' with argument +INHIBIT-BUFFER-HOOKS non-nil. */); Vkill_buffer_query_functions = Qnil; DEFVAR_LISP ("change-major-mode-hook", Vchange_major_mode_hook, @@ -6315,9 +6322,12 @@ from (abs POSITION). If POSITION is positive, point was at the front doc: /* Hook run when the buffer list changes. Functions (implicitly) running this hook are `get-buffer-create', `make-indirect-buffer', `rename-buffer', `kill-buffer', `bury-buffer' -and `select-window'. Functions run by this hook should avoid calling -`select-window' with a nil NORECORD argument or `with-temp-buffer' -since either may lead to infinite recursion. */); +and `select-window'. This hook is not run for internal or temporary +buffers created by `get-buffer-create' or `generate-new-buffer' with +argument INHIBIT-BUFFER-HOOKS non-nil. + +Functions run by this hook should avoid calling `select-window' with a +nil NORECORD argument since it may lead to infinite recursion. */); Vbuffer_list_update_hook = Qnil; DEFSYM (Qbuffer_list_update_hook, "buffer-list-update-hook"); diff --git a/src/buffer.h b/src/buffer.h index fe549c5dac..b8c5162be4 100644 --- a/src/buffer.h +++ b/src/buffer.h @@ -663,11 +663,11 @@ #define BVAR(buf, field) ((buf)->field ## _) /* Non-zero whenever the narrowing is changed in this buffer. */ bool_bf clip_changed : 1; - /* Non-zero for internally used temporary buffers that don't need to - run hooks kill-buffer-hook, buffer-list-update-hook, and - kill-buffer-query-functions. This is used in coding.c to avoid - slowing down en/decoding when there are a lot of these hooks - defined. */ + /* Non-zero for internal or temporary buffers that don't need to + run hooks kill-buffer-hook, kill-buffer-query-functions, and + buffer-list-update-hook. This is used in coding.c to avoid + slowing down en/decoding when a lot of these hooks are + defined, as well as by with-temp-buffer, for example. */ bool_bf inhibit_buffer_hooks : 1; /* List of overlays that end at or before the current center, diff --git a/src/callproc.c b/src/callproc.c index e3346e2eab..4bca1e5ebd 100644 --- a/src/callproc.c +++ b/src/callproc.c @@ -405,9 +405,8 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd, if (! (NILP (buffer) || EQ (buffer, Qt) || FIXNUMP (buffer))) { - Lisp_Object spec_buffer; - spec_buffer = buffer; - buffer = Fget_buffer_create (buffer); + Lisp_Object spec_buffer = buffer; + buffer = Fget_buffer_create (buffer, Qnil); /* Mention the buffer name for a better error message. */ if (NILP (buffer)) CHECK_BUFFER (spec_buffer); diff --git a/src/coding.c b/src/coding.c index 2142e7fa51..1afa4aa474 100644 --- a/src/coding.c +++ b/src/coding.c @@ -7821,7 +7821,7 @@ encode_coding (struct coding_system *coding) /* A string that serves as name of the reusable work buffer, and as base name of temporary work buffers used for code-conversion operations. */ -Lisp_Object Vcode_conversion_workbuf_name; +static Lisp_Object Vcode_conversion_workbuf_name; /* The reusable working buffer, created once and never killed. */ static Lisp_Object Vcode_conversion_reused_workbuf; @@ -7839,7 +7839,7 @@ code_conversion_restore (Lisp_Object arg) if (! NILP (workbuf)) { if (EQ (workbuf, Vcode_conversion_reused_workbuf)) - reused_workbuf_in_use = 0; + reused_workbuf_in_use = false; else Fkill_buffer (workbuf); } @@ -7857,13 +7857,13 @@ code_conversion_save (bool with_work_buf, bool multibyte) { Lisp_Object name = Fgenerate_new_buffer_name (Vcode_conversion_workbuf_name, Qnil); - workbuf = Fget_buffer_create (name); + workbuf = Fget_buffer_create (name, Qt); } else { if (NILP (Fbuffer_live_p (Vcode_conversion_reused_workbuf))) Vcode_conversion_reused_workbuf - = Fget_buffer_create (Vcode_conversion_workbuf_name); + = Fget_buffer_create (Vcode_conversion_workbuf_name, Qt); workbuf = Vcode_conversion_reused_workbuf; } } @@ -7881,7 +7881,7 @@ code_conversion_save (bool with_work_buf, bool multibyte) bset_undo_list (current_buffer, Qt); bset_enable_multibyte_characters (current_buffer, multibyte ? Qt : Qnil); if (EQ (workbuf, Vcode_conversion_reused_workbuf)) - reused_workbuf_in_use = 1; + reused_workbuf_in_use = true; set_buffer_internal (current); } @@ -11639,7 +11639,7 @@ syms_of_coding (void) staticpro (&Vcode_conversion_workbuf_name); Vcode_conversion_workbuf_name = build_pure_c_string (" *code-conversion-work*"); - reused_workbuf_in_use = 0; + reused_workbuf_in_use = false; PDUMPER_REMEMBER_SCALAR (reused_workbuf_in_use); DEFSYM (Qcharset, "charset"); diff --git a/src/coding.h b/src/coding.h index 4973cf89eb..9ad1e954f8 100644 --- a/src/coding.h +++ b/src/coding.h @@ -97,9 +97,6 @@ #define EMACS_CODING_H extern Lisp_Object Vcoding_system_hash_table; -/* Name (or base name) of work buffer for code conversion. */ -extern Lisp_Object Vcode_conversion_workbuf_name; - /* Enumeration of index to an attribute vector of a coding system. */ enum coding_attr_index diff --git a/src/fileio.c b/src/fileio.c index 283813ff89..c8d2e923a0 100644 --- a/src/fileio.c +++ b/src/fileio.c @@ -4003,7 +4003,7 @@ because (1) it preserves some marker positions and (2) it puts less data record_unwind_current_buffer (); - workbuf = Fget_buffer_create (name); + workbuf = Fget_buffer_create (name, Qt); buf = XBUFFER (workbuf); delete_all_overlays (buf); diff --git a/src/minibuf.c b/src/minibuf.c index fc3fd92a88..1940564a80 100644 --- a/src/minibuf.c +++ b/src/minibuf.c @@ -809,7 +809,7 @@ get_minibuffer (EMACS_INT depth) static char const name_fmt[] = " *Minibuf-%"pI"d*"; char name[sizeof name_fmt + INT_STRLEN_BOUND (EMACS_INT)]; AUTO_STRING_WITH_LEN (lname, name, sprintf (name, name_fmt, depth)); - buf = Fget_buffer_create (lname); + buf = Fget_buffer_create (lname, Qnil); /* Although the buffer's name starts with a space, undo should be enabled in it. */ diff --git a/src/print.c b/src/print.c index 008bf5e639..ec271d914c 100644 --- a/src/print.c +++ b/src/print.c @@ -562,7 +562,7 @@ temp_output_buffer_setup (const char *bufname) record_unwind_current_buffer (); - Fset_buffer (Fget_buffer_create (build_string (bufname))); + Fset_buffer (Fget_buffer_create (build_string (bufname), Qnil)); Fkill_all_local_variables (); delete_all_overlays (current_buffer); diff --git a/src/process.c b/src/process.c index bf64ead24e..6dc7d4b647 100644 --- a/src/process.c +++ b/src/process.c @@ -1731,7 +1731,7 @@ DEFUN ("make-process", Fmake_process, Smake_process, 0, MANY, 0, buffer = Fplist_get (contact, QCbuffer); if (!NILP (buffer)) - buffer = Fget_buffer_create (buffer); + buffer = Fget_buffer_create (buffer, Qnil); /* Make sure that the child will be able to chdir to the current buffer's current directory, or its unhandled equivalent. We @@ -1768,7 +1768,7 @@ DEFUN ("make-process", Fmake_process, Smake_process, 0, MANY, 0, QCname, concat2 (name, build_string (" stderr")), QCbuffer, - Fget_buffer_create (xstderr), + Fget_buffer_create (xstderr, Qnil), QCnoquery, query_on_exit ? Qnil : Qt); } @@ -2443,7 +2443,7 @@ DEFUN ("make-pipe-process", Fmake_pipe_process, Smake_pipe_process, buffer = Fplist_get (contact, QCbuffer); if (NILP (buffer)) buffer = name; - buffer = Fget_buffer_create (buffer); + buffer = Fget_buffer_create (buffer, Qnil); pset_buffer (p, buffer); pset_childp (p, contact); @@ -3173,7 +3173,7 @@ DEFUN ("make-serial-process", Fmake_serial_process, Smake_serial_process, buffer = Fplist_get (contact, QCbuffer); if (NILP (buffer)) buffer = name; - buffer = Fget_buffer_create (buffer); + buffer = Fget_buffer_create (buffer, Qnil); pset_buffer (p, buffer); pset_childp (p, contact); @@ -4188,7 +4188,7 @@ DEFUN ("make-network-process", Fmake_network_process, Smake_network_process, open_socket: if (!NILP (buffer)) - buffer = Fget_buffer_create (buffer); + buffer = Fget_buffer_create (buffer, Qnil); /* Unwind bind_polling_period. */ unbind_to (count, Qnil); @@ -4961,7 +4961,7 @@ server_accept_connection (Lisp_Object server, int channel) if (!NILP (buffer)) { args[1] = buffer; - buffer = Fget_buffer_create (Fformat (nargs, args)); + buffer = Fget_buffer_create (Fformat (nargs, args), Qnil); } } diff --git a/src/w32fns.c b/src/w32fns.c index a840f0e122..36bee0676b 100644 --- a/src/w32fns.c +++ b/src/w32fns.c @@ -7372,7 +7372,7 @@ DEFUN ("x-show-tip", Fx_show_tip, Sx_show_tip, 1, 6, 0, tip_f = XFRAME (tip_frame); window = FRAME_ROOT_WINDOW (tip_f); - tip_buf = Fget_buffer_create (tip); + tip_buf = Fget_buffer_create (tip, Qnil); /* We will mark the tip window a "pseudo-window" below, and such windows cannot have display margins. */ bset_left_margin_cols (XBUFFER (tip_buf), make_fixnum (0)); diff --git a/src/window.c b/src/window.c index 6cd3122b43..4dd11eb857 100644 --- a/src/window.c +++ b/src/window.c @@ -617,11 +617,12 @@ DEFUN ("select-window", Fselect_window, Sselect_window, 1, 2, 0, Run `buffer-list-update-hook' unless NORECORD is non-nil. Note that applications and internal routines often select a window temporarily for various purposes; mostly, to simplify coding. As a rule, such -selections should be not recorded and therefore will not pollute +selections should not be recorded and therefore will not pollute `buffer-list-update-hook'. Selections that "really count" are those causing a visible change in the next redisplay of WINDOW's frame and -should be always recorded. So if you think of running a function each -time a window gets selected put it on `buffer-list-update-hook'. +should always be recorded. So if you think of running a function each +time a window gets selected, put it on `buffer-list-update-hook' or +`window-selection-change-functions'. Also note that the main editor command loop sets the current buffer to the buffer of the selected window before each command. */) diff --git a/src/xdisp.c b/src/xdisp.c index ed1d4761b9..22d33f4a1f 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -10853,7 +10853,7 @@ message_dolog (const char *m, ptrdiff_t nbytes, bool nlflag, bool multibyte) /* Ensure the Messages buffer exists, and switch to it. If we created it, set the major-mode. */ bool newbuffer = NILP (Fget_buffer (Vmessages_buffer_name)); - Fset_buffer (Fget_buffer_create (Vmessages_buffer_name)); + Fset_buffer (Fget_buffer_create (Vmessages_buffer_name, Qnil)); if (newbuffer && !NILP (Ffboundp (intern ("messages-buffer-mode")))) call0 (intern ("messages-buffer-mode")); @@ -11339,7 +11339,7 @@ ensure_echo_area_buffers (void) static char const name_fmt[] = " *Echo Area %d*"; char name[sizeof name_fmt + INT_STRLEN_BOUND (int)]; AUTO_STRING_WITH_LEN (lname, name, sprintf (name, name_fmt, i)); - echo_buffer[i] = Fget_buffer_create (lname); + echo_buffer[i] = Fget_buffer_create (lname, Qnil); bset_truncate_lines (XBUFFER (echo_buffer[i]), Qnil); /* to force word wrap in echo area - it was decided to postpone this*/ diff --git a/src/xfns.c b/src/xfns.c index 46e4bd73a6..abe293e903 100644 --- a/src/xfns.c +++ b/src/xfns.c @@ -7041,7 +7041,7 @@ DEFUN ("x-show-tip", Fx_show_tip, Sx_show_tip, 1, 6, 0, tip_f = XFRAME (tip_frame); window = FRAME_ROOT_WINDOW (tip_f); - tip_buf = Fget_buffer_create (tip); + tip_buf = Fget_buffer_create (tip, Qnil); /* We will mark the tip window a "pseudo-window" below, and such windows cannot have display margins. */ bset_left_margin_cols (XBUFFER (tip_buf), make_fixnum (0)); diff --git a/src/xwidget.c b/src/xwidget.c index e078a28a35..accde65b52 100644 --- a/src/xwidget.c +++ b/src/xwidget.c @@ -100,7 +100,8 @@ DEFUN ("make-xwidget", Lisp_Object val; xw->type = type; xw->title = title; - xw->buffer = NILP (buffer) ? Fcurrent_buffer () : Fget_buffer_create (buffer); + xw->buffer = (NILP (buffer) ? Fcurrent_buffer () + : Fget_buffer_create (buffer, Qnil)); xw->height = XFIXNAT (height); xw->width = XFIXNAT (width); xw->kill_without_query = false; diff --git a/test/src/buffer-tests.el b/test/src/buffer-tests.el index 0db66f9751..dd8927457a 100644 --- a/test/src/buffer-tests.el +++ b/test/src/buffer-tests.el @@ -19,9 +19,7 @@ ;;; Code: -(require 'ert) -(require 'seq) -(eval-when-compile (require 'cl-lib)) +(require 'cl-lib) (ert-deftest overlay-modification-hooks-message-other-buf () "Test for bug#21824. @@ -1334,4 +1332,33 @@ buffer-tests-buffer-local-variables-undo (with-temp-buffer (should (assq 'buffer-undo-list (buffer-local-variables))))) +(ert-deftest buffer-tests-inhibit-buffer-hooks () + "Test `get-buffer-create' argument INHIBIT-BUFFER-HOOKS." + (let* (run-bluh (bluh (lambda () (setq run-bluh t)))) + (unwind-protect + (let* ( run-kbh (kbh (lambda () (setq run-kbh t))) + run-kbqf (kbqf (lambda () (setq run-kbqf t))) ) + + ;; Inhibited. + (add-hook 'buffer-list-update-hook bluh) + (with-current-buffer (generate-new-buffer " foo" t) + (add-hook 'kill-buffer-hook kbh nil t) + (add-hook 'kill-buffer-query-functions kbqf nil t) + (kill-buffer)) + (with-temp-buffer) + (with-output-to-string) + (should-not run-bluh) + (should-not run-kbh) + (should-not run-kbqf) + + ;; Not inhibited. + (with-current-buffer (generate-new-buffer " foo") + (should run-bluh) + (add-hook 'kill-buffer-hook kbh nil t) + (add-hook 'kill-buffer-query-functions kbqf nil t) + (kill-buffer)) + (should run-kbh) + (should run-kbqf)) + (remove-hook 'buffer-list-update-hook bluh)))) + ;;; buffer-tests.el ends here -- 2.29.2 ^ permalink raw reply related [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-12-07 22:16 ` Basil L. Contovounesios @ 2020-12-07 22:37 ` Basil L. Contovounesios 2020-12-08 8:09 ` martin rudalics 2020-12-18 14:57 ` Basil L. Contovounesios 2020-12-18 15:36 ` Stefan Monnier 2 siblings, 1 reply; 72+ messages in thread From: Basil L. Contovounesios @ 2020-12-07 22:37 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 34765, larsi, monnier, alexanderm "Basil L. Contovounesios" <contovob@tcd.ie> writes: > +/* Run 'buffer-list-update-hook' if Vrun_hooks is non-nil, and BUF is > + either NULL or does not have buffer hooks inhibited. */ > + > +static void > +run_buffer_list_update_hook (struct buffer *buf) { ^^ Oops, will fix style locally. > + if (! (NILP (Vrun_hooks) || (buf && buf->inhibit_buffer_hooks))) > + call1 (Vrun_hooks, Qbuffer_list_update_hook); > +} -- Basil ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-12-07 22:37 ` Basil L. Contovounesios @ 2020-12-08 8:09 ` martin rudalics 2020-12-14 21:03 ` Basil L. Contovounesios 0 siblings, 1 reply; 72+ messages in thread From: martin rudalics @ 2020-12-08 8:09 UTC (permalink / raw) To: Basil L. Contovounesios, Eli Zaretskii; +Cc: 34765, alexanderm, larsi, monnier >> +run_buffer_list_update_hook (struct buffer *buf) { > ^^ > Oops, will fix style locally. Incidentally, that was the only thing I found. The rest looks good to me. Thank you, martin ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-12-08 8:09 ` martin rudalics @ 2020-12-14 21:03 ` Basil L. Contovounesios 2020-12-15 16:03 ` Eli Zaretskii 0 siblings, 1 reply; 72+ messages in thread From: Basil L. Contovounesios @ 2020-12-14 21:03 UTC (permalink / raw) To: martin rudalics; +Cc: 34765, alexanderm, larsi, monnier martin rudalics <rudalics@gmx.at> writes: >>> +run_buffer_list_update_hook (struct buffer *buf) { >> ^^ >> Oops, will fix style locally. > > Incidentally, that was the only thing I found. The rest looks good to > me. Thanks. I'll give it a few more days in case Eli, Lars, or anyone else has any comments/objections before pushing. -- Basil ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-12-14 21:03 ` Basil L. Contovounesios @ 2020-12-15 16:03 ` Eli Zaretskii 2020-12-15 16:24 ` Basil L. Contovounesios 0 siblings, 1 reply; 72+ messages in thread From: Eli Zaretskii @ 2020-12-15 16:03 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: 34765, larsi, monnier, alexanderm > From: "Basil L. Contovounesios" <contovob@tcd.ie> > Cc: Eli Zaretskii <eliz@gnu.org>, larsi@gnus.org, 34765@debbugs.gnu.org, > alexanderm@web.de, monnier@IRO.UMontreal.CA > Date: Mon, 14 Dec 2020 21:03:16 +0000 > > martin rudalics <rudalics@gmx.at> writes: > > >>> +run_buffer_list_update_hook (struct buffer *buf) { > >> ^^ > >> Oops, will fix style locally. > > > > Incidentally, that was the only thing I found. The rest looks good to > > me. > > Thanks. I'll give it a few more days in case Eli, Lars, or anyone else > has any comments/objections before pushing. I will review it soon. Sorry for the delay. ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-12-15 16:03 ` Eli Zaretskii @ 2020-12-15 16:24 ` Basil L. Contovounesios 0 siblings, 0 replies; 72+ messages in thread From: Basil L. Contovounesios @ 2020-12-15 16:24 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 34765, larsi, monnier, alexanderm Eli Zaretskii <eliz@gnu.org> writes: > I will review it soon. Sorry for the delay. No need to apologise, there's no rush on my end. Thanks, -- Basil ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-12-07 22:16 ` Basil L. Contovounesios 2020-12-07 22:37 ` Basil L. Contovounesios @ 2020-12-18 14:57 ` Basil L. Contovounesios 2020-12-18 15:36 ` Stefan Monnier 2 siblings, 0 replies; 72+ messages in thread From: Basil L. Contovounesios @ 2020-12-18 14:57 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 34765, larsi, monnier, alexanderm "Basil L. Contovounesios" <contovob@tcd.ie> writes: > (defmacro with-temp-file (file &rest body) > "Create a new buffer, evaluate BODY there, and write the buffer to FILE. > The value returned is the value of the last form in BODY. > -See also `with-temp-buffer'." > +The buffer is created using `with-temp-buffer', which see." > (declare (indent 1) (debug t)) > - (let ((temp-file (make-symbol "temp-file")) > - (temp-buffer (make-symbol "temp-buffer"))) > - `(let ((,temp-file ,file) > - (,temp-buffer (generate-new-buffer " *temp file*"))) > - (unwind-protect > - (prog1 > - (with-current-buffer ,temp-buffer > - ,@body) > - (with-current-buffer ,temp-buffer > - (write-region nil nil ,temp-file nil 0))) > - (and (buffer-name ,temp-buffer) > - (kill-buffer ,temp-buffer)))))) > + `(with-temp-buffer > + (prog1 (progn ,@body) > + (write-region nil nil ,file nil 0)))) Reading another thread made me realise that this changes the argument evaluation order, so I'll fix that too. -- Basil ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-12-07 22:16 ` Basil L. Contovounesios 2020-12-07 22:37 ` Basil L. Contovounesios 2020-12-18 14:57 ` Basil L. Contovounesios @ 2020-12-18 15:36 ` Stefan Monnier 2020-12-18 18:49 ` Basil L. Contovounesios 2 siblings, 1 reply; 72+ messages in thread From: Stefan Monnier @ 2020-12-18 15:36 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: 34765, larsi, alexanderm > (defmacro with-temp-file (file &rest body) > "Create a new buffer, evaluate BODY there, and write the buffer to FILE. > The value returned is the value of the last form in BODY. > -See also `with-temp-buffer'." > +The buffer is created using `with-temp-buffer', which see." > (declare (indent 1) (debug t)) > - (let ((temp-file (make-symbol "temp-file")) > - (temp-buffer (make-symbol "temp-buffer"))) > - `(let ((,temp-file ,file) > - (,temp-buffer (generate-new-buffer " *temp file*"))) > - (unwind-protect > - (prog1 > - (with-current-buffer ,temp-buffer > - ,@body) > - (with-current-buffer ,temp-buffer > - (write-region nil nil ,temp-file nil 0))) > - (and (buffer-name ,temp-buffer) > - (kill-buffer ,temp-buffer)))))) > + `(with-temp-buffer > + (prog1 (progn ,@body) > + (write-region nil nil ,file nil 0)))) You lost the `(with-current-buffer ,temp-buffer` around `write-region` so your new code will break if `body` doesn't preserve current-buffer. Stefan ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-12-18 15:36 ` Stefan Monnier @ 2020-12-18 18:49 ` Basil L. Contovounesios 2020-12-19 10:33 ` Eli Zaretskii 0 siblings, 1 reply; 72+ messages in thread From: Basil L. Contovounesios @ 2020-12-18 18:49 UTC (permalink / raw) To: Stefan Monnier; +Cc: 34765, larsi, alexanderm [-- Attachment #1: Type: text/plain, Size: 369 bytes --] Stefan Monnier <monnier@iro.umontreal.ca> writes: > You lost the `(with-current-buffer ,temp-buffer` around `write-region` > so your new code will break if `body` doesn't preserve current-buffer. Ugh, I thought the caller couldn't kill the buffer because it's later written, but I didn't think of excursions. I've now attached an updated patch. Thanks, -- Basil [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Inhibit-buffer-hooks-in-temporary-buffers.patch --] [-- Type: text/x-diff, Size: 45329 bytes --] From d2b2648efa51343a3b7dd84ada89110e84aa8658 Mon Sep 17 00:00:00 2001 From: "Basil L. Contovounesios" <contovob@tcd.ie> Date: Tue, 1 Dec 2020 01:12:32 +0000 Subject: [PATCH] Inhibit buffer hooks in temporary buffers Give get-buffer-create an optional argument to inhibit buffer hooks in internal or temporary buffers for efficiency (bug#34765). * .dir-locals.el (c-mode): Enforce existing indent-tabs-mode policy. * etc/NEWS: Announce new parameter of get-buffer-create and generate-new-buffer, and that with-temp-buffer and with-temp-file now inhibit buffer hooks. * doc/lispref/buffers.texi (Buffer Names): Fix typo. (Creating Buffers): Document new parameter of get-buffer-create and generate-new-buffer. (Buffer List, Killing Buffers): Document when buffer hooks are inhibited. (Current Buffer): * doc/lispref/files.texi (Writing to Files): Document that with-temp-buffer and with-temp-file inhibit buffer hooks. * doc/lispref/internals.texi (Buffer Internals): Document inhibit_buffer_hooks flag. Remove stale comment. * doc/misc/gnus-faq.texi (FAQ 5-8): * lisp/simple.el (shell-command-on-region): Fix indentation. * lisp/files.el (kill-buffer-hook): Document when hook is inhibited. (create-file-buffer): * lisp/gnus/gnus-uu.el (gnus-uu-unshar-article): * lisp/international/mule.el (load-with-code-conversion): * lisp/mh-e/mh-xface.el (mh-x-image-url-fetch-image): * lisp/net/imap.el (imap-open): * lisp/net/mailcap.el (mailcap-maybe-eval): * lisp/progmodes/flymake-proc.el (flymake-proc--read-file-to-temp-buffer) (flymake-proc--copy-buffer-to-temp-buffer): Simplify. * lisp/subr.el (generate-new-buffer): Forward new optional argument to inhibit buffer hooks to get-buffer-create. (with-temp-file, with-temp-buffer, with-output-to-string): * lisp/json.el (json-encode-string): Inhibit buffer hooks in buffer used. * src/buffer.c (run_buffer_list_update_hook): New helper function. (Fget_buffer_create): Use it. Add optional argument to set inhibit_buffer_hooks flag instead of comparing the buffer name to Vcode_conversion_workbuf_name. All callers changed. (Fmake_indirect_buffer, Frename_buffer, Fbury_buffer_internal) (record_buffer): Use run_buffer_list_update_hook. (Fkill_buffer): Document when buffer hooks are inhibited. Use run_buffer_list_update_hook. (init_buffer_once): Inhibit buffer hooks in Vprin1_to_string_buffer. (Vkill_buffer_query_functions, Vbuffer_list_update_hook): Document when hooks are inhibited. * src/buffer.h (struct buffer): Update inhibit_buffer_hooks commentary. * src/coding.h (Vcode_conversion_workbuf_name): * src/coding.c (Vcode_conversion_workbuf_name): Make static again since it is no longer needed in src/buffer.c. (code_conversion_restore, code_conversion_save, syms_of_coding): Prefer boolean over integer constants. * src/fileio.c (Finsert_file_contents): Inhibit buffer hooks in " *code-converting-work*" buffer. * src/window.c (Fselect_window): Fix grammar. Mention window-selection-change-functions alongside buffer-list-update-hook. * test/src/buffer-tests.el: Fix requires. (buffer-tests-inhibit-buffer-hooks): New test. --- .dir-locals.el | 1 + doc/lispref/buffers.texi | 55 +++++++++++++++------ doc/lispref/files.texi | 7 ++- doc/lispref/internals.texi | 11 ++++- doc/misc/gnus-faq.texi | 8 ++-- etc/NEWS | 16 +++++++ lisp/files.el | 6 ++- lisp/gnus/gnus-uu.el | 3 +- lisp/international/mule.el | 9 ++-- lisp/json.el | 2 +- lisp/mh-e/mh-xface.el | 3 +- lisp/net/imap.el | 3 +- lisp/net/mailcap.el | 3 +- lisp/progmodes/flymake-proc.el | 13 +++-- lisp/simple.el | 3 +- lisp/subr.el | 17 ++++--- src/buffer.c | 87 +++++++++++++++++++--------------- src/buffer.h | 10 ++-- src/callproc.c | 5 +- src/coding.c | 12 ++--- src/coding.h | 3 -- src/fileio.c | 2 +- src/minibuf.c | 2 +- src/print.c | 2 +- src/process.c | 12 ++--- src/w32fns.c | 2 +- src/window.c | 7 +-- src/xdisp.c | 4 +- src/xfns.c | 2 +- src/xwidget.c | 3 +- test/src/buffer-tests.el | 33 +++++++++++-- 31 files changed, 217 insertions(+), 129 deletions(-) diff --git a/.dir-locals.el b/.dir-locals.el index 27d50c6069..b313945936 100644 --- a/.dir-locals.el +++ b/.dir-locals.el @@ -9,6 +9,7 @@ (c-noise-macro-names . ("INLINE" "ATTRIBUTE_NO_SANITIZE_UNDEFINED" "UNINIT" "CALLBACK" "ALIGN_STACK")) (electric-quote-comment . nil) (electric-quote-string . nil) + (indent-tabs-mode . t) (mode . bug-reference-prog))) (objc-mode . ((c-file-style . "GNU") (electric-quote-comment . nil) diff --git a/doc/lispref/buffers.texi b/doc/lispref/buffers.texi index 2860343628..c8c4852e30 100644 --- a/doc/lispref/buffers.texi +++ b/doc/lispref/buffers.texi @@ -225,13 +225,18 @@ Current Buffer @defmac with-temp-buffer body@dots{} @anchor{Definition of with-temp-buffer} -The @code{with-temp-buffer} macro evaluates the @var{body} forms -with a temporary buffer as the current buffer. It saves the identity of +The @code{with-temp-buffer} macro evaluates the @var{body} forms with +a temporary buffer as the current buffer. It saves the identity of the current buffer, creates a temporary buffer and makes it current, evaluates the @var{body} forms, and finally restores the previous -current buffer while killing the temporary buffer. By default, undo -information (@pxref{Undo}) is not recorded in the buffer created by -this macro (but @var{body} can enable that, if needed). +current buffer while killing the temporary buffer. + +By default, undo information (@pxref{Undo}) is not recorded in the +buffer created by this macro (but @var{body} can enable that, if +needed). The temporary buffer also does not run the hooks +@code{kill-buffer-hook}, @code{kill-buffer-query-functions} +(@pxref{Killing Buffers}), and @code{buffer-list-update-hook} +(@pxref{Buffer List}). The return value is the value of the last form in @var{body}. You can return the contents of the temporary buffer by using @@ -345,9 +350,9 @@ Buffer Names If the optional second argument @var{ignore} is non-@code{nil}, it should be a string, a potential buffer name. It means to consider -that potential buffer acceptable, if it is tried, even it is the name -of an existing buffer (which would normally be rejected). Thus, if -buffers named @samp{foo}, @samp{foo<2>}, @samp{foo<3>} and +that potential buffer acceptable, if it is tried, even if it is the +name of an existing buffer (which would normally be rejected). Thus, +if buffers named @samp{foo}, @samp{foo<2>}, @samp{foo<3>} and @samp{foo<4>} exist, @example @@ -932,13 +937,17 @@ Buffer List @defvar buffer-list-update-hook This is a normal hook run whenever the buffer list changes. Functions (implicitly) running this hook are @code{get-buffer-create} -(@pxref{Creating Buffers}), @code{rename-buffer} (@pxref{Buffer Names}), -@code{kill-buffer} (@pxref{Killing Buffers}), @code{bury-buffer} (see -above) and @code{select-window} (@pxref{Selecting Windows}). +(@pxref{Creating Buffers}), @code{rename-buffer} (@pxref{Buffer +Names}), @code{kill-buffer} (@pxref{Killing Buffers}), +@code{bury-buffer} (see above), and @code{select-window} +(@pxref{Selecting Windows}). This hook is not run for internal or +temporary buffers created by @code{get-buffer-create} or +@code{generate-new-buffer} with a non-@code{nil} argument +@var{inhibit-buffer-hooks}. Functions run by this hook should avoid calling @code{select-window} -with a nil @var{norecord} argument or @code{with-temp-buffer} since -either may lead to infinite recursion. +with a @code{nil} @var{norecord} argument since this may lead to +infinite recursion. @end defvar @node Creating Buffers @@ -951,12 +960,20 @@ Creating Buffers with the specified name; @code{generate-new-buffer} always creates a new buffer and gives it a unique name. + Both functions accept an optional argument @var{inhibit-buffer-hooks}. +If it is non-@code{nil}, the buffer they create does not run the hooks +@code{kill-buffer-hook}, @code{kill-buffer-query-functions} +(@pxref{Killing Buffers}), and @code{buffer-list-update-hook} +(@pxref{Buffer List}). This avoids slowing down internal or temporary +buffers that are never presented to users or passed on to other +applications. + Other functions you can use to create buffers include @code{with-output-to-temp-buffer} (@pxref{Temporary Displays}) and @code{create-file-buffer} (@pxref{Visiting Files}). Starting a subprocess can also create a buffer (@pxref{Processes}). -@defun get-buffer-create buffer-or-name +@defun get-buffer-create buffer-or-name &optional inhibit-buffer-hooks This function returns a buffer named @var{buffer-or-name}. The buffer returned does not become the current buffer---this function does not change which buffer is current. @@ -980,7 +997,7 @@ Creating Buffers buffer initially disables undo information recording (@pxref{Undo}). @end defun -@defun generate-new-buffer name +@defun generate-new-buffer name &optional inhibit-buffer-hooks This function returns a newly created, empty buffer, but does not make it current. The name of the buffer is generated by passing @var{name} to the function @code{generate-new-buffer-name} (@pxref{Buffer @@ -1092,6 +1109,10 @@ Killing Buffers they are called. The idea of this feature is that these functions will ask for confirmation from the user. If any of them returns @code{nil}, @code{kill-buffer} spares the buffer's life. + +This hook is not run for internal or temporary buffers created by +@code{get-buffer-create} or @code{generate-new-buffer} with a +non-@code{nil} argument @var{inhibit-buffer-hooks}. @end defvar @defvar kill-buffer-hook @@ -1100,6 +1121,10 @@ Killing Buffers The buffer to be killed is current when the hook functions run. @xref{Hooks}. This variable is a permanent local, so its local binding is not cleared by changing major modes. + +This hook is not run for internal or temporary buffers created by +@code{get-buffer-create} or @code{generate-new-buffer} with a +non-@code{nil} argument @var{inhibit-buffer-hooks}. @end defvar @defopt buffer-offer-save diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi index d49ac42bb4..6949ca29c6 100644 --- a/doc/lispref/files.texi +++ b/doc/lispref/files.texi @@ -701,8 +701,11 @@ Writing to Files The current buffer is restored even in case of an abnormal exit via @code{throw} or error (@pxref{Nonlocal Exits}). -See also @code{with-temp-buffer} in @ref{Definition of -with-temp-buffer,, The Current Buffer}. +Like @code{with-temp-buffer} (@pxref{Definition of with-temp-buffer,, +Current Buffer}), the temporary buffer used by this macro does not run +the hooks @code{kill-buffer-hook}, @code{kill-buffer-query-functions} +(@pxref{Killing Buffers}), and @code{buffer-list-update-hook} +(@pxref{Buffer List}). @end defmac @node File Locks diff --git a/doc/lispref/internals.texi b/doc/lispref/internals.texi index 28a5fdb349..757368bc98 100644 --- a/doc/lispref/internals.texi +++ b/doc/lispref/internals.texi @@ -2391,6 +2391,15 @@ Buffer Internals This flag indicates that redisplay optimizations should not be used to display this buffer. +@item inhibit_buffer_hooks +This flag indicates that the buffer should not run the hooks +@code{kill-buffer-hook}, @code{kill-buffer-query-functions} +(@pxref{Killing Buffers}), and @code{buffer-list-update-hook} +(@pxref{Buffer List}). It is set at buffer creation (@pxref{Creating +Buffers}), and avoids slowing down internal or temporary buffers, such +as those created by @code{with-temp-buffer} (@pxref{Definition of +with-temp-buffer,, Current Buffer}). + @item overlay_center This field holds the current overlay center position. @xref{Managing Overlays}. @@ -2404,8 +2413,6 @@ Buffer Internals and @code{overlays_after} is sorted in order of increasing beginning position. -@c FIXME? the following are now all Lisp_Object BUFFER_INTERNAL_FIELD (foo). - @item name A Lisp string that names the buffer. It is guaranteed to be unique. @xref{Buffer Names}. This and the following fields have their names diff --git a/doc/misc/gnus-faq.texi b/doc/misc/gnus-faq.texi index adb812f572..c30e80ff56 100644 --- a/doc/misc/gnus-faq.texi +++ b/doc/misc/gnus-faq.texi @@ -1523,10 +1523,10 @@ FAQ 5-8 @example (setq message-default-headers - (with-temp-buffer - (insert "X-Face: ") - (insert-file-contents "~/.xface") - (buffer-string))) + (with-temp-buffer + (insert "X-Face: ") + (insert-file-contents "~/.xface") + (buffer-string))) @end example @noindent diff --git a/etc/NEWS b/etc/NEWS index 87463372d5..217b0b76a1 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -1817,6 +1817,13 @@ modifies the string's text properties; instead, it uses and returns a copy of the string. This helps avoid trouble when strings are shared or constants. ++++ +** Temporary buffers no longer run certain buffer hooks. +The macros 'with-temp-buffer' and 'with-temp-file' no longer run the +hooks 'kill-buffer-hook', 'kill-buffer-query-functions', and +'buffer-list-update-hook' for the temporary buffers they create. This +avoids slowing them down when a lot of these hooks are defined. + --- ** The obsolete function 'thread-alive-p' has been removed. @@ -2173,6 +2180,15 @@ Until it is solved you could ignore such errors by performing ** The error 'ftp-error' belongs also to category 'remote-file-error'. ++++ +** Buffers can now be created with certain hooks disabled. +The functions 'get-buffer-create' and 'generate-new-buffer' accept a +new optional argument 'inhibit-buffer-hooks'. If non-nil, the new +buffer does not run the hooks 'kill-buffer-hook', +'kill-buffer-query-functions', and 'buffer-list-update-hook'. This +avoids slowing down internal or temporary buffers that are never +presented to users or passed on to other applications. + \f * Changes in Emacs 28.1 on Non-Free Operating Systems diff --git a/lisp/files.el b/lisp/files.el index 093b5f92e5..70d451cccf 100644 --- a/lisp/files.el +++ b/lisp/files.el @@ -1850,6 +1850,10 @@ kill-buffer-hook The buffer being killed is current while the hook is running. See `kill-buffer'. +This hook is not run for internal or temporary buffers created by +`get-buffer-create' or `generate-new-buffer' with argument +INHIBIT-BUFFER-HOOKS non-nil. + Note: Be careful with let-binding this hook considering it is frequently used for cleanup.") @@ -1951,7 +1955,7 @@ create-file-buffer (let ((lastname (file-name-nondirectory filename))) (if (string= lastname "") (setq lastname filename)) - (generate-new-buffer (if (string-match-p "\\` " lastname) + (generate-new-buffer (if (string-prefix-p " " lastname) (concat "|" lastname) lastname)))) diff --git a/lisp/gnus/gnus-uu.el b/lisp/gnus/gnus-uu.el index 70aeac00d7..00ca430cc5 100644 --- a/lisp/gnus/gnus-uu.el +++ b/lisp/gnus/gnus-uu.el @@ -1587,8 +1587,7 @@ gnus-uu-unshar-article (save-excursion (switch-to-buffer (current-buffer)) (delete-other-windows) - (let ((buffer (get-buffer-create (generate-new-buffer-name - "*Warning*")))) + (let ((buffer (generate-new-buffer "*Warning*"))) (unless (unwind-protect (with-current-buffer buffer diff --git a/lisp/international/mule.el b/lisp/international/mule.el index 212e7232b4..6571454dff 100644 --- a/lisp/international/mule.el +++ b/lisp/international/mule.el @@ -307,12 +307,9 @@ load-with-code-conversion (and (null noerror) (signal 'file-error (list "Cannot open load file" file))) ;; Read file with code conversion, and then eval. - (let* ((buffer - ;; We can't use `generate-new-buffer' because files.el - ;; is not yet loaded. - (get-buffer-create (generate-new-buffer-name " *load*"))) - (load-in-progress t) - (source (save-match-data (string-match "\\.el\\'" fullname)))) + (let ((buffer (generate-new-buffer " *load*")) + (load-in-progress t) + (source (string-suffix-p ".el" fullname))) (unless nomessage (if source (message "Loading %s (source)..." file) diff --git a/lisp/json.el b/lisp/json.el index c2fc1574fa..5f512b94cd 100644 --- a/lisp/json.el +++ b/lisp/json.el @@ -435,7 +435,7 @@ json-encode-string (concat "\"" (substring-no-properties string) "\"") (with-current-buffer (or json--string-buffer - (with-current-buffer (generate-new-buffer " *json-string*") + (with-current-buffer (generate-new-buffer " *json-string*" t) ;; This seems to afford decent performance gains. (setq-local inhibit-modification-hooks t) (setq json--string-buffer (current-buffer)))) diff --git a/lisp/mh-e/mh-xface.el b/lisp/mh-e/mh-xface.el index 909f1fe95d..65039310e7 100644 --- a/lisp/mh-e/mh-xface.el +++ b/lisp/mh-e/mh-xface.el @@ -425,8 +425,7 @@ mh-x-image-url-fetch-image be displayed in a buffer and position specified by MARKER. The actual display is carried out by the SENTINEL function." (if mh-wget-executable - (let ((buffer (get-buffer-create (generate-new-buffer-name - mh-temp-fetch-buffer))) + (let ((buffer (generate-new-buffer mh-temp-fetch-buffer)) (filename (or (mh-funcall-if-exists make-temp-file "mhe-fetch") (expand-file-name (make-temp-name "~/mhe-fetch"))))) (with-current-buffer buffer diff --git a/lisp/net/imap.el b/lisp/net/imap.el index 0394f0efea..50b08d9612 100644 --- a/lisp/net/imap.el +++ b/lisp/net/imap.el @@ -1033,8 +1033,7 @@ imap-open (when (funcall (nth 1 (assq stream imap-stream-alist)) buffer) ;; Stream changed? (if (not (eq imap-default-stream stream)) - (with-current-buffer (get-buffer-create - (generate-new-buffer-name " *temp*")) + (with-current-buffer (generate-new-buffer " *temp*") (mapc 'make-local-variable imap-local-variables) (set-buffer-multibyte nil) (buffer-disable-undo) diff --git a/lisp/net/mailcap.el b/lisp/net/mailcap.el index d0f8c1272d..bc99f02fe3 100644 --- a/lisp/net/mailcap.el +++ b/lisp/net/mailcap.el @@ -386,8 +386,7 @@ mailcap-maybe-eval (when (save-window-excursion (delete-other-windows) - (let ((buffer (get-buffer-create (generate-new-buffer-name - "*Warning*")))) + (let ((buffer (generate-new-buffer "*Warning*"))) (unwind-protect (with-current-buffer buffer (insert (substitute-command-keys diff --git a/lisp/progmodes/flymake-proc.el b/lisp/progmodes/flymake-proc.el index 152dc725c7..e9d5018f30 100644 --- a/lisp/progmodes/flymake-proc.el +++ b/lisp/progmodes/flymake-proc.el @@ -429,16 +429,15 @@ flymake-proc--replace-region (defun flymake-proc--read-file-to-temp-buffer (file-name) "Insert contents of FILE-NAME into newly created temp buffer." - (let* ((temp-buffer (get-buffer-create (generate-new-buffer-name (concat "flymake:" (file-name-nondirectory file-name)))))) - (with-current-buffer temp-buffer - (insert-file-contents file-name)) - temp-buffer)) + (with-current-buffer (generate-new-buffer + (concat "flymake:" (file-name-nondirectory file-name))) + (insert-file-contents file-name) + (current-buffer))) (defun flymake-proc--copy-buffer-to-temp-buffer (buffer) "Copy contents of BUFFER into newly created temp buffer." - (with-current-buffer - (get-buffer-create (generate-new-buffer-name - (concat "flymake:" (buffer-name buffer)))) + (with-current-buffer (generate-new-buffer + (concat "flymake:" (buffer-name buffer))) (insert-buffer-substring buffer) (current-buffer))) diff --git a/lisp/simple.el b/lisp/simple.el index 090162b973..aab650e98a 100644 --- a/lisp/simple.el +++ b/lisp/simple.el @@ -4306,8 +4306,7 @@ shell-command-on-region (defun shell-command-to-string (command) "Execute shell command COMMAND and return its output as a string." (with-output-to-string - (with-current-buffer - standard-output + (with-current-buffer standard-output (shell-command command t)))) (defun process-file (program &optional infile buffer display &rest args) diff --git a/lisp/subr.el b/lisp/subr.el index 77c19c5bbf..6d5d7b4bac 100644 --- a/lisp/subr.el +++ b/lisp/subr.el @@ -3701,10 +3701,11 @@ internal--after-with-selected-window (when (window-live-p (nth 1 state)) (select-window (nth 1 state) 'norecord))) -(defun generate-new-buffer (name) +(defun generate-new-buffer (name &optional inhibit-buffer-hooks) "Create and return a buffer with a name based on NAME. -Choose the buffer's name using `generate-new-buffer-name'." - (get-buffer-create (generate-new-buffer-name name))) +Choose the buffer's name using `generate-new-buffer-name'. +See `get-buffer-create' for the meaning of INHIBIT-BUFFER-HOOKS." + (get-buffer-create (generate-new-buffer-name name) inhibit-buffer-hooks)) (defmacro with-selected-window (window &rest body) "Execute the forms in BODY with WINDOW as the selected window. @@ -3866,12 +3867,14 @@ with-output-to-temp-buffer (defmacro with-temp-file (file &rest body) "Create a new buffer, evaluate BODY there, and write the buffer to FILE. The value returned is the value of the last form in BODY. +The buffer does not run the hooks `kill-buffer-hook', +`kill-buffer-query-functions', and `buffer-list-update-hook'. See also `with-temp-buffer'." (declare (indent 1) (debug t)) (let ((temp-file (make-symbol "temp-file")) (temp-buffer (make-symbol "temp-buffer"))) `(let ((,temp-file ,file) - (,temp-buffer (generate-new-buffer " *temp file*"))) + (,temp-buffer (generate-new-buffer " *temp file*" t))) (unwind-protect (prog1 (with-current-buffer ,temp-buffer @@ -3906,10 +3909,12 @@ with-temp-message (defmacro with-temp-buffer (&rest body) "Create a temporary buffer, and evaluate BODY there like `progn'. +The buffer does not run the hooks `kill-buffer-hook', +`kill-buffer-query-functions', and `buffer-list-update-hook'. See also `with-temp-file' and `with-output-to-string'." (declare (indent 0) (debug t)) (let ((temp-buffer (make-symbol "temp-buffer"))) - `(let ((,temp-buffer (generate-new-buffer " *temp*"))) + `(let ((,temp-buffer (generate-new-buffer " *temp*" t))) ;; `kill-buffer' can change current-buffer in some odd cases. (with-current-buffer ,temp-buffer (unwind-protect @@ -3944,7 +3949,7 @@ with-silent-modifications (defmacro with-output-to-string (&rest body) "Execute BODY, return the text it sent to `standard-output', as a string." (declare (indent 0) (debug t)) - `(let ((standard-output (generate-new-buffer " *string-output*"))) + `(let ((standard-output (generate-new-buffer " *string-output*" t))) (unwind-protect (progn (let ((standard-output standard-output)) diff --git a/src/buffer.c b/src/buffer.c index dfc34faf6e..9564f66096 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -37,7 +37,6 @@ Copyright (C) 1985-1989, 1993-1995, 1997-2020 Free Software Foundation, #include "window.h" #include "commands.h" #include "character.h" -#include "coding.h" #include "buffer.h" #include "region-cache.h" #include "indent.h" @@ -514,16 +513,32 @@ get_truename_buffer (register Lisp_Object filename) return Qnil; } -DEFUN ("get-buffer-create", Fget_buffer_create, Sget_buffer_create, 1, 1, 0, +/* Run 'buffer-list-update-hook' if Vrun_hooks is non-nil, and BUF is + either NULL or does not have buffer hooks inhibited. */ + +static void +run_buffer_list_update_hook (struct buffer *buf) +{ + if (! (NILP (Vrun_hooks) || (buf && buf->inhibit_buffer_hooks))) + call1 (Vrun_hooks, Qbuffer_list_update_hook); +} + +DEFUN ("get-buffer-create", Fget_buffer_create, Sget_buffer_create, 1, 2, 0, doc: /* Return the buffer specified by BUFFER-OR-NAME, creating a new one if needed. If BUFFER-OR-NAME is a string and a live buffer with that name exists, return that buffer. If no such buffer exists, create a new buffer with -that name and return it. If BUFFER-OR-NAME starts with a space, the new -buffer does not keep undo information. +that name and return it. + +If BUFFER-OR-NAME starts with a space, the new buffer does not keep undo +information. If optional argument INHIBIT-BUFFER-HOOKS is non-nil, the +new buffer does not run the hooks `kill-buffer-hook', +`kill-buffer-query-functions', and `buffer-list-update-hook'. This +avoids slowing down internal or temporary buffers that are never +presented to users or passed on to other applications. If BUFFER-OR-NAME is a buffer instead of a string, return it as given, even if it is dead. The return value is never nil. */) - (register Lisp_Object buffer_or_name) + (register Lisp_Object buffer_or_name, Lisp_Object inhibit_buffer_hooks) { register Lisp_Object buffer, name; register struct buffer *b; @@ -598,11 +613,7 @@ DEFUN ("get-buffer-create", Fget_buffer_create, Sget_buffer_create, 1, 1, 0, set_string_intervals (name, NULL); bset_name (b, name); - b->inhibit_buffer_hooks - = (STRINGP (Vcode_conversion_workbuf_name) - && strncmp (SSDATA (name), SSDATA (Vcode_conversion_workbuf_name), - SBYTES (Vcode_conversion_workbuf_name)) == 0); - + b->inhibit_buffer_hooks = !NILP (inhibit_buffer_hooks); bset_undo_list (b, SREF (name, 0) != ' ' ? Qnil : Qt); reset_buffer (b); @@ -614,9 +625,8 @@ DEFUN ("get-buffer-create", Fget_buffer_create, Sget_buffer_create, 1, 1, 0, /* Put this in the alist of all live buffers. */ XSETBUFFER (buffer, b); Vbuffer_alist = nconc2 (Vbuffer_alist, list1 (Fcons (name, buffer))); - /* And run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !b->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + + run_buffer_list_update_hook (b); return buffer; } @@ -890,9 +900,7 @@ DEFUN ("make-indirect-buffer", Fmake_indirect_buffer, Smake_indirect_buffer, set_buffer_internal_1 (old_b); } - /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks)) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (NULL); return buf; } @@ -1536,9 +1544,7 @@ DEFUN ("rename-buffer", Frename_buffer, Srename_buffer, 1, 2, && !NILP (BVAR (current_buffer, auto_save_file_name))) call0 (intern ("rename-auto-save-file")); - /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !current_buffer->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (current_buffer); /* Refetch since that last call may have done GC. */ return BVAR (current_buffer, name); @@ -1612,7 +1618,7 @@ DEFUN ("other-buffer", Fother_buffer, Sother_buffer, 0, 3, 0, buf = Fget_buffer (scratch); if (NILP (buf)) { - buf = Fget_buffer_create (scratch); + buf = Fget_buffer_create (scratch, Qnil); Fset_buffer_major_mode (buf); } return buf; @@ -1636,7 +1642,7 @@ other_buffer_safely (Lisp_Object buffer) buf = Fget_buffer (scratch); if (NILP (buf)) { - buf = Fget_buffer_create (scratch); + buf = Fget_buffer_create (scratch, Qnil); Fset_buffer_major_mode (buf); } @@ -1713,7 +1719,9 @@ DEFUN ("kill-buffer", Fkill_buffer, Skill_buffer, 0, 1, "bKill buffer: ", the buffer is not killed. The hook `kill-buffer-hook' is run before the buffer is actually killed. The buffer being killed will be current while the hook is running. Functions called by any of these hooks are -supposed to not change the current buffer. +supposed to not change the current buffer. Neither hook is run for +internal or temporary buffers created by `get-buffer-create' or +`generate-new-buffer' with argument INHIBIT-BUFFER-HOOKS non-nil. Any processes that have this buffer as the `process-buffer' are killed with SIGHUP. This function calls `replace-buffer-in-windows' for @@ -1973,9 +1981,7 @@ DEFUN ("kill-buffer", Fkill_buffer, Skill_buffer, 0, 1, "bKill buffer: ", bset_width_table (b, Qnil); unblock_input (); - /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !b->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (b); return Qt; } @@ -2015,9 +2021,7 @@ record_buffer (Lisp_Object buffer) fset_buffer_list (f, Fcons (buffer, Fdelq (buffer, f->buffer_list))); fset_buried_buffer_list (f, Fdelq (buffer, f->buried_buffer_list)); - /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !XBUFFER (buffer)->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (XBUFFER (buffer)); } @@ -2054,9 +2058,7 @@ DEFUN ("bury-buffer-internal", Fbury_buffer_internal, Sbury_buffer_internal, fset_buried_buffer_list (f, Fcons (buffer, Fdelq (buffer, f->buried_buffer_list))); - /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !XBUFFER (buffer)->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (XBUFFER (buffer)); return Qnil; } @@ -5349,10 +5351,11 @@ init_buffer_once (void) Fput (Qkill_buffer_hook, Qpermanent_local, Qt); /* Super-magic invisible buffer. */ - Vprin1_to_string_buffer = Fget_buffer_create (build_pure_c_string (" prin1")); + Vprin1_to_string_buffer = + Fget_buffer_create (build_pure_c_string (" prin1"), Qt); Vbuffer_alist = Qnil; - Fset_buffer (Fget_buffer_create (build_pure_c_string ("*scratch*"))); + Fset_buffer (Fget_buffer_create (build_pure_c_string ("*scratch*"), Qnil)); inhibit_modification_hooks = 0; } @@ -5397,7 +5400,7 @@ init_buffer (void) #endif /* USE_MMAP_FOR_BUFFERS */ AUTO_STRING (scratch, "*scratch*"); - Fset_buffer (Fget_buffer_create (scratch)); + Fset_buffer (Fget_buffer_create (scratch, Qnil)); if (NILP (BVAR (&buffer_defaults, enable_multibyte_characters))) Fset_buffer_multibyte (Qnil); @@ -6300,9 +6303,14 @@ from (abs POSITION). If POSITION is positive, point was at the front DEFVAR_LISP ("kill-buffer-query-functions", Vkill_buffer_query_functions, doc: /* List of functions called with no args to query before killing a buffer. The buffer being killed will be current while the functions are running. +See `kill-buffer'. If any of them returns nil, the buffer is not killed. Functions run by -this hook are supposed to not change the current buffer. */); +this hook are supposed to not change the current buffer. + +This hook is not run for internal or temporary buffers created by +`get-buffer-create' or `generate-new-buffer' with argument +INHIBIT-BUFFER-HOOKS non-nil. */); Vkill_buffer_query_functions = Qnil; DEFVAR_LISP ("change-major-mode-hook", Vchange_major_mode_hook, @@ -6315,9 +6323,12 @@ from (abs POSITION). If POSITION is positive, point was at the front doc: /* Hook run when the buffer list changes. Functions (implicitly) running this hook are `get-buffer-create', `make-indirect-buffer', `rename-buffer', `kill-buffer', `bury-buffer' -and `select-window'. Functions run by this hook should avoid calling -`select-window' with a nil NORECORD argument or `with-temp-buffer' -since either may lead to infinite recursion. */); +and `select-window'. This hook is not run for internal or temporary +buffers created by `get-buffer-create' or `generate-new-buffer' with +argument INHIBIT-BUFFER-HOOKS non-nil. + +Functions run by this hook should avoid calling `select-window' with a +nil NORECORD argument since it may lead to infinite recursion. */); Vbuffer_list_update_hook = Qnil; DEFSYM (Qbuffer_list_update_hook, "buffer-list-update-hook"); diff --git a/src/buffer.h b/src/buffer.h index fe549c5dac..b8c5162be4 100644 --- a/src/buffer.h +++ b/src/buffer.h @@ -663,11 +663,11 @@ #define BVAR(buf, field) ((buf)->field ## _) /* Non-zero whenever the narrowing is changed in this buffer. */ bool_bf clip_changed : 1; - /* Non-zero for internally used temporary buffers that don't need to - run hooks kill-buffer-hook, buffer-list-update-hook, and - kill-buffer-query-functions. This is used in coding.c to avoid - slowing down en/decoding when there are a lot of these hooks - defined. */ + /* Non-zero for internal or temporary buffers that don't need to + run hooks kill-buffer-hook, kill-buffer-query-functions, and + buffer-list-update-hook. This is used in coding.c to avoid + slowing down en/decoding when a lot of these hooks are + defined, as well as by with-temp-buffer, for example. */ bool_bf inhibit_buffer_hooks : 1; /* List of overlays that end at or before the current center, diff --git a/src/callproc.c b/src/callproc.c index e3346e2eab..4bca1e5ebd 100644 --- a/src/callproc.c +++ b/src/callproc.c @@ -405,9 +405,8 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd, if (! (NILP (buffer) || EQ (buffer, Qt) || FIXNUMP (buffer))) { - Lisp_Object spec_buffer; - spec_buffer = buffer; - buffer = Fget_buffer_create (buffer); + Lisp_Object spec_buffer = buffer; + buffer = Fget_buffer_create (buffer, Qnil); /* Mention the buffer name for a better error message. */ if (NILP (buffer)) CHECK_BUFFER (spec_buffer); diff --git a/src/coding.c b/src/coding.c index 2142e7fa51..1afa4aa474 100644 --- a/src/coding.c +++ b/src/coding.c @@ -7821,7 +7821,7 @@ encode_coding (struct coding_system *coding) /* A string that serves as name of the reusable work buffer, and as base name of temporary work buffers used for code-conversion operations. */ -Lisp_Object Vcode_conversion_workbuf_name; +static Lisp_Object Vcode_conversion_workbuf_name; /* The reusable working buffer, created once and never killed. */ static Lisp_Object Vcode_conversion_reused_workbuf; @@ -7839,7 +7839,7 @@ code_conversion_restore (Lisp_Object arg) if (! NILP (workbuf)) { if (EQ (workbuf, Vcode_conversion_reused_workbuf)) - reused_workbuf_in_use = 0; + reused_workbuf_in_use = false; else Fkill_buffer (workbuf); } @@ -7857,13 +7857,13 @@ code_conversion_save (bool with_work_buf, bool multibyte) { Lisp_Object name = Fgenerate_new_buffer_name (Vcode_conversion_workbuf_name, Qnil); - workbuf = Fget_buffer_create (name); + workbuf = Fget_buffer_create (name, Qt); } else { if (NILP (Fbuffer_live_p (Vcode_conversion_reused_workbuf))) Vcode_conversion_reused_workbuf - = Fget_buffer_create (Vcode_conversion_workbuf_name); + = Fget_buffer_create (Vcode_conversion_workbuf_name, Qt); workbuf = Vcode_conversion_reused_workbuf; } } @@ -7881,7 +7881,7 @@ code_conversion_save (bool with_work_buf, bool multibyte) bset_undo_list (current_buffer, Qt); bset_enable_multibyte_characters (current_buffer, multibyte ? Qt : Qnil); if (EQ (workbuf, Vcode_conversion_reused_workbuf)) - reused_workbuf_in_use = 1; + reused_workbuf_in_use = true; set_buffer_internal (current); } @@ -11639,7 +11639,7 @@ syms_of_coding (void) staticpro (&Vcode_conversion_workbuf_name); Vcode_conversion_workbuf_name = build_pure_c_string (" *code-conversion-work*"); - reused_workbuf_in_use = 0; + reused_workbuf_in_use = false; PDUMPER_REMEMBER_SCALAR (reused_workbuf_in_use); DEFSYM (Qcharset, "charset"); diff --git a/src/coding.h b/src/coding.h index 4973cf89eb..9ad1e954f8 100644 --- a/src/coding.h +++ b/src/coding.h @@ -97,9 +97,6 @@ #define EMACS_CODING_H extern Lisp_Object Vcoding_system_hash_table; -/* Name (or base name) of work buffer for code conversion. */ -extern Lisp_Object Vcode_conversion_workbuf_name; - /* Enumeration of index to an attribute vector of a coding system. */ enum coding_attr_index diff --git a/src/fileio.c b/src/fileio.c index c97f4daf20..51f12e104e 100644 --- a/src/fileio.c +++ b/src/fileio.c @@ -4004,7 +4004,7 @@ because (1) it preserves some marker positions (in unchanged portions record_unwind_current_buffer (); - workbuf = Fget_buffer_create (name); + workbuf = Fget_buffer_create (name, Qt); buf = XBUFFER (workbuf); delete_all_overlays (buf); diff --git a/src/minibuf.c b/src/minibuf.c index fc3fd92a88..1940564a80 100644 --- a/src/minibuf.c +++ b/src/minibuf.c @@ -809,7 +809,7 @@ get_minibuffer (EMACS_INT depth) static char const name_fmt[] = " *Minibuf-%"pI"d*"; char name[sizeof name_fmt + INT_STRLEN_BOUND (EMACS_INT)]; AUTO_STRING_WITH_LEN (lname, name, sprintf (name, name_fmt, depth)); - buf = Fget_buffer_create (lname); + buf = Fget_buffer_create (lname, Qnil); /* Although the buffer's name starts with a space, undo should be enabled in it. */ diff --git a/src/print.c b/src/print.c index 008bf5e639..ec271d914c 100644 --- a/src/print.c +++ b/src/print.c @@ -562,7 +562,7 @@ temp_output_buffer_setup (const char *bufname) record_unwind_current_buffer (); - Fset_buffer (Fget_buffer_create (build_string (bufname))); + Fset_buffer (Fget_buffer_create (build_string (bufname), Qnil)); Fkill_all_local_variables (); delete_all_overlays (current_buffer); diff --git a/src/process.c b/src/process.c index 4fe8ac7fc0..9efefb1de7 100644 --- a/src/process.c +++ b/src/process.c @@ -1731,7 +1731,7 @@ DEFUN ("make-process", Fmake_process, Smake_process, 0, MANY, 0, buffer = Fplist_get (contact, QCbuffer); if (!NILP (buffer)) - buffer = Fget_buffer_create (buffer); + buffer = Fget_buffer_create (buffer, Qnil); /* Make sure that the child will be able to chdir to the current buffer's current directory, or its unhandled equivalent. We @@ -1768,7 +1768,7 @@ DEFUN ("make-process", Fmake_process, Smake_process, 0, MANY, 0, QCname, concat2 (name, build_string (" stderr")), QCbuffer, - Fget_buffer_create (xstderr), + Fget_buffer_create (xstderr, Qnil), QCnoquery, query_on_exit ? Qnil : Qt); } @@ -2443,7 +2443,7 @@ DEFUN ("make-pipe-process", Fmake_pipe_process, Smake_pipe_process, buffer = Fplist_get (contact, QCbuffer); if (NILP (buffer)) buffer = name; - buffer = Fget_buffer_create (buffer); + buffer = Fget_buffer_create (buffer, Qnil); pset_buffer (p, buffer); pset_childp (p, contact); @@ -3173,7 +3173,7 @@ DEFUN ("make-serial-process", Fmake_serial_process, Smake_serial_process, buffer = Fplist_get (contact, QCbuffer); if (NILP (buffer)) buffer = name; - buffer = Fget_buffer_create (buffer); + buffer = Fget_buffer_create (buffer, Qnil); pset_buffer (p, buffer); pset_childp (p, contact); @@ -4188,7 +4188,7 @@ DEFUN ("make-network-process", Fmake_network_process, Smake_network_process, open_socket: if (!NILP (buffer)) - buffer = Fget_buffer_create (buffer); + buffer = Fget_buffer_create (buffer, Qnil); /* Unwind bind_polling_period. */ unbind_to (count, Qnil); @@ -4961,7 +4961,7 @@ server_accept_connection (Lisp_Object server, int channel) if (!NILP (buffer)) { args[1] = buffer; - buffer = Fget_buffer_create (Fformat (nargs, args)); + buffer = Fget_buffer_create (Fformat (nargs, args), Qnil); } } diff --git a/src/w32fns.c b/src/w32fns.c index a840f0e122..36bee0676b 100644 --- a/src/w32fns.c +++ b/src/w32fns.c @@ -7372,7 +7372,7 @@ DEFUN ("x-show-tip", Fx_show_tip, Sx_show_tip, 1, 6, 0, tip_f = XFRAME (tip_frame); window = FRAME_ROOT_WINDOW (tip_f); - tip_buf = Fget_buffer_create (tip); + tip_buf = Fget_buffer_create (tip, Qnil); /* We will mark the tip window a "pseudo-window" below, and such windows cannot have display margins. */ bset_left_margin_cols (XBUFFER (tip_buf), make_fixnum (0)); diff --git a/src/window.c b/src/window.c index bcc989b5a7..5db166e345 100644 --- a/src/window.c +++ b/src/window.c @@ -617,11 +617,12 @@ DEFUN ("select-window", Fselect_window, Sselect_window, 1, 2, 0, Run `buffer-list-update-hook' unless NORECORD is non-nil. Note that applications and internal routines often select a window temporarily for various purposes; mostly, to simplify coding. As a rule, such -selections should be not recorded and therefore will not pollute +selections should not be recorded and therefore will not pollute `buffer-list-update-hook'. Selections that "really count" are those causing a visible change in the next redisplay of WINDOW's frame and -should be always recorded. So if you think of running a function each -time a window gets selected put it on `buffer-list-update-hook'. +should always be recorded. So if you think of running a function each +time a window gets selected, put it on `buffer-list-update-hook' or +`window-selection-change-functions'. Also note that the main editor command loop sets the current buffer to the buffer of the selected window before each command. */) diff --git a/src/xdisp.c b/src/xdisp.c index 0fd5ec5ec5..b5adee5105 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -10880,7 +10880,7 @@ message_dolog (const char *m, ptrdiff_t nbytes, bool nlflag, bool multibyte) /* Ensure the Messages buffer exists, and switch to it. If we created it, set the major-mode. */ bool newbuffer = NILP (Fget_buffer (Vmessages_buffer_name)); - Fset_buffer (Fget_buffer_create (Vmessages_buffer_name)); + Fset_buffer (Fget_buffer_create (Vmessages_buffer_name, Qnil)); if (newbuffer && !NILP (Ffboundp (intern ("messages-buffer-mode")))) call0 (intern ("messages-buffer-mode")); @@ -11366,7 +11366,7 @@ ensure_echo_area_buffers (void) static char const name_fmt[] = " *Echo Area %d*"; char name[sizeof name_fmt + INT_STRLEN_BOUND (int)]; AUTO_STRING_WITH_LEN (lname, name, sprintf (name, name_fmt, i)); - echo_buffer[i] = Fget_buffer_create (lname); + echo_buffer[i] = Fget_buffer_create (lname, Qnil); bset_truncate_lines (XBUFFER (echo_buffer[i]), Qnil); /* to force word wrap in echo area - it was decided to postpone this*/ diff --git a/src/xfns.c b/src/xfns.c index 46e4bd73a6..abe293e903 100644 --- a/src/xfns.c +++ b/src/xfns.c @@ -7041,7 +7041,7 @@ DEFUN ("x-show-tip", Fx_show_tip, Sx_show_tip, 1, 6, 0, tip_f = XFRAME (tip_frame); window = FRAME_ROOT_WINDOW (tip_f); - tip_buf = Fget_buffer_create (tip); + tip_buf = Fget_buffer_create (tip, Qnil); /* We will mark the tip window a "pseudo-window" below, and such windows cannot have display margins. */ bset_left_margin_cols (XBUFFER (tip_buf), make_fixnum (0)); diff --git a/src/xwidget.c b/src/xwidget.c index e078a28a35..accde65b52 100644 --- a/src/xwidget.c +++ b/src/xwidget.c @@ -100,7 +100,8 @@ DEFUN ("make-xwidget", Lisp_Object val; xw->type = type; xw->title = title; - xw->buffer = NILP (buffer) ? Fcurrent_buffer () : Fget_buffer_create (buffer); + xw->buffer = (NILP (buffer) ? Fcurrent_buffer () + : Fget_buffer_create (buffer, Qnil)); xw->height = XFIXNAT (height); xw->width = XFIXNAT (width); xw->kill_without_query = false; diff --git a/test/src/buffer-tests.el b/test/src/buffer-tests.el index 0db66f9751..dd8927457a 100644 --- a/test/src/buffer-tests.el +++ b/test/src/buffer-tests.el @@ -19,9 +19,7 @@ ;;; Code: -(require 'ert) -(require 'seq) -(eval-when-compile (require 'cl-lib)) +(require 'cl-lib) (ert-deftest overlay-modification-hooks-message-other-buf () "Test for bug#21824. @@ -1334,4 +1332,33 @@ buffer-tests-buffer-local-variables-undo (with-temp-buffer (should (assq 'buffer-undo-list (buffer-local-variables))))) +(ert-deftest buffer-tests-inhibit-buffer-hooks () + "Test `get-buffer-create' argument INHIBIT-BUFFER-HOOKS." + (let* (run-bluh (bluh (lambda () (setq run-bluh t)))) + (unwind-protect + (let* ( run-kbh (kbh (lambda () (setq run-kbh t))) + run-kbqf (kbqf (lambda () (setq run-kbqf t))) ) + + ;; Inhibited. + (add-hook 'buffer-list-update-hook bluh) + (with-current-buffer (generate-new-buffer " foo" t) + (add-hook 'kill-buffer-hook kbh nil t) + (add-hook 'kill-buffer-query-functions kbqf nil t) + (kill-buffer)) + (with-temp-buffer) + (with-output-to-string) + (should-not run-bluh) + (should-not run-kbh) + (should-not run-kbqf) + + ;; Not inhibited. + (with-current-buffer (generate-new-buffer " foo") + (should run-bluh) + (add-hook 'kill-buffer-hook kbh nil t) + (add-hook 'kill-buffer-query-functions kbqf nil t) + (kill-buffer)) + (should run-kbh) + (should run-kbqf)) + (remove-hook 'buffer-list-update-hook bluh)))) + ;;; buffer-tests.el ends here -- 2.29.2 ^ permalink raw reply related [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-12-18 18:49 ` Basil L. Contovounesios @ 2020-12-19 10:33 ` Eli Zaretskii 2020-12-19 14:15 ` Basil L. Contovounesios 0 siblings, 1 reply; 72+ messages in thread From: Eli Zaretskii @ 2020-12-19 10:33 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: 34765, larsi, monnier, alexanderm > From: "Basil L. Contovounesios" <contovob@tcd.ie> > Cc: Eli Zaretskii <eliz@gnu.org>, rudalics@gmx.at, larsi@gnus.org, > 34765@debbugs.gnu.org, alexanderm@web.de > Date: Fri, 18 Dec 2020 18:49:52 +0000 > > * .dir-locals.el (c-mode): Enforce existing indent-tabs-mode policy. This should be in a separate commit, IMO. > +By default, undo information (@pxref{Undo}) is not recorded in the > +buffer created by this macro (but @var{body} can enable that, if > +needed). The temporary buffer also does not run the hooks > +@code{kill-buffer-hook}, @code{kill-buffer-query-functions} > +(@pxref{Killing Buffers}), and @code{buffer-list-update-hook} > +(@pxref{Buffer List}). It would be good to have here index entries about undo and those hooks not being used by default in temporary buffers. > +Like @code{with-temp-buffer} (@pxref{Definition of with-temp-buffer,, ^^^^^^^^^^ I think this word will look better if not capitalized. > +static void > +run_buffer_list_update_hook (struct buffer *buf) > +{ > + if (! (NILP (Vrun_hooks) || (buf && buf->inhibit_buffer_hooks))) ^^^ Why this test? is it possible for this function to be called with buf a NULL pointer? Thanks. ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-12-19 10:33 ` Eli Zaretskii @ 2020-12-19 14:15 ` Basil L. Contovounesios 2020-12-19 16:06 ` Eli Zaretskii 0 siblings, 1 reply; 72+ messages in thread From: Basil L. Contovounesios @ 2020-12-19 14:15 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 34765, larsi, monnier, alexanderm Eli Zaretskii <eliz@gnu.org> writes: >> From: "Basil L. Contovounesios" <contovob@tcd.ie> >> Cc: Eli Zaretskii <eliz@gnu.org>, rudalics@gmx.at, larsi@gnus.org, >> 34765@debbugs.gnu.org, alexanderm@web.de >> Date: Fri, 18 Dec 2020 18:49:52 +0000 >> >> * .dir-locals.el (c-mode): Enforce existing indent-tabs-mode policy. > > This should be in a separate commit, IMO. Okay, I pushed it separately. >> +By default, undo information (@pxref{Undo}) is not recorded in the >> +buffer created by this macro (but @var{body} can enable that, if >> +needed). The temporary buffer also does not run the hooks >> +@code{kill-buffer-hook}, @code{kill-buffer-query-functions} >> +(@pxref{Killing Buffers}), and @code{buffer-list-update-hook} >> +(@pxref{Buffer List}). > > It would be good to have here index entries about undo and those hooks > not being used by default in temporary buffers. Something like this? @cindex undo in temporary buffers @cindex @code{kill-buffer-hook} in temporary buffers @cindex @code{kill-buffer-query-functions} in temporary buffers @cindex @code{buffer-list-update-hook} in temporary buffers >> +Like @code{with-temp-buffer} (@pxref{Definition of with-temp-buffer,, > ^^^^^^^^^^ > I think this word will look better if not capitalized. The printed label "see Current Buffer" should be displayed instead of this word, which is part of the anchor. Is that okay? >> +static void >> +run_buffer_list_update_hook (struct buffer *buf) >> +{ >> + if (! (NILP (Vrun_hooks) || (buf && buf->inhibit_buffer_hooks))) > ^^^ > Why this test? is it possible for this function to be called with buf > a NULL pointer? Yes, in Fmake_indirect_buffer, which doesn't check inhibit_buffer_hooks. The alternatives would be for Fmake_indirect_buffer to not call run_buffer_list_update_hook, or to not bother adding run_buffer_list_update_hook at all. Do you have a preference? Thanks, -- Basil ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-12-19 14:15 ` Basil L. Contovounesios @ 2020-12-19 16:06 ` Eli Zaretskii 2020-12-19 21:10 ` Basil L. Contovounesios 0 siblings, 1 reply; 72+ messages in thread From: Eli Zaretskii @ 2020-12-19 16:06 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: 34765, larsi, monnier, alexanderm > From: "Basil L. Contovounesios" <contovob@tcd.ie> > Cc: monnier@iro.umontreal.ca, rudalics@gmx.at, larsi@gnus.org, > 34765@debbugs.gnu.org, alexanderm@web.de > Date: Sat, 19 Dec 2020 14:15:30 +0000 > > >> +By default, undo information (@pxref{Undo}) is not recorded in the > >> +buffer created by this macro (but @var{body} can enable that, if > >> +needed). The temporary buffer also does not run the hooks > >> +@code{kill-buffer-hook}, @code{kill-buffer-query-functions} > >> +(@pxref{Killing Buffers}), and @code{buffer-list-update-hook} > >> +(@pxref{Buffer List}). > > > > It would be good to have here index entries about undo and those hooks > > not being used by default in temporary buffers. > > Something like this? > > @cindex undo in temporary buffers > @cindex @code{kill-buffer-hook} in temporary buffers > @cindex @code{kill-buffer-query-functions} in temporary buffers > @cindex @code{buffer-list-update-hook} in temporary buffers Yes. > >> +Like @code{with-temp-buffer} (@pxref{Definition of with-temp-buffer,, > > ^^^^^^^^^^ > > I think this word will look better if not capitalized. > > The printed label "see Current Buffer" should be displayed instead of > this word, which is part of the anchor. Is that okay? Sorry, I didn't see that this is an anchor. So I think the anchor should not start with a capital letter, as it reads more naturally that way, I think. And then the name should be used without capitalization in the cross-references. Obviously, this is a minor nit. > >> +static void > >> +run_buffer_list_update_hook (struct buffer *buf) > >> +{ > >> + if (! (NILP (Vrun_hooks) || (buf && buf->inhibit_buffer_hooks))) > > ^^^ > > Why this test? is it possible for this function to be called with buf > > a NULL pointer? > > Yes, in Fmake_indirect_buffer, which doesn't check inhibit_buffer_hooks. > > The alternatives would be for Fmake_indirect_buffer to not call > run_buffer_list_update_hook, or to not bother adding > run_buffer_list_update_hook at all. Do you have a preference? I think just having a comment there saying that make-indirect-buffer calls this with NULL argument should be okay. Thanks. ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-12-19 16:06 ` Eli Zaretskii @ 2020-12-19 21:10 ` Basil L. Contovounesios 2020-12-20 15:05 ` Eli Zaretskii 0 siblings, 1 reply; 72+ messages in thread From: Basil L. Contovounesios @ 2020-12-19 21:10 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 34765, larsi, monnier, alexanderm [-- Attachment #1: Type: text/plain, Size: 1903 bytes --] Eli Zaretskii <eliz@gnu.org> writes: >> Something like this? >> >> @cindex undo in temporary buffers >> @cindex @code{kill-buffer-hook} in temporary buffers >> @cindex @code{kill-buffer-query-functions} in temporary buffers >> @cindex @code{buffer-list-update-hook} in temporary buffers > > Yes. Done. >> >> +Like @code{with-temp-buffer} (@pxref{Definition of with-temp-buffer,, >> > ^^^^^^^^^^ >> > I think this word will look better if not capitalized. >> >> The printed label "see Current Buffer" should be displayed instead of >> this word, which is part of the anchor. Is that okay? > > Sorry, I didn't see that this is an anchor. So I think the anchor > should not start with a capital letter, as it reads more naturally > that way, I think. And then the name should be used without > capitalization in the cross-references. Obviously, this is a minor > nit. I'm hesitant to change the anchor because all 37 such "Definition of..." anchors in the tree are capitalised, as are the printed labels of all their refs. I'm happy to downcase them, but maybe this should be done wholesale in a separate commit? >> >> +static void >> >> +run_buffer_list_update_hook (struct buffer *buf) >> >> +{ >> >> + if (! (NILP (Vrun_hooks) || (buf && buf->inhibit_buffer_hooks))) >> > ^^^ >> > Why this test? is it possible for this function to be called with buf >> > a NULL pointer? >> >> Yes, in Fmake_indirect_buffer, which doesn't check inhibit_buffer_hooks. >> >> The alternatives would be for Fmake_indirect_buffer to not call >> run_buffer_list_update_hook, or to not bother adding >> run_buffer_list_update_hook at all. Do you have a preference? > > I think just having a comment there saying that make-indirect-buffer > calls this with NULL argument should be okay. How's the attached? Thanks, -- Basil [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Inhibit-buffer-hooks-in-temporary-buffers.patch --] [-- Type: text/x-diff, Size: 45024 bytes --] From 5ad1cc0d3f67ba3077d34949412990fb6e4b50a6 Mon Sep 17 00:00:00 2001 From: "Basil L. Contovounesios" <contovob@tcd.ie> Date: Sat, 19 Dec 2020 12:39:45 +0000 Subject: [PATCH] Inhibit buffer hooks in temporary buffers Give get-buffer-create an optional argument to inhibit buffer hooks in internal or temporary buffers for efficiency (bug#34765). * etc/NEWS: Announce new parameter of get-buffer-create and generate-new-buffer, and that with-temp-buffer and with-temp-file now inhibit buffer hooks. * doc/lispref/buffers.texi (Buffer Names): Fix typo. (Creating Buffers): Document new parameter of get-buffer-create and generate-new-buffer. (Buffer List, Killing Buffers): Document when buffer hooks are inhibited. (Current Buffer): * doc/lispref/files.texi (Writing to Files): Document that with-temp-buffer and with-temp-file inhibit buffer hooks. * doc/lispref/internals.texi (Buffer Internals): Document inhibit_buffer_hooks flag. Remove stale comment. * doc/misc/gnus-faq.texi (FAQ 5-8): * lisp/simple.el (shell-command-on-region): Fix indentation. * lisp/files.el (kill-buffer-hook): Document when hook is inhibited. (create-file-buffer): * lisp/gnus/gnus-uu.el (gnus-uu-unshar-article): * lisp/international/mule.el (load-with-code-conversion): * lisp/mh-e/mh-xface.el (mh-x-image-url-fetch-image): * lisp/net/imap.el (imap-open): * lisp/net/mailcap.el (mailcap-maybe-eval): * lisp/progmodes/flymake-proc.el (flymake-proc--read-file-to-temp-buffer) (flymake-proc--copy-buffer-to-temp-buffer): Simplify. * lisp/subr.el (generate-new-buffer): Forward new optional argument to inhibit buffer hooks to get-buffer-create. (with-temp-file, with-temp-buffer, with-output-to-string): * lisp/json.el (json-encode-string): Inhibit buffer hooks in buffer used. * src/buffer.c (run_buffer_list_update_hook): New helper function. (Fget_buffer_create): Use it. Add optional argument to set inhibit_buffer_hooks flag instead of comparing the buffer name to Vcode_conversion_workbuf_name. All callers changed. (Fmake_indirect_buffer, Frename_buffer, Fbury_buffer_internal) (record_buffer): Use run_buffer_list_update_hook. (Fkill_buffer): Document when buffer hooks are inhibited. Use run_buffer_list_update_hook. (init_buffer_once): Inhibit buffer hooks in Vprin1_to_string_buffer. (Vkill_buffer_query_functions, Vbuffer_list_update_hook): Document when hooks are inhibited. * src/buffer.h (struct buffer): Update inhibit_buffer_hooks commentary. * src/coding.h (Vcode_conversion_workbuf_name): * src/coding.c (Vcode_conversion_workbuf_name): Make static again since it is no longer needed in src/buffer.c. (code_conversion_restore, code_conversion_save, syms_of_coding): Prefer boolean over integer constants. * src/fileio.c (Finsert_file_contents): Inhibit buffer hooks in " *code-converting-work*" buffer. * src/window.c (Fselect_window): Fix grammar. Mention window-selection-change-functions alongside buffer-list-update-hook. * test/src/buffer-tests.el: Fix requires. (buffer-tests-inhibit-buffer-hooks): New test. --- doc/lispref/buffers.texi | 59 +++++++++++++++++------ doc/lispref/files.texi | 7 ++- doc/lispref/internals.texi | 11 ++++- doc/misc/gnus-faq.texi | 8 ++-- etc/NEWS | 16 +++++++ lisp/files.el | 6 ++- lisp/gnus/gnus-uu.el | 3 +- lisp/international/mule.el | 9 ++-- lisp/json.el | 2 +- lisp/mh-e/mh-xface.el | 3 +- lisp/net/imap.el | 3 +- lisp/net/mailcap.el | 3 +- lisp/progmodes/flymake-proc.el | 13 +++-- lisp/simple.el | 3 +- lisp/subr.el | 17 ++++--- src/buffer.c | 88 +++++++++++++++++++--------------- src/buffer.h | 10 ++-- src/callproc.c | 5 +- src/coding.c | 12 ++--- src/coding.h | 3 -- src/fileio.c | 2 +- src/minibuf.c | 2 +- src/print.c | 2 +- src/process.c | 12 ++--- src/w32fns.c | 2 +- src/window.c | 7 +-- src/xdisp.c | 4 +- src/xfns.c | 2 +- src/xwidget.c | 3 +- test/src/buffer-tests.el | 33 +++++++++++-- 30 files changed, 221 insertions(+), 129 deletions(-) diff --git a/doc/lispref/buffers.texi b/doc/lispref/buffers.texi index 2860343628..33eb23984d 100644 --- a/doc/lispref/buffers.texi +++ b/doc/lispref/buffers.texi @@ -225,13 +225,22 @@ Current Buffer @defmac with-temp-buffer body@dots{} @anchor{Definition of with-temp-buffer} -The @code{with-temp-buffer} macro evaluates the @var{body} forms -with a temporary buffer as the current buffer. It saves the identity of +The @code{with-temp-buffer} macro evaluates the @var{body} forms with +a temporary buffer as the current buffer. It saves the identity of the current buffer, creates a temporary buffer and makes it current, evaluates the @var{body} forms, and finally restores the previous -current buffer while killing the temporary buffer. By default, undo -information (@pxref{Undo}) is not recorded in the buffer created by -this macro (but @var{body} can enable that, if needed). +current buffer while killing the temporary buffer. + +@cindex undo in temporary buffers +@cindex @code{kill-buffer-hook} in temporary buffers +@cindex @code{kill-buffer-query-functions} in temporary buffers +@cindex @code{buffer-list-update-hook} in temporary buffers +By default, undo information (@pxref{Undo}) is not recorded in the +buffer created by this macro (but @var{body} can enable that, if +needed). The temporary buffer also does not run the hooks +@code{kill-buffer-hook}, @code{kill-buffer-query-functions} +(@pxref{Killing Buffers}), and @code{buffer-list-update-hook} +(@pxref{Buffer List}). The return value is the value of the last form in @var{body}. You can return the contents of the temporary buffer by using @@ -345,9 +354,9 @@ Buffer Names If the optional second argument @var{ignore} is non-@code{nil}, it should be a string, a potential buffer name. It means to consider -that potential buffer acceptable, if it is tried, even it is the name -of an existing buffer (which would normally be rejected). Thus, if -buffers named @samp{foo}, @samp{foo<2>}, @samp{foo<3>} and +that potential buffer acceptable, if it is tried, even if it is the +name of an existing buffer (which would normally be rejected). Thus, +if buffers named @samp{foo}, @samp{foo<2>}, @samp{foo<3>} and @samp{foo<4>} exist, @example @@ -932,13 +941,17 @@ Buffer List @defvar buffer-list-update-hook This is a normal hook run whenever the buffer list changes. Functions (implicitly) running this hook are @code{get-buffer-create} -(@pxref{Creating Buffers}), @code{rename-buffer} (@pxref{Buffer Names}), -@code{kill-buffer} (@pxref{Killing Buffers}), @code{bury-buffer} (see -above) and @code{select-window} (@pxref{Selecting Windows}). +(@pxref{Creating Buffers}), @code{rename-buffer} (@pxref{Buffer +Names}), @code{kill-buffer} (@pxref{Killing Buffers}), +@code{bury-buffer} (see above), and @code{select-window} +(@pxref{Selecting Windows}). This hook is not run for internal or +temporary buffers created by @code{get-buffer-create} or +@code{generate-new-buffer} with a non-@code{nil} argument +@var{inhibit-buffer-hooks}. Functions run by this hook should avoid calling @code{select-window} -with a nil @var{norecord} argument or @code{with-temp-buffer} since -either may lead to infinite recursion. +with a @code{nil} @var{norecord} argument since this may lead to +infinite recursion. @end defvar @node Creating Buffers @@ -951,12 +964,20 @@ Creating Buffers with the specified name; @code{generate-new-buffer} always creates a new buffer and gives it a unique name. + Both functions accept an optional argument @var{inhibit-buffer-hooks}. +If it is non-@code{nil}, the buffer they create does not run the hooks +@code{kill-buffer-hook}, @code{kill-buffer-query-functions} +(@pxref{Killing Buffers}), and @code{buffer-list-update-hook} +(@pxref{Buffer List}). This avoids slowing down internal or temporary +buffers that are never presented to users or passed on to other +applications. + Other functions you can use to create buffers include @code{with-output-to-temp-buffer} (@pxref{Temporary Displays}) and @code{create-file-buffer} (@pxref{Visiting Files}). Starting a subprocess can also create a buffer (@pxref{Processes}). -@defun get-buffer-create buffer-or-name +@defun get-buffer-create buffer-or-name &optional inhibit-buffer-hooks This function returns a buffer named @var{buffer-or-name}. The buffer returned does not become the current buffer---this function does not change which buffer is current. @@ -980,7 +1001,7 @@ Creating Buffers buffer initially disables undo information recording (@pxref{Undo}). @end defun -@defun generate-new-buffer name +@defun generate-new-buffer name &optional inhibit-buffer-hooks This function returns a newly created, empty buffer, but does not make it current. The name of the buffer is generated by passing @var{name} to the function @code{generate-new-buffer-name} (@pxref{Buffer @@ -1092,6 +1113,10 @@ Killing Buffers they are called. The idea of this feature is that these functions will ask for confirmation from the user. If any of them returns @code{nil}, @code{kill-buffer} spares the buffer's life. + +This hook is not run for internal or temporary buffers created by +@code{get-buffer-create} or @code{generate-new-buffer} with a +non-@code{nil} argument @var{inhibit-buffer-hooks}. @end defvar @defvar kill-buffer-hook @@ -1100,6 +1125,10 @@ Killing Buffers The buffer to be killed is current when the hook functions run. @xref{Hooks}. This variable is a permanent local, so its local binding is not cleared by changing major modes. + +This hook is not run for internal or temporary buffers created by +@code{get-buffer-create} or @code{generate-new-buffer} with a +non-@code{nil} argument @var{inhibit-buffer-hooks}. @end defvar @defopt buffer-offer-save diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi index d49ac42bb4..6949ca29c6 100644 --- a/doc/lispref/files.texi +++ b/doc/lispref/files.texi @@ -701,8 +701,11 @@ Writing to Files The current buffer is restored even in case of an abnormal exit via @code{throw} or error (@pxref{Nonlocal Exits}). -See also @code{with-temp-buffer} in @ref{Definition of -with-temp-buffer,, The Current Buffer}. +Like @code{with-temp-buffer} (@pxref{Definition of with-temp-buffer,, +Current Buffer}), the temporary buffer used by this macro does not run +the hooks @code{kill-buffer-hook}, @code{kill-buffer-query-functions} +(@pxref{Killing Buffers}), and @code{buffer-list-update-hook} +(@pxref{Buffer List}). @end defmac @node File Locks diff --git a/doc/lispref/internals.texi b/doc/lispref/internals.texi index fa3dacbb7a..0adbef33ca 100644 --- a/doc/lispref/internals.texi +++ b/doc/lispref/internals.texi @@ -2391,6 +2391,15 @@ Buffer Internals This flag indicates that redisplay optimizations should not be used to display this buffer. +@item inhibit_buffer_hooks +This flag indicates that the buffer should not run the hooks +@code{kill-buffer-hook}, @code{kill-buffer-query-functions} +(@pxref{Killing Buffers}), and @code{buffer-list-update-hook} +(@pxref{Buffer List}). It is set at buffer creation (@pxref{Creating +Buffers}), and avoids slowing down internal or temporary buffers, such +as those created by @code{with-temp-buffer} (@pxref{Definition of +with-temp-buffer,, Current Buffer}). + @item overlay_center This field holds the current overlay center position. @xref{Managing Overlays}. @@ -2404,8 +2413,6 @@ Buffer Internals and @code{overlays_after} is sorted in order of increasing beginning position. -@c FIXME? the following are now all Lisp_Object BUFFER_INTERNAL_FIELD (foo). - @item name A Lisp string that names the buffer. It is guaranteed to be unique. @xref{Buffer Names}. This and the following fields have their names diff --git a/doc/misc/gnus-faq.texi b/doc/misc/gnus-faq.texi index adb812f572..c30e80ff56 100644 --- a/doc/misc/gnus-faq.texi +++ b/doc/misc/gnus-faq.texi @@ -1523,10 +1523,10 @@ FAQ 5-8 @example (setq message-default-headers - (with-temp-buffer - (insert "X-Face: ") - (insert-file-contents "~/.xface") - (buffer-string))) + (with-temp-buffer + (insert "X-Face: ") + (insert-file-contents "~/.xface") + (buffer-string))) @end example @noindent diff --git a/etc/NEWS b/etc/NEWS index 4a8e70e6a6..1b4c21cb45 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -1821,6 +1821,13 @@ modifies the string's text properties; instead, it uses and returns a copy of the string. This helps avoid trouble when strings are shared or constants. ++++ +** Temporary buffers no longer run certain buffer hooks. +The macros 'with-temp-buffer' and 'with-temp-file' no longer run the +hooks 'kill-buffer-hook', 'kill-buffer-query-functions', and +'buffer-list-update-hook' for the temporary buffers they create. This +avoids slowing them down when a lot of these hooks are defined. + --- ** The obsolete function 'thread-alive-p' has been removed. @@ -2177,6 +2184,15 @@ Until it is solved you could ignore such errors by performing ** The error 'ftp-error' belongs also to category 'remote-file-error'. ++++ +** Buffers can now be created with certain hooks disabled. +The functions 'get-buffer-create' and 'generate-new-buffer' accept a +new optional argument 'inhibit-buffer-hooks'. If non-nil, the new +buffer does not run the hooks 'kill-buffer-hook', +'kill-buffer-query-functions', and 'buffer-list-update-hook'. This +avoids slowing down internal or temporary buffers that are never +presented to users or passed on to other applications. + \f * Changes in Emacs 28.1 on Non-Free Operating Systems diff --git a/lisp/files.el b/lisp/files.el index 093b5f92e5..70d451cccf 100644 --- a/lisp/files.el +++ b/lisp/files.el @@ -1850,6 +1850,10 @@ kill-buffer-hook The buffer being killed is current while the hook is running. See `kill-buffer'. +This hook is not run for internal or temporary buffers created by +`get-buffer-create' or `generate-new-buffer' with argument +INHIBIT-BUFFER-HOOKS non-nil. + Note: Be careful with let-binding this hook considering it is frequently used for cleanup.") @@ -1951,7 +1955,7 @@ create-file-buffer (let ((lastname (file-name-nondirectory filename))) (if (string= lastname "") (setq lastname filename)) - (generate-new-buffer (if (string-match-p "\\` " lastname) + (generate-new-buffer (if (string-prefix-p " " lastname) (concat "|" lastname) lastname)))) diff --git a/lisp/gnus/gnus-uu.el b/lisp/gnus/gnus-uu.el index 70aeac00d7..00ca430cc5 100644 --- a/lisp/gnus/gnus-uu.el +++ b/lisp/gnus/gnus-uu.el @@ -1587,8 +1587,7 @@ gnus-uu-unshar-article (save-excursion (switch-to-buffer (current-buffer)) (delete-other-windows) - (let ((buffer (get-buffer-create (generate-new-buffer-name - "*Warning*")))) + (let ((buffer (generate-new-buffer "*Warning*"))) (unless (unwind-protect (with-current-buffer buffer diff --git a/lisp/international/mule.el b/lisp/international/mule.el index 212e7232b4..6571454dff 100644 --- a/lisp/international/mule.el +++ b/lisp/international/mule.el @@ -307,12 +307,9 @@ load-with-code-conversion (and (null noerror) (signal 'file-error (list "Cannot open load file" file))) ;; Read file with code conversion, and then eval. - (let* ((buffer - ;; We can't use `generate-new-buffer' because files.el - ;; is not yet loaded. - (get-buffer-create (generate-new-buffer-name " *load*"))) - (load-in-progress t) - (source (save-match-data (string-match "\\.el\\'" fullname)))) + (let ((buffer (generate-new-buffer " *load*")) + (load-in-progress t) + (source (string-suffix-p ".el" fullname))) (unless nomessage (if source (message "Loading %s (source)..." file) diff --git a/lisp/json.el b/lisp/json.el index c2fc1574fa..5f512b94cd 100644 --- a/lisp/json.el +++ b/lisp/json.el @@ -435,7 +435,7 @@ json-encode-string (concat "\"" (substring-no-properties string) "\"") (with-current-buffer (or json--string-buffer - (with-current-buffer (generate-new-buffer " *json-string*") + (with-current-buffer (generate-new-buffer " *json-string*" t) ;; This seems to afford decent performance gains. (setq-local inhibit-modification-hooks t) (setq json--string-buffer (current-buffer)))) diff --git a/lisp/mh-e/mh-xface.el b/lisp/mh-e/mh-xface.el index 909f1fe95d..65039310e7 100644 --- a/lisp/mh-e/mh-xface.el +++ b/lisp/mh-e/mh-xface.el @@ -425,8 +425,7 @@ mh-x-image-url-fetch-image be displayed in a buffer and position specified by MARKER. The actual display is carried out by the SENTINEL function." (if mh-wget-executable - (let ((buffer (get-buffer-create (generate-new-buffer-name - mh-temp-fetch-buffer))) + (let ((buffer (generate-new-buffer mh-temp-fetch-buffer)) (filename (or (mh-funcall-if-exists make-temp-file "mhe-fetch") (expand-file-name (make-temp-name "~/mhe-fetch"))))) (with-current-buffer buffer diff --git a/lisp/net/imap.el b/lisp/net/imap.el index 0394f0efea..50b08d9612 100644 --- a/lisp/net/imap.el +++ b/lisp/net/imap.el @@ -1033,8 +1033,7 @@ imap-open (when (funcall (nth 1 (assq stream imap-stream-alist)) buffer) ;; Stream changed? (if (not (eq imap-default-stream stream)) - (with-current-buffer (get-buffer-create - (generate-new-buffer-name " *temp*")) + (with-current-buffer (generate-new-buffer " *temp*") (mapc 'make-local-variable imap-local-variables) (set-buffer-multibyte nil) (buffer-disable-undo) diff --git a/lisp/net/mailcap.el b/lisp/net/mailcap.el index d0f8c1272d..bc99f02fe3 100644 --- a/lisp/net/mailcap.el +++ b/lisp/net/mailcap.el @@ -386,8 +386,7 @@ mailcap-maybe-eval (when (save-window-excursion (delete-other-windows) - (let ((buffer (get-buffer-create (generate-new-buffer-name - "*Warning*")))) + (let ((buffer (generate-new-buffer "*Warning*"))) (unwind-protect (with-current-buffer buffer (insert (substitute-command-keys diff --git a/lisp/progmodes/flymake-proc.el b/lisp/progmodes/flymake-proc.el index 152dc725c7..e9d5018f30 100644 --- a/lisp/progmodes/flymake-proc.el +++ b/lisp/progmodes/flymake-proc.el @@ -429,16 +429,15 @@ flymake-proc--replace-region (defun flymake-proc--read-file-to-temp-buffer (file-name) "Insert contents of FILE-NAME into newly created temp buffer." - (let* ((temp-buffer (get-buffer-create (generate-new-buffer-name (concat "flymake:" (file-name-nondirectory file-name)))))) - (with-current-buffer temp-buffer - (insert-file-contents file-name)) - temp-buffer)) + (with-current-buffer (generate-new-buffer + (concat "flymake:" (file-name-nondirectory file-name))) + (insert-file-contents file-name) + (current-buffer))) (defun flymake-proc--copy-buffer-to-temp-buffer (buffer) "Copy contents of BUFFER into newly created temp buffer." - (with-current-buffer - (get-buffer-create (generate-new-buffer-name - (concat "flymake:" (buffer-name buffer)))) + (with-current-buffer (generate-new-buffer + (concat "flymake:" (buffer-name buffer))) (insert-buffer-substring buffer) (current-buffer))) diff --git a/lisp/simple.el b/lisp/simple.el index 090162b973..aab650e98a 100644 --- a/lisp/simple.el +++ b/lisp/simple.el @@ -4306,8 +4306,7 @@ shell-command-on-region (defun shell-command-to-string (command) "Execute shell command COMMAND and return its output as a string." (with-output-to-string - (with-current-buffer - standard-output + (with-current-buffer standard-output (shell-command command t)))) (defun process-file (program &optional infile buffer display &rest args) diff --git a/lisp/subr.el b/lisp/subr.el index 77c19c5bbf..6d5d7b4bac 100644 --- a/lisp/subr.el +++ b/lisp/subr.el @@ -3701,10 +3701,11 @@ internal--after-with-selected-window (when (window-live-p (nth 1 state)) (select-window (nth 1 state) 'norecord))) -(defun generate-new-buffer (name) +(defun generate-new-buffer (name &optional inhibit-buffer-hooks) "Create and return a buffer with a name based on NAME. -Choose the buffer's name using `generate-new-buffer-name'." - (get-buffer-create (generate-new-buffer-name name))) +Choose the buffer's name using `generate-new-buffer-name'. +See `get-buffer-create' for the meaning of INHIBIT-BUFFER-HOOKS." + (get-buffer-create (generate-new-buffer-name name) inhibit-buffer-hooks)) (defmacro with-selected-window (window &rest body) "Execute the forms in BODY with WINDOW as the selected window. @@ -3866,12 +3867,14 @@ with-output-to-temp-buffer (defmacro with-temp-file (file &rest body) "Create a new buffer, evaluate BODY there, and write the buffer to FILE. The value returned is the value of the last form in BODY. +The buffer does not run the hooks `kill-buffer-hook', +`kill-buffer-query-functions', and `buffer-list-update-hook'. See also `with-temp-buffer'." (declare (indent 1) (debug t)) (let ((temp-file (make-symbol "temp-file")) (temp-buffer (make-symbol "temp-buffer"))) `(let ((,temp-file ,file) - (,temp-buffer (generate-new-buffer " *temp file*"))) + (,temp-buffer (generate-new-buffer " *temp file*" t))) (unwind-protect (prog1 (with-current-buffer ,temp-buffer @@ -3906,10 +3909,12 @@ with-temp-message (defmacro with-temp-buffer (&rest body) "Create a temporary buffer, and evaluate BODY there like `progn'. +The buffer does not run the hooks `kill-buffer-hook', +`kill-buffer-query-functions', and `buffer-list-update-hook'. See also `with-temp-file' and `with-output-to-string'." (declare (indent 0) (debug t)) (let ((temp-buffer (make-symbol "temp-buffer"))) - `(let ((,temp-buffer (generate-new-buffer " *temp*"))) + `(let ((,temp-buffer (generate-new-buffer " *temp*" t))) ;; `kill-buffer' can change current-buffer in some odd cases. (with-current-buffer ,temp-buffer (unwind-protect @@ -3944,7 +3949,7 @@ with-silent-modifications (defmacro with-output-to-string (&rest body) "Execute BODY, return the text it sent to `standard-output', as a string." (declare (indent 0) (debug t)) - `(let ((standard-output (generate-new-buffer " *string-output*"))) + `(let ((standard-output (generate-new-buffer " *string-output*" t))) (unwind-protect (progn (let ((standard-output standard-output)) diff --git a/src/buffer.c b/src/buffer.c index dfc34faf6e..9e44345616 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -37,7 +37,6 @@ Copyright (C) 1985-1989, 1993-1995, 1997-2020 Free Software Foundation, #include "window.h" #include "commands.h" #include "character.h" -#include "coding.h" #include "buffer.h" #include "region-cache.h" #include "indent.h" @@ -514,16 +513,33 @@ get_truename_buffer (register Lisp_Object filename) return Qnil; } -DEFUN ("get-buffer-create", Fget_buffer_create, Sget_buffer_create, 1, 1, 0, +/* Run buffer-list-update-hook if Vrun_hooks is non-nil, and BUF is NULL + or does not have buffer hooks inhibited. BUF is NULL when called by + make-indirect-buffer, since it does not inhibit buffer hooks. */ + +static void +run_buffer_list_update_hook (struct buffer *buf) +{ + if (! (NILP (Vrun_hooks) || (buf && buf->inhibit_buffer_hooks))) + call1 (Vrun_hooks, Qbuffer_list_update_hook); +} + +DEFUN ("get-buffer-create", Fget_buffer_create, Sget_buffer_create, 1, 2, 0, doc: /* Return the buffer specified by BUFFER-OR-NAME, creating a new one if needed. If BUFFER-OR-NAME is a string and a live buffer with that name exists, return that buffer. If no such buffer exists, create a new buffer with -that name and return it. If BUFFER-OR-NAME starts with a space, the new -buffer does not keep undo information. +that name and return it. + +If BUFFER-OR-NAME starts with a space, the new buffer does not keep undo +information. If optional argument INHIBIT-BUFFER-HOOKS is non-nil, the +new buffer does not run the hooks `kill-buffer-hook', +`kill-buffer-query-functions', and `buffer-list-update-hook'. This +avoids slowing down internal or temporary buffers that are never +presented to users or passed on to other applications. If BUFFER-OR-NAME is a buffer instead of a string, return it as given, even if it is dead. The return value is never nil. */) - (register Lisp_Object buffer_or_name) + (register Lisp_Object buffer_or_name, Lisp_Object inhibit_buffer_hooks) { register Lisp_Object buffer, name; register struct buffer *b; @@ -598,11 +614,7 @@ DEFUN ("get-buffer-create", Fget_buffer_create, Sget_buffer_create, 1, 1, 0, set_string_intervals (name, NULL); bset_name (b, name); - b->inhibit_buffer_hooks - = (STRINGP (Vcode_conversion_workbuf_name) - && strncmp (SSDATA (name), SSDATA (Vcode_conversion_workbuf_name), - SBYTES (Vcode_conversion_workbuf_name)) == 0); - + b->inhibit_buffer_hooks = !NILP (inhibit_buffer_hooks); bset_undo_list (b, SREF (name, 0) != ' ' ? Qnil : Qt); reset_buffer (b); @@ -614,9 +626,8 @@ DEFUN ("get-buffer-create", Fget_buffer_create, Sget_buffer_create, 1, 1, 0, /* Put this in the alist of all live buffers. */ XSETBUFFER (buffer, b); Vbuffer_alist = nconc2 (Vbuffer_alist, list1 (Fcons (name, buffer))); - /* And run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !b->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + + run_buffer_list_update_hook (b); return buffer; } @@ -890,9 +901,7 @@ DEFUN ("make-indirect-buffer", Fmake_indirect_buffer, Smake_indirect_buffer, set_buffer_internal_1 (old_b); } - /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks)) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (NULL); return buf; } @@ -1536,9 +1545,7 @@ DEFUN ("rename-buffer", Frename_buffer, Srename_buffer, 1, 2, && !NILP (BVAR (current_buffer, auto_save_file_name))) call0 (intern ("rename-auto-save-file")); - /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !current_buffer->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (current_buffer); /* Refetch since that last call may have done GC. */ return BVAR (current_buffer, name); @@ -1612,7 +1619,7 @@ DEFUN ("other-buffer", Fother_buffer, Sother_buffer, 0, 3, 0, buf = Fget_buffer (scratch); if (NILP (buf)) { - buf = Fget_buffer_create (scratch); + buf = Fget_buffer_create (scratch, Qnil); Fset_buffer_major_mode (buf); } return buf; @@ -1636,7 +1643,7 @@ other_buffer_safely (Lisp_Object buffer) buf = Fget_buffer (scratch); if (NILP (buf)) { - buf = Fget_buffer_create (scratch); + buf = Fget_buffer_create (scratch, Qnil); Fset_buffer_major_mode (buf); } @@ -1713,7 +1720,9 @@ DEFUN ("kill-buffer", Fkill_buffer, Skill_buffer, 0, 1, "bKill buffer: ", the buffer is not killed. The hook `kill-buffer-hook' is run before the buffer is actually killed. The buffer being killed will be current while the hook is running. Functions called by any of these hooks are -supposed to not change the current buffer. +supposed to not change the current buffer. Neither hook is run for +internal or temporary buffers created by `get-buffer-create' or +`generate-new-buffer' with argument INHIBIT-BUFFER-HOOKS non-nil. Any processes that have this buffer as the `process-buffer' are killed with SIGHUP. This function calls `replace-buffer-in-windows' for @@ -1973,9 +1982,7 @@ DEFUN ("kill-buffer", Fkill_buffer, Skill_buffer, 0, 1, "bKill buffer: ", bset_width_table (b, Qnil); unblock_input (); - /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !b->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (b); return Qt; } @@ -2015,9 +2022,7 @@ record_buffer (Lisp_Object buffer) fset_buffer_list (f, Fcons (buffer, Fdelq (buffer, f->buffer_list))); fset_buried_buffer_list (f, Fdelq (buffer, f->buried_buffer_list)); - /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !XBUFFER (buffer)->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (XBUFFER (buffer)); } @@ -2054,9 +2059,7 @@ DEFUN ("bury-buffer-internal", Fbury_buffer_internal, Sbury_buffer_internal, fset_buried_buffer_list (f, Fcons (buffer, Fdelq (buffer, f->buried_buffer_list))); - /* Run buffer-list-update-hook. */ - if (!NILP (Vrun_hooks) && !XBUFFER (buffer)->inhibit_buffer_hooks) - call1 (Vrun_hooks, Qbuffer_list_update_hook); + run_buffer_list_update_hook (XBUFFER (buffer)); return Qnil; } @@ -5349,10 +5352,11 @@ init_buffer_once (void) Fput (Qkill_buffer_hook, Qpermanent_local, Qt); /* Super-magic invisible buffer. */ - Vprin1_to_string_buffer = Fget_buffer_create (build_pure_c_string (" prin1")); + Vprin1_to_string_buffer = + Fget_buffer_create (build_pure_c_string (" prin1"), Qt); Vbuffer_alist = Qnil; - Fset_buffer (Fget_buffer_create (build_pure_c_string ("*scratch*"))); + Fset_buffer (Fget_buffer_create (build_pure_c_string ("*scratch*"), Qnil)); inhibit_modification_hooks = 0; } @@ -5397,7 +5401,7 @@ init_buffer (void) #endif /* USE_MMAP_FOR_BUFFERS */ AUTO_STRING (scratch, "*scratch*"); - Fset_buffer (Fget_buffer_create (scratch)); + Fset_buffer (Fget_buffer_create (scratch, Qnil)); if (NILP (BVAR (&buffer_defaults, enable_multibyte_characters))) Fset_buffer_multibyte (Qnil); @@ -6300,9 +6304,14 @@ from (abs POSITION). If POSITION is positive, point was at the front DEFVAR_LISP ("kill-buffer-query-functions", Vkill_buffer_query_functions, doc: /* List of functions called with no args to query before killing a buffer. The buffer being killed will be current while the functions are running. +See `kill-buffer'. If any of them returns nil, the buffer is not killed. Functions run by -this hook are supposed to not change the current buffer. */); +this hook are supposed to not change the current buffer. + +This hook is not run for internal or temporary buffers created by +`get-buffer-create' or `generate-new-buffer' with argument +INHIBIT-BUFFER-HOOKS non-nil. */); Vkill_buffer_query_functions = Qnil; DEFVAR_LISP ("change-major-mode-hook", Vchange_major_mode_hook, @@ -6315,9 +6324,12 @@ from (abs POSITION). If POSITION is positive, point was at the front doc: /* Hook run when the buffer list changes. Functions (implicitly) running this hook are `get-buffer-create', `make-indirect-buffer', `rename-buffer', `kill-buffer', `bury-buffer' -and `select-window'. Functions run by this hook should avoid calling -`select-window' with a nil NORECORD argument or `with-temp-buffer' -since either may lead to infinite recursion. */); +and `select-window'. This hook is not run for internal or temporary +buffers created by `get-buffer-create' or `generate-new-buffer' with +argument INHIBIT-BUFFER-HOOKS non-nil. + +Functions run by this hook should avoid calling `select-window' with a +nil NORECORD argument since it may lead to infinite recursion. */); Vbuffer_list_update_hook = Qnil; DEFSYM (Qbuffer_list_update_hook, "buffer-list-update-hook"); diff --git a/src/buffer.h b/src/buffer.h index fe549c5dac..b8c5162be4 100644 --- a/src/buffer.h +++ b/src/buffer.h @@ -663,11 +663,11 @@ #define BVAR(buf, field) ((buf)->field ## _) /* Non-zero whenever the narrowing is changed in this buffer. */ bool_bf clip_changed : 1; - /* Non-zero for internally used temporary buffers that don't need to - run hooks kill-buffer-hook, buffer-list-update-hook, and - kill-buffer-query-functions. This is used in coding.c to avoid - slowing down en/decoding when there are a lot of these hooks - defined. */ + /* Non-zero for internal or temporary buffers that don't need to + run hooks kill-buffer-hook, kill-buffer-query-functions, and + buffer-list-update-hook. This is used in coding.c to avoid + slowing down en/decoding when a lot of these hooks are + defined, as well as by with-temp-buffer, for example. */ bool_bf inhibit_buffer_hooks : 1; /* List of overlays that end at or before the current center, diff --git a/src/callproc.c b/src/callproc.c index e3346e2eab..4bca1e5ebd 100644 --- a/src/callproc.c +++ b/src/callproc.c @@ -405,9 +405,8 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd, if (! (NILP (buffer) || EQ (buffer, Qt) || FIXNUMP (buffer))) { - Lisp_Object spec_buffer; - spec_buffer = buffer; - buffer = Fget_buffer_create (buffer); + Lisp_Object spec_buffer = buffer; + buffer = Fget_buffer_create (buffer, Qnil); /* Mention the buffer name for a better error message. */ if (NILP (buffer)) CHECK_BUFFER (spec_buffer); diff --git a/src/coding.c b/src/coding.c index 2142e7fa51..1afa4aa474 100644 --- a/src/coding.c +++ b/src/coding.c @@ -7821,7 +7821,7 @@ encode_coding (struct coding_system *coding) /* A string that serves as name of the reusable work buffer, and as base name of temporary work buffers used for code-conversion operations. */ -Lisp_Object Vcode_conversion_workbuf_name; +static Lisp_Object Vcode_conversion_workbuf_name; /* The reusable working buffer, created once and never killed. */ static Lisp_Object Vcode_conversion_reused_workbuf; @@ -7839,7 +7839,7 @@ code_conversion_restore (Lisp_Object arg) if (! NILP (workbuf)) { if (EQ (workbuf, Vcode_conversion_reused_workbuf)) - reused_workbuf_in_use = 0; + reused_workbuf_in_use = false; else Fkill_buffer (workbuf); } @@ -7857,13 +7857,13 @@ code_conversion_save (bool with_work_buf, bool multibyte) { Lisp_Object name = Fgenerate_new_buffer_name (Vcode_conversion_workbuf_name, Qnil); - workbuf = Fget_buffer_create (name); + workbuf = Fget_buffer_create (name, Qt); } else { if (NILP (Fbuffer_live_p (Vcode_conversion_reused_workbuf))) Vcode_conversion_reused_workbuf - = Fget_buffer_create (Vcode_conversion_workbuf_name); + = Fget_buffer_create (Vcode_conversion_workbuf_name, Qt); workbuf = Vcode_conversion_reused_workbuf; } } @@ -7881,7 +7881,7 @@ code_conversion_save (bool with_work_buf, bool multibyte) bset_undo_list (current_buffer, Qt); bset_enable_multibyte_characters (current_buffer, multibyte ? Qt : Qnil); if (EQ (workbuf, Vcode_conversion_reused_workbuf)) - reused_workbuf_in_use = 1; + reused_workbuf_in_use = true; set_buffer_internal (current); } @@ -11639,7 +11639,7 @@ syms_of_coding (void) staticpro (&Vcode_conversion_workbuf_name); Vcode_conversion_workbuf_name = build_pure_c_string (" *code-conversion-work*"); - reused_workbuf_in_use = 0; + reused_workbuf_in_use = false; PDUMPER_REMEMBER_SCALAR (reused_workbuf_in_use); DEFSYM (Qcharset, "charset"); diff --git a/src/coding.h b/src/coding.h index 4973cf89eb..9ad1e954f8 100644 --- a/src/coding.h +++ b/src/coding.h @@ -97,9 +97,6 @@ #define EMACS_CODING_H extern Lisp_Object Vcoding_system_hash_table; -/* Name (or base name) of work buffer for code conversion. */ -extern Lisp_Object Vcode_conversion_workbuf_name; - /* Enumeration of index to an attribute vector of a coding system. */ enum coding_attr_index diff --git a/src/fileio.c b/src/fileio.c index c97f4daf20..51f12e104e 100644 --- a/src/fileio.c +++ b/src/fileio.c @@ -4004,7 +4004,7 @@ because (1) it preserves some marker positions (in unchanged portions record_unwind_current_buffer (); - workbuf = Fget_buffer_create (name); + workbuf = Fget_buffer_create (name, Qt); buf = XBUFFER (workbuf); delete_all_overlays (buf); diff --git a/src/minibuf.c b/src/minibuf.c index fc3fd92a88..1940564a80 100644 --- a/src/minibuf.c +++ b/src/minibuf.c @@ -809,7 +809,7 @@ get_minibuffer (EMACS_INT depth) static char const name_fmt[] = " *Minibuf-%"pI"d*"; char name[sizeof name_fmt + INT_STRLEN_BOUND (EMACS_INT)]; AUTO_STRING_WITH_LEN (lname, name, sprintf (name, name_fmt, depth)); - buf = Fget_buffer_create (lname); + buf = Fget_buffer_create (lname, Qnil); /* Although the buffer's name starts with a space, undo should be enabled in it. */ diff --git a/src/print.c b/src/print.c index 008bf5e639..ec271d914c 100644 --- a/src/print.c +++ b/src/print.c @@ -562,7 +562,7 @@ temp_output_buffer_setup (const char *bufname) record_unwind_current_buffer (); - Fset_buffer (Fget_buffer_create (build_string (bufname))); + Fset_buffer (Fget_buffer_create (build_string (bufname), Qnil)); Fkill_all_local_variables (); delete_all_overlays (current_buffer); diff --git a/src/process.c b/src/process.c index 4fe8ac7fc0..9efefb1de7 100644 --- a/src/process.c +++ b/src/process.c @@ -1731,7 +1731,7 @@ DEFUN ("make-process", Fmake_process, Smake_process, 0, MANY, 0, buffer = Fplist_get (contact, QCbuffer); if (!NILP (buffer)) - buffer = Fget_buffer_create (buffer); + buffer = Fget_buffer_create (buffer, Qnil); /* Make sure that the child will be able to chdir to the current buffer's current directory, or its unhandled equivalent. We @@ -1768,7 +1768,7 @@ DEFUN ("make-process", Fmake_process, Smake_process, 0, MANY, 0, QCname, concat2 (name, build_string (" stderr")), QCbuffer, - Fget_buffer_create (xstderr), + Fget_buffer_create (xstderr, Qnil), QCnoquery, query_on_exit ? Qnil : Qt); } @@ -2443,7 +2443,7 @@ DEFUN ("make-pipe-process", Fmake_pipe_process, Smake_pipe_process, buffer = Fplist_get (contact, QCbuffer); if (NILP (buffer)) buffer = name; - buffer = Fget_buffer_create (buffer); + buffer = Fget_buffer_create (buffer, Qnil); pset_buffer (p, buffer); pset_childp (p, contact); @@ -3173,7 +3173,7 @@ DEFUN ("make-serial-process", Fmake_serial_process, Smake_serial_process, buffer = Fplist_get (contact, QCbuffer); if (NILP (buffer)) buffer = name; - buffer = Fget_buffer_create (buffer); + buffer = Fget_buffer_create (buffer, Qnil); pset_buffer (p, buffer); pset_childp (p, contact); @@ -4188,7 +4188,7 @@ DEFUN ("make-network-process", Fmake_network_process, Smake_network_process, open_socket: if (!NILP (buffer)) - buffer = Fget_buffer_create (buffer); + buffer = Fget_buffer_create (buffer, Qnil); /* Unwind bind_polling_period. */ unbind_to (count, Qnil); @@ -4961,7 +4961,7 @@ server_accept_connection (Lisp_Object server, int channel) if (!NILP (buffer)) { args[1] = buffer; - buffer = Fget_buffer_create (Fformat (nargs, args)); + buffer = Fget_buffer_create (Fformat (nargs, args), Qnil); } } diff --git a/src/w32fns.c b/src/w32fns.c index a840f0e122..36bee0676b 100644 --- a/src/w32fns.c +++ b/src/w32fns.c @@ -7372,7 +7372,7 @@ DEFUN ("x-show-tip", Fx_show_tip, Sx_show_tip, 1, 6, 0, tip_f = XFRAME (tip_frame); window = FRAME_ROOT_WINDOW (tip_f); - tip_buf = Fget_buffer_create (tip); + tip_buf = Fget_buffer_create (tip, Qnil); /* We will mark the tip window a "pseudo-window" below, and such windows cannot have display margins. */ bset_left_margin_cols (XBUFFER (tip_buf), make_fixnum (0)); diff --git a/src/window.c b/src/window.c index bcc989b5a7..5db166e345 100644 --- a/src/window.c +++ b/src/window.c @@ -617,11 +617,12 @@ DEFUN ("select-window", Fselect_window, Sselect_window, 1, 2, 0, Run `buffer-list-update-hook' unless NORECORD is non-nil. Note that applications and internal routines often select a window temporarily for various purposes; mostly, to simplify coding. As a rule, such -selections should be not recorded and therefore will not pollute +selections should not be recorded and therefore will not pollute `buffer-list-update-hook'. Selections that "really count" are those causing a visible change in the next redisplay of WINDOW's frame and -should be always recorded. So if you think of running a function each -time a window gets selected put it on `buffer-list-update-hook'. +should always be recorded. So if you think of running a function each +time a window gets selected, put it on `buffer-list-update-hook' or +`window-selection-change-functions'. Also note that the main editor command loop sets the current buffer to the buffer of the selected window before each command. */) diff --git a/src/xdisp.c b/src/xdisp.c index 0fd5ec5ec5..b5adee5105 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -10880,7 +10880,7 @@ message_dolog (const char *m, ptrdiff_t nbytes, bool nlflag, bool multibyte) /* Ensure the Messages buffer exists, and switch to it. If we created it, set the major-mode. */ bool newbuffer = NILP (Fget_buffer (Vmessages_buffer_name)); - Fset_buffer (Fget_buffer_create (Vmessages_buffer_name)); + Fset_buffer (Fget_buffer_create (Vmessages_buffer_name, Qnil)); if (newbuffer && !NILP (Ffboundp (intern ("messages-buffer-mode")))) call0 (intern ("messages-buffer-mode")); @@ -11366,7 +11366,7 @@ ensure_echo_area_buffers (void) static char const name_fmt[] = " *Echo Area %d*"; char name[sizeof name_fmt + INT_STRLEN_BOUND (int)]; AUTO_STRING_WITH_LEN (lname, name, sprintf (name, name_fmt, i)); - echo_buffer[i] = Fget_buffer_create (lname); + echo_buffer[i] = Fget_buffer_create (lname, Qnil); bset_truncate_lines (XBUFFER (echo_buffer[i]), Qnil); /* to force word wrap in echo area - it was decided to postpone this*/ diff --git a/src/xfns.c b/src/xfns.c index 46e4bd73a6..abe293e903 100644 --- a/src/xfns.c +++ b/src/xfns.c @@ -7041,7 +7041,7 @@ DEFUN ("x-show-tip", Fx_show_tip, Sx_show_tip, 1, 6, 0, tip_f = XFRAME (tip_frame); window = FRAME_ROOT_WINDOW (tip_f); - tip_buf = Fget_buffer_create (tip); + tip_buf = Fget_buffer_create (tip, Qnil); /* We will mark the tip window a "pseudo-window" below, and such windows cannot have display margins. */ bset_left_margin_cols (XBUFFER (tip_buf), make_fixnum (0)); diff --git a/src/xwidget.c b/src/xwidget.c index e078a28a35..accde65b52 100644 --- a/src/xwidget.c +++ b/src/xwidget.c @@ -100,7 +100,8 @@ DEFUN ("make-xwidget", Lisp_Object val; xw->type = type; xw->title = title; - xw->buffer = NILP (buffer) ? Fcurrent_buffer () : Fget_buffer_create (buffer); + xw->buffer = (NILP (buffer) ? Fcurrent_buffer () + : Fget_buffer_create (buffer, Qnil)); xw->height = XFIXNAT (height); xw->width = XFIXNAT (width); xw->kill_without_query = false; diff --git a/test/src/buffer-tests.el b/test/src/buffer-tests.el index 0db66f9751..dd8927457a 100644 --- a/test/src/buffer-tests.el +++ b/test/src/buffer-tests.el @@ -19,9 +19,7 @@ ;;; Code: -(require 'ert) -(require 'seq) -(eval-when-compile (require 'cl-lib)) +(require 'cl-lib) (ert-deftest overlay-modification-hooks-message-other-buf () "Test for bug#21824. @@ -1334,4 +1332,33 @@ buffer-tests-buffer-local-variables-undo (with-temp-buffer (should (assq 'buffer-undo-list (buffer-local-variables))))) +(ert-deftest buffer-tests-inhibit-buffer-hooks () + "Test `get-buffer-create' argument INHIBIT-BUFFER-HOOKS." + (let* (run-bluh (bluh (lambda () (setq run-bluh t)))) + (unwind-protect + (let* ( run-kbh (kbh (lambda () (setq run-kbh t))) + run-kbqf (kbqf (lambda () (setq run-kbqf t))) ) + + ;; Inhibited. + (add-hook 'buffer-list-update-hook bluh) + (with-current-buffer (generate-new-buffer " foo" t) + (add-hook 'kill-buffer-hook kbh nil t) + (add-hook 'kill-buffer-query-functions kbqf nil t) + (kill-buffer)) + (with-temp-buffer) + (with-output-to-string) + (should-not run-bluh) + (should-not run-kbh) + (should-not run-kbqf) + + ;; Not inhibited. + (with-current-buffer (generate-new-buffer " foo") + (should run-bluh) + (add-hook 'kill-buffer-hook kbh nil t) + (add-hook 'kill-buffer-query-functions kbqf nil t) + (kill-buffer)) + (should run-kbh) + (should run-kbqf)) + (remove-hook 'buffer-list-update-hook bluh)))) + ;;; buffer-tests.el ends here -- 2.29.2 ^ permalink raw reply related [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2020-12-19 21:10 ` Basil L. Contovounesios @ 2020-12-20 15:05 ` Eli Zaretskii 0 siblings, 0 replies; 72+ messages in thread From: Eli Zaretskii @ 2020-12-20 15:05 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: 34765, larsi, monnier, alexanderm > From: "Basil L. Contovounesios" <contovob@tcd.ie> > Cc: monnier@iro.umontreal.ca, rudalics@gmx.at, larsi@gnus.org, > 34765@debbugs.gnu.org, alexanderm@web.de > Date: Sat, 19 Dec 2020 21:10:13 +0000 > > >> The printed label "see Current Buffer" should be displayed instead of > >> this word, which is part of the anchor. Is that okay? > > > > Sorry, I didn't see that this is an anchor. So I think the anchor > > should not start with a capital letter, as it reads more naturally > > that way, I think. And then the name should be used without > > capitalization in the cross-references. Obviously, this is a minor > > nit. > > I'm hesitant to change the anchor because all 37 such "Definition of..." > anchors in the tree are capitalised, as are the printed labels of all > their refs. > > I'm happy to downcase them, but maybe this should be done wholesale in a > separate commit? That's okay, let's leave the anchor capitalization alone for now. > >> The alternatives would be for Fmake_indirect_buffer to not call > >> run_buffer_list_update_hook, or to not bother adding > >> run_buffer_list_update_hook at all. Do you have a preference? > > > > I think just having a comment there saying that make-indirect-buffer > > calls this with NULL argument should be okay. > > How's the attached? LGTM, thanks. ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-03-05 22:57 bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook Alexander Miller 2019-03-06 9:39 ` martin rudalics 2019-03-06 11:29 ` Alexander Miller @ 2019-03-07 6:18 ` Alexander Miller 2019-03-07 8:29 ` martin rudalics 2020-12-20 17:57 ` Basil L. Contovounesios 3 siblings, 1 reply; 72+ messages in thread From: Alexander Miller @ 2019-03-07 6:18 UTC (permalink / raw) To: rudalics; +Cc: 34765, alexanderm > And do not preserve NORECORD? They did. > Maybe magit should simply try to reuse the same temporary buffer > instead of recreating it excessively. Creating/killing temporary > buffers does not come without some overhead. It didn't. IIRC a temp buffer was created 2 or 3 times every time I would move point. The problem is that this *somehow* triggered a feedback loop with treemacs and magit feeding off each other. I haven't really understood what happened there as that would require some very deep digging inside magit's internals. There's a recipe for emacs -q in the github issue if you want to see this for yourself. > I see. But please try 'window-selection-change-functions' > sooner or later so we know whether it fixes these problems. Way ahead of you ;) https://github.com/Alexander-Miller/treemacs/issues/321 ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-03-07 6:18 ` Alexander Miller @ 2019-03-07 8:29 ` martin rudalics 2019-03-07 9:44 ` Alexander Miller 0 siblings, 1 reply; 72+ messages in thread From: martin rudalics @ 2019-03-07 8:29 UTC (permalink / raw) To: Alexander Miller; +Cc: 34765 > > And do not preserve NORECORD? > > They did. So what was the problem there? How did 'buffer-list-update-hook' get invoked in such a wild manner? > It didn't. IIRC a temp buffer was created 2 or 3 times every time > I would move point. This alone qualifies as a bug IMHO. > The problem is that this *somehow* triggered a > feedback loop with treemacs and magit feeding off each other. I > haven't really understood what happened there as that would require > some very deep digging inside magit's internals. I think that creating a new temporary buffer should always be the exception and never the rule. Does the point moving happen in the treemacs buffer or in that of a plain (probably git-controlled) buffer? > There's a recipe for > emacs -q in the github issue if you want to see this for yourself. Can you pass me a corresponding URL? > > I see. But please try 'window-selection-change-functions' > > sooner or later so we know whether it fixes these problems. > > Way ahead of you ;) > https://github.com/Alexander-Miller/treemacs/issues/321 Aha. martin ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-03-07 8:29 ` martin rudalics @ 2019-03-07 9:44 ` Alexander Miller 2019-03-07 13:46 ` martin rudalics 0 siblings, 1 reply; 72+ messages in thread From: Alexander Miller @ 2019-03-07 9:44 UTC (permalink / raw) To: martin rudalics; +Cc: 34765 > So what was the problem there? How did 'buffer-list-update-hook' get > invoked in such a wild manner As I understand it: advising select-window served the purpose of remembering the current window so the modeline can be highlighted in tune with the selection. The advising code would also call force-mode-line-update, that in turn lead to a re-calculation of the frame title, in spacemacs this could in turn call format-spec, that would use a temp-buffer, that triggers buffer-list-update-hook, that triggers treemacs' follow-mode callback, that calls with-selected-window, that leads back to the modeline's select-window advice. That's more or less what the feedback loop looked like. Come to think of it, maybe just suppressing the hook during my own window-selection will do the trick. > This alone qualifies as a bug IMHO. You'll have to take that up with Jonas. I linked this discussion in github, so he'll probably read this. > I think that creating a new temporary buffer should always be the > exception and never the rule. Does the point moving happen in the > treemacs buffer or in that of a plain (probably git-controlled) > buffer? You misunderstood. The issue only happened when the treemacs window was visible, but point was in magit's status buffer and a region was active (e.g. when you want to select single lines for staging or reverting). Non-magit buffers had no issues. > Can you pass me a corresponding URL? Did that already, Here it is again: https://github.com/magit/magit/issues/3738#issuecomment-464520582 ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-03-07 9:44 ` Alexander Miller @ 2019-03-07 13:46 ` martin rudalics 0 siblings, 0 replies; 72+ messages in thread From: martin rudalics @ 2019-03-07 13:46 UTC (permalink / raw) To: Alexander Miller; +Cc: 34765 > As I understand it: advising select-window served the purpose of > remembering the > current window so the modeline can be highlighted in tune with the > selection. The > advising code would also call force-mode-line-update, that in turn lead > to a re-calculation > of the frame title, in spacemacs this could in turn call format-spec, > that would use a > temp-buffer, that triggers buffer-list-update-hook, that triggers > treemacs' follow-mode > callback, that calls with-selected-window, that leads back to the > modeline's select-window > advice. That's more or less what the feedback loop looked like. Thanks. IIUC this means that we'll have to put an extra warning in the doc-string of 'buffer-list-update-hook' to not kill, rename or create buffers or select another window with NORECORD nil in its functions. I listed all functions calling that hook in the hope that people would then avoid that but your example shows that such endless recursions can be quite tricky to detect. One major design goal of the new window change functions was to simply not invoke them recursively so such scenarios should not happen there (and was immediately proven wrong by the redisplay mechanism itself). > > Can you pass me a corresponding URL? > > Did that already, Here it is again: > https://github.com/magit/magit/issues/3738#issuecomment-464520582 I thought you had another one (with more insight into how that problem triggered). But the scenario above is sufficient to imagine what can happen ... martin ^ permalink raw reply [flat|nested] 72+ messages in thread
* bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook 2019-03-05 22:57 bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook Alexander Miller ` (2 preceding siblings ...) 2019-03-07 6:18 ` Alexander Miller @ 2020-12-20 17:57 ` Basil L. Contovounesios 3 siblings, 0 replies; 72+ messages in thread From: Basil L. Contovounesios @ 2020-12-20 17:57 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 34765-done, larsi, monnier, alexanderm tags 34765 fixed close 34765 28.1 quit Eli Zaretskii <eliz@gnu.org> writes: >> From: "Basil L. Contovounesios" <contovob@tcd.ie> >> Cc: monnier@iro.umontreal.ca, rudalics@gmx.at, larsi@gnus.org, >> 34765@debbugs.gnu.org, alexanderm@web.de >> Date: Sat, 19 Dec 2020 21:10:13 +0000 >> >> How's the attached? > > LGTM, thanks. Thanks. Pushed to master, and closing. Inhibit buffer hooks in temporary buffers 1a0a11f7d2 2020-12-20 17:32:24 +0000 https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=1a0a11f7d2d1dbecb9f754b1e129d50e489058e6 -- Basil ^ permalink raw reply [flat|nested] 72+ messages in thread
end of thread, other threads:[~2020-12-20 17:57 UTC | newest] Thread overview: 72+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-03-05 22:57 bug#34765: 26.1; with-temp-buffer should not run buffer-list-update-hook Alexander Miller 2019-03-06 9:39 ` martin rudalics 2019-03-06 11:29 ` Alexander Miller 2019-03-06 14:13 ` martin rudalics 2019-03-06 15:41 ` Eli Zaretskii 2019-03-06 17:57 ` martin rudalics 2019-04-23 9:21 ` martin rudalics 2019-04-23 10:36 ` Eli Zaretskii 2019-04-24 7:27 ` martin rudalics 2019-04-24 11:12 ` Eli Zaretskii 2019-04-24 12:55 ` Stefan Monnier 2019-04-25 8:06 ` martin rudalics 2019-04-25 8:50 ` Eli Zaretskii 2019-04-25 10:31 ` martin rudalics 2019-04-25 10:49 ` Eli Zaretskii 2019-04-26 7:40 ` martin rudalics 2019-04-26 8:09 ` Eli Zaretskii 2019-04-25 13:01 ` Stefan Monnier 2019-04-25 14:34 ` Eli Zaretskii 2019-04-26 7:41 ` martin rudalics 2019-04-26 8:10 ` Eli Zaretskii 2019-04-26 7:41 ` martin rudalics 2019-04-26 8:10 ` Eli Zaretskii 2019-04-26 11:00 ` martin rudalics 2019-04-26 11:26 ` Eli Zaretskii 2019-04-27 8:30 ` martin rudalics 2019-04-26 17:14 ` Basil L. Contovounesios 2019-04-27 8:31 ` martin rudalics 2019-05-20 13:42 ` Basil L. Contovounesios 2019-05-21 7:32 ` martin rudalics 2019-05-21 7:58 ` Basil L. Contovounesios 2019-05-21 10:04 ` martin rudalics 2019-05-22 7:29 ` Eli Zaretskii 2019-05-22 8:32 ` martin rudalics 2019-05-22 10:06 ` Eli Zaretskii 2019-05-23 8:38 ` martin rudalics 2019-05-23 14:37 ` Eli Zaretskii 2019-05-24 8:01 ` martin rudalics 2019-05-22 14:12 ` Stefan Monnier 2019-05-22 15:50 ` Eli Zaretskii 2019-05-22 7:21 ` Eli Zaretskii 2020-08-26 11:06 ` Lars Ingebrigtsen 2020-09-05 7:05 ` Eli Zaretskii 2020-10-07 3:27 ` Lars Ingebrigtsen 2020-10-07 7:58 ` martin rudalics 2020-11-29 21:03 ` Basil L. Contovounesios 2020-11-30 9:05 ` martin rudalics 2020-11-30 18:07 ` Basil L. Contovounesios 2020-11-30 19:01 ` martin rudalics 2020-11-30 20:33 ` Basil L. Contovounesios 2020-12-01 9:34 ` martin rudalics 2020-11-30 19:42 ` Eli Zaretskii 2020-11-30 20:34 ` Basil L. Contovounesios 2020-12-07 22:16 ` Basil L. Contovounesios 2020-12-07 22:37 ` Basil L. Contovounesios 2020-12-08 8:09 ` martin rudalics 2020-12-14 21:03 ` Basil L. Contovounesios 2020-12-15 16:03 ` Eli Zaretskii 2020-12-15 16:24 ` Basil L. Contovounesios 2020-12-18 14:57 ` Basil L. Contovounesios 2020-12-18 15:36 ` Stefan Monnier 2020-12-18 18:49 ` Basil L. Contovounesios 2020-12-19 10:33 ` Eli Zaretskii 2020-12-19 14:15 ` Basil L. Contovounesios 2020-12-19 16:06 ` Eli Zaretskii 2020-12-19 21:10 ` Basil L. Contovounesios 2020-12-20 15:05 ` Eli Zaretskii 2019-03-07 6:18 ` Alexander Miller 2019-03-07 8:29 ` martin rudalics 2019-03-07 9:44 ` Alexander Miller 2019-03-07 13:46 ` martin rudalics 2020-12-20 17:57 ` Basil L. Contovounesios
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).