bug#54438: 27.2; inconsistent-behavior-global-vs-local-hook

bug#54438: 27.2; inconsistent-behavior-global-vs-local-hook

[dalanicolai]

[-- Attachment #1: Type: text/plain, Size: 6995 bytes --]

I am trying to pop to some (input) buffer when opening some (main)
buffer (for a gitter chat client, which is handy for Spacemacs
interactive support). Anyway, I am trying to use a local hook-function
which 'pops up' the input buffer. As I am using pop-to-buffer, the input
buffer should get selected. However, it does not get selected when using
a local hook, while it does work as expected when using the global hook.

I have used the following test code:

(defun pop-up ()
  (pop-to-buffer "pop-up"))

(defun test ()
  (interactive)
  (with-current-buffer (get-buffer-create "test")
    (add-hook 'window-configuration-change-hook #'pop-up)))
    ;; (add-hook 'window-configuration-change-hook #'pop-up nil t)

    ;; the following line triggers the `pop-up` hook function
    (pop-to-buffer "test")))

;; evaluate code to remove the (global) hook
(remove-hook 'window-configuration-change-hook #'pop-up)

So you can start from emacs -Q, evaluate the above code, and do `M-x
test`, as expected the 'pop-up' buffer will be selected.

Now remove the hook (by evaluating the last line), then comment out the
global hook and uncomment the local hook. Again do `M-x test`, this
time, the 'test' buffer, wrongly and inconsistently, will be selected.
To me this behavior looks like a bug.



In GNU Emacs 27.2 (build 1, x86_64-redhat-linux-gnu, GTK+ Version 3.24.30,
cairo version 1.17.4)
 of 2021-08-07 built on buildvm-x86-31.iad2.fedoraproject.org
Windowing system distributor 'The X.Org Foundation', version 11.0.12014000
System Description: Fedora Linux 35 (Workstation Edition)

Recent messages:
Quit
scroll-up-command: End of buffer
user-error: No buttons!
You can run the command ‘kill-buffer’ with C-x k
delete-window: Attempt to delete minibuffer or sole ordinary window
test2
(test evil-refresh-cursor window--adjust-process-windows)
user-error: No replacement character typed
evil-line-move: End of buffer
Quit [4 times]
(evil-refresh-cursor window--adjust-process-windows)
Configured using:
 'configure --build=x86_64-redhat-linux-gnu
 --host=x86_64-redhat-linux-gnu --program-prefix=
 --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr
 --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc
 --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64
 --libexecdir=/usr/libexec --localstatedir=/var
 --sharedstatedir=/var/lib --mandir=/usr/share/man
 --infodir=/usr/share/info --with-dbus --with-gif --with-jpeg --with-png
 --with-rsvg --with-tiff --with-xft --with-xpm --with-x-toolkit=gtk3
 --with-gpm=no --with-xwidgets --with-modules --with-harfbuzz
 --with-cairo --with-json build_alias=x86_64-redhat-linux-gnu
 host_alias=x86_64-redhat-linux-gnu CC=gcc 'CFLAGS=-DMAIL_USE_LOCKF -O2
 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches
 -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2
 -Wp,-D_GLIBCXX_ASSERTIONS
 -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong
 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic
 -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection'
 LDFLAGS=-Wl,-z,relro
 PKG_CONFIG_PATH=:/usr/lib64/pkgconfig:/usr/share/pkgconfig'

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

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  marginalia-mode: t
  savehist-mode: t
  vertico-mode: t
  recentf-mode: t
  shell-dirtrack-mode: t
  evil-mode: t
  evil-local-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  override-global-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
format-spec rfc822 mml mml-sec epa derived epg epg-config gnus-util
rmail rmail-loaddefs mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils help-fns radix-tree cl-print debug
backtrace find-func cursor-sensor time-date consult-vertico consult
bookmark text-property-search pp mule-util rx marginalia savehist
orderless vertico evil-nerd-commenter evil-nerd-commenter-operator
evil-nerd-commenter-sdk sgml-mode dom which-key bind-map recentf
tree-widget wid-edit evil evil-keybindings evil-integration 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 advice evil-common
windmove thingatpt rect evil-digraphs evil-vars ring edmacro kmacro
undo-tree diff cl-extra help-mode use-package use-package-ensure
use-package-delight use-package-diminish use-package-bind-key bind-key
easy-mmode use-package-core finder-inf info package easymenu browse-url
url-handlers url-parse auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json subr-x map url-vars seq byte-opt gv
bytecomp byte-compile cconv cl-loaddefs cl-lib chemacs tooltip eldoc
electric uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win
x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame minibuffer cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite charscript charprop case-table epa-hook
jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote threads dbusbind inotify dynamic-setting system-font-setting
font-render-setting xwidget-internal cairo move-toolbar gtk x-toolkit x
multi-tty make-network-process emacs)

