bug#46097: 27.1; Minibuffer may not be current when running minibuffer-exit-hook

bug#46097: 27.1; Minibuffer may not be current when running minibuffer-exit-hook

[klubujevetru]

Open the minibuffer with C-x C-f, switch to another window with C-x o
and abort the minibuffer with C-] (abort-recursive-edit).
minibuffer-exit-hook ends up running without the minibuffer being the
current buffer.

This may cause problems if a minibuffer adds a cleanup function the hook
locally, see for example discussion at
https://github.com/oantolin/embark/issues/114
The cleanup function fails to run in this situation because the local
value of the hook isn't considered.


In GNU Emacs 27.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.22, cairo version 1.17.3)
 of 2020-08-28 built on juergen
Windowing system distributor 'The X.Org Foundation', version 11.0.12010000
System Description: Arch Linux

Recent messages:
TEST: #<window 8181 on  *Minibuf-3*>  *Minibuf-3*

TEST: #<window 8181 on  *Minibuf-3*>  *Minibuf-3*
TEST: #<window 8181 on  *Minibuf-2*>  *Minibuf-2*
Quit
TEST: #<window 8181 on  *Minibuf-3*>  *Minibuf-3*
Ispell process killed
Local Ispell dictionary set to english
Starting new Ispell process /usr/bin/aspell with english dictionary...done
TEST: #<window 8181 on  *Minibuf-1*>  *Minibuf-1*

Configured using:
 'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/var --with-x-toolkit=gtk3 --with-xft --with-wide-int
 --with-modules --with-cairo --with-harfbuzz 'CFLAGS=-march=x86-64
 -mtune=generic -O2 -pipe -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 CAIRO SOUND GPM DBUS GSETTINGS GLIB NOTIFY
INOTIFY ACL GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS LIBSYSTEMD JSON
PDUMPER LCMS2 GMP

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

Memory information:
((conses 16 37731258 4232377)
 (symbols 48 94311 3643)
 (strings 32 2626060 933887)
 (string-bytes 1 79259002)
 (vectors 16 624752)
 (vector-slots 8 7835270 2124558)
 (floats 8 19039 20108)
 (intervals 56 5766073 96670)
 (buffers 1000 521))

bug#46097: 27.1; Minibuffer may not be current when running minibuffer-exit-hook

[martin rudalics]

 > Open the minibuffer with C-x C-f, switch to another window with C-x o
 > and abort the minibuffer with C-] (abort-recursive-edit).
 > minibuffer-exit-hook ends up running without the minibuffer being the
 > current buffer.
 >
 > This may cause problems if a minibuffer adds a cleanup function the hook
 > locally, see for example discussion at
 > https://github.com/oantolin/embark/issues/114
 > The cleanup function fails to run in this situation because the local
 > value of the hook isn't considered.

But both, current buffer and selected window, might be of interest for
the function running that hook.  So I think we should not change the
current behavior but rather document it and provide better access to
find the minibuffer that was just aborted.  The only way I found is

(window-buffer (active-minibuffer-window))

Why don't we provide a function like 'active-minibuffer'?  Or even a
function like 'minibuffer-list'?

martin

bug#46097: [External] : bug#46097: 27.1; Minibuffer may not be current when running minibuffer-exit-hook

[Drew Adams]

Great apologies for chiming in here without
having followed the thread.  Please ignore,
if what I say is irrelevant - sorry.
___

The bug report says:

 Open the minibuffer with C-x C-f, switch to another window with C-x o
 and abort the minibuffer with C-] (abort-recursive-edit).
 minibuffer-exit-hook ends up running without the minibuffer being the
 current buffer.

 This may cause problems if a minibuffer adds a cleanup function the hook
 locally, see for example discussion at... The cleanup function fails to
 run in this situation because the local value of the hook isn't considered.

