bug#45792: 28.0.50; regression in commit c7c154bb, minibuffer is not closed after opening a file

bug#45792: 28.0.50; regression in commit c7c154bb, minibuffer is not closed after opening a file

[Platon Pronko]

When I open a file using C-x C-f, minibuffer does not close after I finish typing the filename
- current window does not change at all, minibuffer stays open (as if Emacs "froze").
Pressing C-g or any other key unfreezes the window and file is displayed.

I bisected the problem to commit c7c154bb, "Fix incompleteness in the implementation of minibuffer-follows-selected-frame".

Problem is somehow related to the fact that I use dvorak input method.

The problem goes away if I comment out the section under "We've exited the recursive edit without an error" in minibuf.c.

Steps to reproduce:

1. init.el contents:

(define-minor-mode dvorak-minor-mode "Use english-dvorak input method." :lighter nil
   (if
     (not (string= " *Minibuf-0*" (buffer-name (current-buffer))))
     (if dvorak-minor-mode (activate-input-method "english-dvorak"))))
(define-global-minor-mode global-dvorak-mode dvorak-minor-mode dvorak-minor-mode)
(global-dvorak-mode t)

2. Start emacs with: ./src/emacs -Q --eval '(load-file "~/.emacs.d/init.el")'

3. Type C-x C-f, type in some text-file filename, press Return.

4. Observe no file being opened and "[DV@]" in minibuffer.



In GNU Emacs 28.0.50 (build 15, x86_64-pc-linux-gnu, GTK+ Version 3.24.24, cairo version 1.17.4)
  of 2021-01-11 built on the-big-maker
Repository revision: c7c154bb5756e0ae71d342c5d8aabf725877f186
Repository branch: HEAD
Windowing system distributor 'The X.Org Foundation', version 11.0.12010000
System Description: Arch Linux

Configured using:
  'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
  --localstatedir=/var --mandir=/usr/share/man --with-gameuser=:games
  --with-sound=alsa --with-modules --without-gconf --without-gsettings
  --with-x-toolkit=gtk3 --without-xaw3d'

Configured features:
XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND GPM DBUS 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

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

Major mode: Text

Minor modes in effect:
   global-dvorak-mode: t
   dvorak-minor-mode: t
   tooltip-mode: t
   global-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
rfc822 mml mml-sec epa derived epg epg-config gnus-util rmail
rmail-loaddefs auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json map text-property-search time-date
subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils quail help-mode
easymenu cl-loaddefs cl-lib easy-mmode iso-transl tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame minibuffer cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite charscript charprop case-table epa-hook
jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice button
loaddefs faces cus-face macroexp files window text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote threads dbusbind inotify lcms2
dynamic-setting font-render-setting cairo move-toolbar gtk x-toolkit x
multi-tty make-network-process emacs)

Memory information:
((conses 16 61568 4854)
  (symbols 48 7179 1)
  (strings 32 21381 1187)
  (string-bytes 1 655960)
  (vectors 16 13820)
  (vector-slots 8 173393 10673)
  (floats 8 22 45)
  (intervals 56 265 0)
  (buffers 984 13))

bug#45792: 28.0.50; regression in commit c7c154bb, minibuffer is not closed after opening a file

[martin rudalics]

 > 1. init.el contents:
 >
 > (define-minor-mode dvorak-minor-mode "Use english-dvorak input method." :lighter nil
 >    (if
 >      (not (string= " *Minibuf-0*" (buffer-name (current-buffer))))
 >      (if dvorak-minor-mode (activate-input-method "english-dvorak"))))

I understand that this may have worked for you over the past decades but
it's not really clean.  Why don't you just use "(minibufferp)" here?  If
the problem persists, we then can dig further.

Thanks, martin

bug#45792: 28.0.50; regression in commit c7c154bb, minibuffer is not closed after opening a file

[Platon Pronko]

Yes, of course minibufferp is much cleaner - I just wasn't aware of its existence :)

But the problem still persists if I refactor the code to use it. Here's the code I used:

(define-minor-mode dvorak-minor-mode "Use english-dvorak input method." :lighter nil
   (if (and dvorak-minor-mode (not (minibufferp)))
     (activate-input-method "english-dvorak")))
(define-global-minor-mode global-dvorak-mode dvorak-minor-mode dvorak-minor-mode)
(global-dvorak-mode t)