Memory information:
((conses 16 289213 15010)
 (symbols 48 19826 1)
 (strings 32 93633 2529)
 (string-bytes 1 2476732)
 (vectors 16 32184)
 (vector-slots 8 380120 15630)
 (floats 8 94 84)
 (intervals 56 693 0)
 (buffers 1000 14))

[-- Attachment #2: Type: text/html, Size: 7581 bytes --]

bug#54438: 27.2; inconsistent-behavior-global-vs-local-hook

[Michael Heerdegen]

dalanicolai <dalanicolai@gmail.com> writes:

> (defun pop-up ()
>   (pop-to-buffer "pop-up"))
>
> (defun test ()
>   (interactive)
>   (with-current-buffer (get-buffer-create "test")
>     (add-hook 'window-configuration-change-hook #'pop-up)))
>     ;; (add-hook 'window-configuration-change-hook #'pop-up nil t)
>
>     ;; the following line triggers the `pop-up` hook function
>     (pop-to-buffer "test")))

I think it's a bad idea to do something that changes the window
configuration in `window-configuration-change-hook'.  It's not
unexpected to get an infinite recursion or any other unpredictable
behavior.

Michael.

bug#54438: 27.2; inconsistent-behavior-global-vs-local-hook

[martin rudalics]

 > I think it's a bad idea to do something that changes the window
 > configuration in `window-configuration-change-hook'.  It's not
 > unexpected to get an infinite recursion or any other unpredictable
 > behavior.

People should avoid it in packages because they could easily step on
each other's toes.  Written carefully, it could be useful in private
code.

martin

bug#54438: 27.2; inconsistent-behavior-global-vs-local-hook

[dalanicolai]