A priori, my opinion is that it's wrong for any
function on `minibuffer-exit-hook' to _assume_ that
the minibuffer window is selected when it's invoked.

If a hook function needs that window to be selected
at some point then it should select it.
___

The minibuffer is largely like other buffers.  It's a
_huge_ mistake (design mistake, user mistake, or other)
to consider that interaction with the minibuffer is, in
general, modal - that the user is locked into _only_
interacting with the minibuffer.

This mistaken assumption is not rare, I'm afraid.
That's presumably because many uses of the minibuffer
are simple, and seem to be just type-input-then-RET at
a prompt, or type-and-complete-input-then-RET.

That common use case can mislead users into thinking
that that's the "normal", or a "required", behavior.
And that's quite limiting.

And if someone with that (mis)understanding starts
modifying the Emacs code that governs minibuffer
behavior, we get misguided, overly limiting behavior
imposed on Emacs, under the guise/excuse of "clean-up"
or in the name of "consistency".

bug#46097: 27.1; Minibuffer may not be current when running minibuffer-exit-hook

[martin rudalics]

Resending since the debbugs address was dropped.  Please always keep
the debbugs address CC'd.  Thanks.

> Yes, currently, adding a function to minibuffer-exit-hook locally is
> unreliable because it isn't guaranteed that the function will be called.
> I now believe this to be correct behaviour.
> I used to think that this was an elegant way for a minibuffer to perform
> cleanup on exit, because the hook function doesn't have to remove itself
> from the hook.

The first problem I see here is that I have no clear idea what "buffer
local" means when we talk about the minibuffer.  The second problem is
that I have no idea which buffer is (or should be) current when
'minibuffer-exit-hook' is run: It could be the minibuffer just exited,
the next lower level minibuffer when exiting a recursive minibuffer or
some normal buffer when going back to the top level.

As long as these issues are not resolved (and clearly documented), it
hardly makes sense to guess what kind of effect one can achieve by
running a function on 'minibuffer-exit-hook'.

> There are two minor inconveniences if a cleanup function is in (the
> global value of) minibuffer-exit-hook. First, it has to remove itself
> and second, it has to make sure it doesn't clean up too early after
> possible inner recursive minibuffers, so something like
>
>      (let ((depth (minibuffer-depth))
>            h)
>        (setq h (lambda ()
>                  (when (= depth (minibuffer-depth))
>                    (remove-hook 'minibuffer-exit-hook h)
>                    (cleanup))))
>        (add-hook 'minibuffer-exit-hook h))
>
> My proposal is to use minibuffer's local value of change-major-mode-hook
> instead:
>
>      (add-hook 'change-major-mode-hook #'cleanup nil t)
>
> This mostly has the same behaviour as the above, except that the cleanup
> also happens when changing the major mode of the minibuffer, which may
> perhaps even be desirable.

This sounds like capitulation.  And where would you run the 'add-hook' -
still in 'minibuffer-setup-hook' I suppose?  If we can't make these two
- 'minibuffer-setup-hook' and 'minibuffer-exit-hook' - symmetric and to
some extent dependable, we have a serious design flaw.

martin

bug#46097: 27.1; Minibuffer may not be current when running minibuffer-exit-hook

[martin rudalics]

> Resending since the debbugs address was dropped.  Please always keep
> the debbugs address CC'd.  Thanks.

I very likely dropped it myself.  Sorry for the confusion.

martin

bug#46097: 27.1; Minibuffer may not be current when running minibuffer-exit-hook

[Lars Ingebrigtsen]

klubujevetru <klubujevetru@cock.li> writes:

> Open the minibuffer with C-x C-f, switch to another window with C-x o
> and abort the minibuffer with C-] (abort-recursive-edit).
> minibuffer-exit-hook ends up running without the minibuffer being the
> current buffer.
>
> This may cause problems if a minibuffer adds a cleanup function the hook
> locally, see for example discussion at
> https://github.com/oantolin/embark/issues/114
> The cleanup function fails to run in this situation because the local
> value of the hook isn't considered.

(I'm going through old bug reports that unfortunately weren't resolved
at the time.)

I can reproduce this in Emacs 27.1, but this has apparently been changed
sometime after this -- in Emacs 29, the hook seems to always be run with
the minibuffer as the active buffer, so I'm closing this bug report.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no