Actually even removing (not (minibufferp)) altogether has no effect on the issue.

Best regards,
Platon Pronko

bug#45792: 28.0.50; regression in commit c7c154bb, minibuffer is not closed after opening a file

[Alan Mackenzie]

Hello, Platon.

Thanks for such a high quality bug report, and thanks for investigating
it as far as you have done.

On Mon, Jan 11, 2021 at 19:09:53 +0300, Platon Pronko wrote:
> When I open a file using C-x C-f, minibuffer does not close after I
> finish typing the filename - current window does not change at all,
> minibuffer stays open (as if Emacs "froze").  Pressing C-g or any other
> key unfreezes the window and file is displayed.

It seems to be a redisplay problem.  Emacs is doing everything to visit
the new file _except_ the final redisplay action.  Any keypress (or mouse
action) triggers that missing redisplay.

> I bisected the problem to commit c7c154bb, "Fix incompleteness in the
> implementation of minibuffer-follows-selected-frame".

OK.

> Problem is somehow related to the fact that I use dvorak input method.

I spent yesterday evening trying to narrow it down more.  The
define-global-minor-mode causes dvorak-minor-mode to get added to three
hooks.  Of these, either (most likely both of them, too) of
change-major-mode-hook or after-change-major-mode-hook will cause the bug
when dvorak-minor-mode is added to it.

> The problem goes away if I comment out the section under "We've exited
> the recursive edit without an error" in minibuf.c.

I'm trying to work out, from my notes, exactly why I put that section of
code in.  I might well just take it out.  But it would be good to
understand the mechanism of the bug, first.

> Steps to reproduce:

> 1. init.el contents:

> (define-minor-mode dvorak-minor-mode "Use english-dvorak input method." :lighter nil
>    (if
>      (not (string= " *Minibuf-0*" (buffer-name (current-buffer))))
>      (if dvorak-minor-mode (activate-input-method "english-dvorak"))))
> (define-global-minor-mode global-dvorak-mode dvorak-minor-mode dvorak-minor-mode)
> (global-dvorak-mode t)

As you've said yourself in another post, the testing for " *Minibuf-0*"
doesn't seem to effect the bug.  So, it would appear to be the adding of
dvorak-minor-mode to these two hooks.  (See above.)

> 2. Start emacs with: ./src/emacs -Q --eval '(load-file "~/.emacs.d/init.el")'

> 3. Type C-x C-f, type in some text-file filename, press Return.

> 4. Observe no file being opened and "[DV@]" in minibuffer.

As already said, the file probably is being opened, it just isn't getting
displayed.  I'll be carrying on looking at this.

> In GNU Emacs 28.0.50 (build 15, x86_64-pc-linux-gnu, GTK+ Version 3.24.24, cairo version 1.17.4)
>   of 2021-01-11 built on the-big-maker
> Repository revision: c7c154bb5756e0ae71d342c5d8aabf725877f186
> Repository branch: HEAD
> Windowing system distributor 'The X.Org Foundation', version 11.0.12010000
> System Description: Arch Linux

> Configured using:
>   'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
>   --localstatedir=/var --mandir=/usr/share/man --with-gameuser=:games
>   --with-sound=alsa --with-modules --without-gconf --without-gsettings
>   --with-x-toolkit=gtk3 --without-xaw3d'

[ .... ]

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#45792: 28.0.50; regression in commit c7c154bb, minibuffer is not closed after opening a file

[Alan Mackenzie]

Hello again, Platon.

On Mon, Jan 11, 2021 at 19:09:53 +0300, Platon Pronko wrote:
> When I open a file using C-x C-f, minibuffer does not close after I
> finish typing the filename
> - current window does not change at all, minibuffer stays open (as if
> Emacs "froze").
> Pressing C-g or any other key unfreezes the window and file is
> displayed.

What is happening is that Emacs is getting caught on a (sit-for 1000000)
in quail-minibuffer-message.  The good news is that it will respond
eventually.  The bad news is that 11½ days is a rather sluggish response
time.

> I bisected the problem to commit c7c154bb, "Fix incompleteness in the
> implementation of minibuffer-follows-selected-frame".

This commit has triggered the problem, which was there all along.  It is
not the cause of it.

> Problem is somehow related to the fact that I use dvorak input method.