[-- Attachment #1: Type: text/plain, Size: 1614 bytes --]

Thank you for the reply. Ah, sorry I forgot to remove the parens to include
the `pop-to-buffer` within the defun.

About the 'restoring mechanism'; the global hook seems not to restore the
window selected before running the hook.

So here are the two separated test cases (they both use the 'pop-up'
function)

(defun pop-up ()
  (pop-to-buffer "pop-up"))

(defun test ()
  (interactive)
  (with-current-buffer (get-buffer-create "test")
    (add-hook 'window-configuration-change-hook #'pop-up)
    (pop-to-buffer "test")))

;; evaluate code to remove the (global) hook
(remove-hook 'window-configuration-change-hook #'pop-up)

(defun test ()
  (interactive)
  (with-current-buffer (get-buffer-create "test")
    (add-hook 'window-configuration-change-hook #'pop-up nil t)
    (pop-to-buffer "test")))


So the 'restoring mechanism' works like you explained for the local hook
but not for the global hook
(where after calling 'test', the pop-up buffer will be selected).

Also, I would expect that a hook just calls the hook function, and it would
not 'restore the window
 selected before the hook'. I would say the restoring is undesired behavior.



On Fri, 18 Mar 2022 at 09:37, martin rudalics <rudalics@gmx.at> wrote:

>  > I think it's a bad idea to do something that changes the window
>  > configuration in `window-configuration-change-hook'.  It's not
>  > unexpected to get an infinite recursion or any other unpredictable
>  > behavior.
>
> People should avoid it in packages because they could easily step on
> each other's toes.  Written carefully, it could be useful in private
> code.
>
> martin
>

[-- Attachment #2: Type: text/html, Size: 2342 bytes --]

bug#54438: 27.2; inconsistent-behavior-global-vs-local-hook

[dalanicolai]

[-- Attachment #1: Type: text/plain, Size: 1847 bytes --]

And, moreover, the behavior for the global vs local hook is inconsistent...

On Fri, 18 Mar 2022 at 10:11, dalanicolai <dalanicolai@gmail.com> wrote:

> Thank you for the reply. Ah, sorry I forgot to remove the parens to
> include the `pop-to-buffer` within the defun.
>
> About the 'restoring mechanism'; the global hook seems not to restore the
> window selected before running the hook.
>
> So here are the two separated test cases (they both use the 'pop-up'
> function)
>
> (defun pop-up ()
>   (pop-to-buffer "pop-up"))
>
> (defun test ()
>   (interactive)
>   (with-current-buffer (get-buffer-create "test")
>     (add-hook 'window-configuration-change-hook #'pop-up)
>     (pop-to-buffer "test")))
>
> ;; evaluate code to remove the (global) hook
> (remove-hook 'window-configuration-change-hook #'pop-up)
>
> (defun test ()
>   (interactive)
>   (with-current-buffer (get-buffer-create "test")
>     (add-hook 'window-configuration-change-hook #'pop-up nil t)
>     (pop-to-buffer "test")))
>
>
> So the 'restoring mechanism' works like you explained for the local hook
> but not for the global hook
> (where after calling 'test', the pop-up buffer will be selected).
>
> Also, I would expect that a hook just calls the hook function, and it
> would not 'restore the window
>  selected before the hook'. I would say the restoring is undesired
> behavior.
>
>
>
> On Fri, 18 Mar 2022 at 09:37, martin rudalics <rudalics@gmx.at> wrote:
>
>>  > I think it's a bad idea to do something that changes the window
>>  > configuration in `window-configuration-change-hook'.  It's not
>>  > unexpected to get an infinite recursion or any other unpredictable
>>  > behavior.
>>
>> People should avoid it in packages because they could easily step on
>> each other's toes.  Written carefully, it could be useful in private
>> code.
>>
>> martin
>>
>

[-- Attachment #2: Type: text/html, Size: 2778 bytes --]

bug#54438: 27.2; inconsistent-behavior-global-vs-local-hook

[martin rudalics]

 > About the 'restoring mechanism'; the global hook seems not to restore the
 > window selected before running the hook.

Right.  You can easily test that yourself by calling
'run-window-configuration-change-hook' directly.

 > So the 'restoring mechanism' works like you explained for the local hook
 > but not for the global hook
 > (where after calling 'test', the pop-up buffer will be selected).

Right.  Because for running the global hook, Emacs does not enter the
(costly) window selection rigmarole.  But I think that even for the
global hook you might be able to run into troubles when the current
buffer and that of the selected window differ.  A case for the expert
only though.

 > Also, I would expect that a hook just calls the hook function, and it would
 > not 'restore the window
 >   selected before the hook'. I would say the restoring is undesired behavior.

The idea is probably that a function on that hook should be allowed to
inadvertently select some other window and have the hook runner fix
that.  Maybe Stefan can tell us more.

In either case you can avoid these problems by running whatever you have
in mind on 'window-state-change-hook' with the following caveat:

    • Window change functions should not create or delete windows or
      change the buffer, size or selection status of any window because
      there is no guarantee that the information about such a change will
      be propagated to other window change functions.  If at all, any
      such change should be executed only by the last function listed by
      the default value of ‘window-state-change-hook’.

martin

bug#54438: 27.2; inconsistent-behavior-global-vs-local-hook

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> Also, I would expect that a hook just calls the hook function, and it would
>> not 'restore the window
>>   selected before the hook'. I would say the restoring is undesired behavior.
>
> The idea is probably that a function on that hook should be allowed to
> inadvertently select some other window and have the hook runner fix
> that.  Maybe Stefan can tell us more.

No, IIRC it is just a consequence of needing to select windows when
running the local part of the hook (since that part is supposed to be
run for every window (which was modified) displaying that buffer).


        Stefan

bug#54438: 27.2; inconsistent-behavior-global-vs-local-hook

[dalanicolai]

[-- Attachment #1: Type: text/plain, Size: 809 bytes --]

Thanks for your explanations... (although I will need some time to think
about and understand them)

On Fri, 18 Mar 2022 at 13:14, Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> >> Also, I would expect that a hook just calls the hook function, and it
> would
> >> not 'restore the window
> >>   selected before the hook'. I would say the restoring is undesired
> behavior.
> >
> > The idea is probably that a function on that hook should be allowed to
> > inadvertently select some other window and have the hook runner fix
> > that.  Maybe Stefan can tell us more.
>
> No, IIRC it is just a consequence of needing to select windows when
> running the local part of the hook (since that part is supposed to be
> run for every window (which was modified) displaying that buffer).
>
>
>         Stefan
>
>

[-- Attachment #2: Type: text/html, Size: 1185 bytes --]

bug#54438: 27.2; inconsistent-behavior-global-vs-local-hook

[martin rudalics]

 > I have used the following test code:
 >
 > (defun pop-up ()
 >    (pop-to-buffer "pop-up"))
 >
 > (defun test ()
 >    (interactive)
 >    (with-current-buffer (get-buffer-create "test")
 >      (add-hook 'window-configuration-change-hook #'pop-up)))
 >      ;; (add-hook 'window-configuration-change-hook #'pop-up nil t)
 >
 >      ;; the following line triggers the `pop-up` hook function
 >      (pop-to-buffer "test")))

What is the last paren here supposed to close?  Please provide two
separate test sequences - one for the local and one for the global hook
case.

 > ;; evaluate code to remove the (global) hook
 > (remove-hook 'window-configuration-change-hook #'pop-up)
 >
 > So you can start from emacs -Q, evaluate the above code, and do `M-x
 > test`, as expected the 'pop-up' buffer will be selected.
 >
 > Now remove the hook (by evaluating the last line), then comment out the
 > global hook and uncomment the local hook. Again do `M-x test`, this
 > time, the 'test' buffer, wrongly and inconsistently, will be selected.
 > To me this behavior looks like a bug.

I'd recommend to use 'window-configuration-change-hook' only when you
want the window/buffer of concern to be selected/made current while
running the hook.  Otherwise, you will inevitably run into problems when

- OT1H you want to select another window (that for "pop-up") while
   running the hook, and

- OTOH the mechanism at the end of run_window_configuration_change_hook
   (in window.c) restores the window selected before running the hook
   (apparently that for "test" in your case) which is certainly not a new
   window that popped up while running the hook.

martin