More precisely, any input method which uses quail-activate to start
itself.  For testing, I actually switched to "german-postfix" in your
recipe, so as to be able actually to use Emacs after applying it.  ;-)

> The problem goes away if I comment out the section under "We've exited
> the recursive edit without an error" in minibuf.c.

That may be so, but it is the trigger for an existing problem rather
than being the problem itself.

> Steps to reproduce:

> 1. init.el contents:

> (define-minor-mode dvorak-minor-mode "Use english-dvorak input method." :lighter nil
>    (if
>      (not (string= " *Minibuf-0*" (buffer-name (current-buffer))))
>      (if dvorak-minor-mode (activate-input-method "english-dvorak"))))
> (define-global-minor-mode global-dvorak-mode dvorak-minor-mode dvorak-minor-mode)
> (global-dvorak-mode t)

> 2. Start emacs with: ./src/emacs -Q --eval '(load-file "~/.emacs.d/init.el")'

Just as a matter of interest, you can write that more easily as
./src/emacs -Q -l ~/.emacs.d/init.el.

> 3. Type C-x C-f, type in some text-file filename, press Return.

> 4. Observe no file being opened and "[DV@]" in minibuffer.

What happens is that read-minibuffer does its thing, then a C function
read_minibuf_unwind calls minibuffer-inactive-mode since the minibuffer
is no longer live.  This mode change calls (run-mode-hooks
'minibuffer-inactive-mode-hook), which as a side effect runs the hook
after-change-major-mode-hook, onto which define-global-mode has put
global-dvorak-mode-enable-in-buffers.  g-d-m-e-i-b calls
dvorak-minor-mode (for the expired minibuffer) which calls
activate-input-method, ....., then quail-minibuffer-message.

This last function is the one with the million second timeout.

Clearly, changing mode to minibuffer-inactive-mode shouldn't trigger the
input method mechanisms.  I'm not sure at the moment how to stop this
happening, so I can't as yet close the bug.

> In GNU Emacs 28.0.50 (build 15, x86_64-pc-linux-gnu, GTK+ Version 3.24.24, cairo version 1.17.4)
>   of 2021-01-11 built on the-big-maker
> Repository revision: c7c154bb5756e0ae71d342c5d8aabf725877f186
> Repository branch: HEAD
> Windowing system distributor 'The X.Org Foundation', version 11.0.12010000
> System Description: Arch Linux

[ .... ]

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#45792: 28.0.50; regression in commit c7c154bb, minibuffer is not closed after opening a file

[Platon Pronko]

Hi!

I'm terribly sorry, apparently I wasted a lot of your time.

I tested (minibufferp) refactoring again and it seems that when I
did that for the previous email I messed something up. When I ran the code today
the issue went away.

Original reason for " *Minibuf-0*" test in dvorak-minor-mode was to
stop emacs --daemon from hanging on startup. (now that you explained about
quail-minibuffer-message and sit-for, I assume that's was the reason why that was happening)

But the buffer that was triggering the current issue is called " *Minibuf-1*", so it
passes the test and activate-input-method is called.

I had no idea that activate-input-method was dangerous when called in
global minor modes, but that's probably just the way it is.

Again, terribly sorry for wasting your time on what turned out to be
a configuration issue.

> Just as a matter of interest, you can write that more easily as
> ./src/emacs -Q -l ~/.emacs.d/init.el.

Thanks, that's a useful shortcut!

Best regards,
Platon Pronko

bug#45792: 28.0.50; regression in commit c7c154bb, minibuffer is not closed after opening a file

[Alan Mackenzie]

Hello again, Platon.

Terribly sorry to take nearly two weeks to reply.  Real life got in the
way.

On Sun, Jan 17, 2021 at 10:50:32 +0300, Platon Pronko wrote:
> Hi!

> I'm terribly sorry, apparently I wasted a lot of your time.

No problem, but I don't think you did.  There were real bugs connected
with input methods, and I think your initial scenario highlighted one of
these bugs.

> I tested (minibufferp) refactoring again and it seems that when I
> did that for the previous email I messed something up. When I ran the code today
> the issue went away.

It hasn't gone away for me.  ;-)

> Original reason for " *Minibuf-0*" test in dvorak-minor-mode was to
> stop emacs --daemon from hanging on startup. (now that you explained about
> quail-minibuffer-message and sit-for, I assume that's was the reason why that was happening)

Possibly.

> But the buffer that was triggering the current issue is called " *Minibuf-1*", so it
> passes the test and activate-input-method is called.

Yes.

> I had no idea that activate-input-method was dangerous when called in
> global minor modes, but that's probably just the way it is.

I disagree.  The fact that it is dangerous is a bug which needs fixing.
The fix I have at the moment is not to call the (sit-for 1000000) when
the current buffer is a minibuffer.  To be honest, I don't understand
quail.el very well, but I'm not sure there's anybody still on the
development team who understands it any better.

> Again, terribly sorry for wasting your time on what turned out to be
> a configuration issue.

No need to apologise - there were real bugs there.  Thanks for the
original report!

> > Just as a matter of interest, you can write that more easily as
> > ./src/emacs -Q -l ~/.emacs.d/init.el.

> Thanks, that's a useful shortcut!

:-)

I'm hoping to commit a patch for the bug over the weekend, sometime.
It's been a difficult bug to diagnose.

> Best regards,
> Platon Pronko

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#45792: 28.0.50; regression in commit c7c154bb, minibuffer is not closed after opening a file

[Platon Pronko]

Hi!

> No problem, but I don't think you did.  There were real bugs connected
> with input methods, and I think your initial scenario highlighted one of
> these bugs.

I'm glad to hear that!

> It hasn't gone away for me.  

This is quite strange. (minibufferp) guard was supposed to avoid running
any code whatsoever on minibuffer, so there shouldn't be any calls
to activate-input-method and associated bugs shouldn't be triggered.

Here's the code I'm talking about, just in case:

(define-minor-mode dvorak-minor-mode "Use english-dvorak input method." :lighter nil
   (if (and dvorak-minor-mode (not (minibufferp)))
     (activate-input-method "english-dvorak")))
(define-global-minor-mode global-dvorak-mode dvorak-minor-mode dvorak-minor-mode)
(global-dvorak-mode t)

Best regards,
Platon Pronko

bug#45792: 28.0.50; regression in commit c7c154bb, minibuffer is not closed after opening a file

[Alan Mackenzie]

Hello, Platon.

On Sun, Jan 31, 2021 at 11:54:24 +0300, Platon Pronko wrote:
> Hi!

> > No problem, but I don't think you did.  There were real bugs connected
> > with input methods, and I think your initial scenario highlighted one of
> > these bugs.

> I'm glad to hear that!

> > It hasn't gone away for me.  

> This is quite strange. (minibufferp) guard was supposed to avoid running
> any code whatsoever on minibuffer, so there shouldn't be any calls
> to activate-input-method and associated bugs shouldn't be triggered.

> Here's the code I'm talking about, just in case:

> (define-minor-mode dvorak-minor-mode "Use english-dvorak input method." :lighter nil
>    (if (and dvorak-minor-mode (not (minibufferp)))
>      (activate-input-method "english-dvorak")))
> (define-global-minor-mode global-dvorak-mode dvorak-minor-mode dvorak-minor-mode)
> (global-dvorak-mode t)

Apologies, I'd not paid enough attention to your test code.  With the
(minibufferp) check, the bug indeed doesn't show itself.

However, I think users ought to be able to use input methods in
minibuffers too.

What was triggering the bug without the (minibufferp) check was the
minibuffer's transition from fundamental-mode to
minibuffer-inactive-mode.  A change in major mode triggers a call to
global minor modes.  Here, the activate-input-method causes "guidance"
to be displayed on the mode line and an infinite wait to start.  This is
normally OK, because the user's command has just been completed, but
this isn't the case when a minibuffer has just been used.

So I've committed a patch which suppresses the display of "guidance" in
an expired minibuffer.  The aim is that you can omit the (minibufferp)
check from your code if you want, and have the input method working in
minibuffers.

Could you please try out the new code, and let me know whether it's
working properly, or what's still not quite right.  Then I can either
close the bug or start fixing it again.  Thanks!

> Best regards,
> Platon Pronko

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#45792: 28.0.50; regression in commit c7c154bb, minibuffer is not closed after opening a file

[Platon Pronko]

Hi!

Tested the commit, the issue is gone - both with and without (not (minibufferp)).

However I noticed that (minibufferp) can return nil when it is called on *Minibuf-1* (I noticed this when suddenly dvorak was enabled in minibuffer even though (not (minibufferp)) guard was still in the code).

Here's some debug code I used to check minibufferp return:

(define-minor-mode dvorak-minor-mode "Use english-dvorak input method." :lighter nil
   (message (format "%s %s" (buffer-name (current-buffer)) (minibufferp)))
   (if (and dvorak-minor-mode (not (minibufferp)))
     (activate-input-method "english-dvorak")))
(define-global-minor-mode global-dvorak-mode dvorak-minor-mode dvorak-minor-mode)
(global-dvorak-mode t)

And here's sample output in the *Message* buffer:

For information about GNU Emacs and the GNU system, type C-h C-a.
*scratch* nil
Loading quail/latin-post...done
  *Minibuf-0* t
*Messages* nil
  *Echo Area 0* nil
  *Echo Area 1* nil
  *load* nil
  *Minibuf-1* nil  <<-- here it thinks that Minibuf-1 is not a minibuffer
  *Minibuf-0* t

Is this supposed to happen?

Best regards,
Platon Pronko

bug#45792: 28.0.50; regression in commit c7c154bb, minibuffer is not closed after opening a file

[Alan Mackenzie]

Hello, Platon.

On Sun, Jan 31, 2021 at 15:11:11 +0300, Platon Pronko wrote:
> Hi!

> Tested the commit, the issue is gone - both with and without (not
> (minibufferp)).

Thanks!  I'll mark the bug as closed, just as soon as your other
observation has been dealt with.

> However I noticed that (minibufferp) can return nil when it is called
> on *Minibuf-1* (I noticed this when suddenly dvorak was enabled in
> minibuffer even though (not (minibufferp)) guard was still in the
> code).

> Here's some debug code I used to check minibufferp return:

> (define-minor-mode dvorak-minor-mode "Use english-dvorak input method." :lighter nil
>    (message (format "%s %s" (buffer-name (current-buffer)) (minibufferp)))
>    (if (and dvorak-minor-mode (not (minibufferp)))
>      (activate-input-method "english-dvorak")))
> (define-global-minor-mode global-dvorak-mode dvorak-minor-mode dvorak-minor-mode)
> (global-dvorak-mode t)

> And here's sample output in the *Message* buffer:

> For information about GNU Emacs and the GNU system, type C-h C-a.
> *scratch* nil
> Loading quail/latin-post...done
>   *Minibuf-0* t
> *Messages* nil
>   *Echo Area 0* nil
>   *Echo Area 1* nil
>   *load* nil
>   *Minibuf-1* nil  <<-- here it thinks that Minibuf-1 is not a minibuffer
>   *Minibuf-0* t

> Is this supposed to happen?

No, it is most definitely not, and well done for spotting it.  It's
taken me most of the afternoon to fix it, and I still don't understand
fully what's causing it.

But, basically, the minibuffers are kept on a Lisp list, one minibuffer
per recursive depth.  They are nconc'd onto the end of the list as they
are created, and minibufferp works by seeing if the buffer is a member
of this list.

Before my fix, the major mode of the newly created minibuffer was
getting set before it was in its place on the list.  I presume something
in this major mode setting was trying to get a minibuffer, for some
reason, when the minibuffer list was not in a consistent state.

I've changed the order of these operations, so that the new minibuffer
is firmly in the list before its mode gets set.  The problem appears to
have been fixed.

Give me half an hour (or so), and I will have committed this fix to the
Emacs repository master branch.

Again, thanks for spotting such an obscure bug.

> Best regards,
> Platon Pronko

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#45792: 28.0.50; regression in commit c7c154bb, minibuffer is not closed after opening a file

[Platon Pronko]

Hi Alan!

Tested your fix, can confirm that (minibufferp) now works as expected.

Best regards,
Platon Pronko

bug#45792: 28.0.50; regression in commit c7c154bb, minibuffer is not closed after opening a file

[Alan Mackenzie]

Hello, Platon.

On Sun, Jan 31, 2021 at 22:37:44 +0300, Platon Pronko wrote:
> Hi Alan!

> Tested your fix, can confirm that (minibufferp) now works as expected.

Many thanks.  I'm closing the bug with this post.

> Best regards,
> Platon Pronko

-- 
Alan Mackenzie (Nuremberg, Germany).