GNU bug report logs - #47894
28.0.50; isearch does not work if enable-recursive-minibuffers is on and some input method is set.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 47894 in the body.
You can then email your comments to 47894 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: max.brieiev <at> gmail.com
To: bug-gnu-emacs <at> gnu.org
Subject: 28.0.50; y.oy
Date: Mon, 19 Apr 2021 19:06:58 +0300

[Message part 1 (text/plain, inline)]

Some recent change made isearch to misbehave.

isearch works normally when enable-recursive-minibuffers is off and no
input method is set.

However, when both enable-recursive-minibuffers is on and some input
method is set, pressing C-s and then entering some text does not start a
search. Successive presses of C-s lead to some garbled content being
produced in minibuffer window (see screenshot).

Steps to reproduce.

1. emacs -Q
2. M-x set-input-method RET programmer-dvorak
3. M-x customize-option RET enable-recursive-minibuffers
Toggle the option into "On" state
4. Inside, for example, *scratch* buffer press C-s, then enter text to
search. Observe that interactive search doesn't start. Also, successive
presses of C-s produce some garbled text in minibuffer window as can be
seen on screenshot below.

[screenshot.png (image/png, inline)]

[Message part 3 (text/plain, inline)]

In GNU Emacs 28.0.50 (build 7, x86_64-pc-linux-gnu, GTK+ Version 3.24.28, cairo version 1.17.4)
 of 2021-04-19 built on arch-max
Repository revision: 20b9ac8de4668e24111226c62c8b91678bfb391c
Repository branch: makepkg
Windowing system distributor '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-native-compilation --with-pgtk --with-x-toolkit=gtk3
 --without-xaw3d --without-m17n-flt --with-cairo --with-xwidgets
 --without-compress-install 'CFLAGS=-march=x86-64 -mtune=generic -O2
 -pipe -fno-plt -g -fuse-ld=gold -g -fuse-ld=gold'
 CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM HARFBUZZ JPEG JSON LCMS2
LIBOTF LIBSYSTEMD LIBXML2 MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER
PGTK PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS XIM
XWIDGETS GTK3 ZLIB

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

Major mode: Group

Minor modes in effect:
  cursor-sensor-mode: t
  gnus-undo-mode: t
  show-paren-mode: t
  display-battery-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
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow emacsbug sendmail sort smiley ansi-color gnus-async gnus-bcklg
gnus-draft comp comp-cstr warnings rx cl-extra gnus-ml disp-table
gnus-cite mail-extr cursor-sensor nndraft nnmh gnutls network-stream nsm
gnus-agent gnus-srvr gnus-score score-mode nnvirtual gnus-msg gnus-art
mm-uu mml2015 mm-view mml-smime smime dig nntp gnus-cache gnus-sum shr
kinsoku svg dom gnus-group gnus-undo gnus-start gnus-dbus gnus-cloud
nnimap nnmail mail-source utf7 netrc nnoo parse-time iso8601 gnus-spec
gnus-int gnus-range message rmc puny dired dired-loaddefs rfc822 mml
mml-sec epa derived epg epg-config mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader gnus-win view quail
help-mode edmacro kmacro format-spec tango-theme paren gnus nnheader
gnus-util rmail rmail-loaddefs rfc2047 rfc2045 ietf-drums
text-property-search time-date mail-utils mm-util mail-prsvr wid-edit
battery dbus xml cus-start cus-load info package browse-url url
url-proxy url-privacy url-expand url-methods url-history url-cookie
url-domsuf url-util mailcap url-handlers url-parse auth-source cl-seq
eieio eieio-core cl-macs eieio-loaddefs password-cache json subr-x map
url-vars seq byte-opt gv bytecomp byte-compile cconv cl-loaddefs cl-lib
iso-transl tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/pgtk-win pgtk-win term/common-win 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 easymenu timer select scroll-bar mouse jit-lock
font-lock syntax 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
xwidget-internal dbusbind inotify dynamic-setting font-render-setting
cairo move-toolbar gtk x-toolkit pgtk lcms2 multi-tty
make-network-process nativecomp emacs)

Memory information:
((conses 16 222637 14508)
 (symbols 48 17398 0)
 (strings 32 53662 3750)
 (string-bytes 1 1767033)
 (vectors 16 30337)
 (vector-slots 8 541117 33856)
 (floats 8 266 44)
 (intervals 56 499 0)
 (buffers 992 24))

Message #8 received at 47894 <at> debbugs.gnu.org (full text, mbox):

From: max.brieiev <at> gmail.com
To: 47894 <at> debbugs.gnu.org
Subject: Re: bug#47894: 28.0.50; isearch does not work if
 enable-recursive-minibuffers is on and some input method is set.
Date: Mon, 19 Apr 2021 19:31:35 +0300

Sorry for meaningless subject line in previous message (my first bug report). 

Message #11 received at 47894 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: max.brieiev <at> gmail.com
Cc: 47894 <at> debbugs.gnu.org
Subject: Re: bug#47894: 28.0.50; isearch does not work if
 enable-recursive-minibuffers is on and some input method is set.
Date: Mon, 19 Apr 2021 23:49:30 +0300

retitle 47894 28.0.50; isearch does not work if enable-recursive-minibuffers is on and some input method is set.
thanks

> Sorry for meaningless subject line in previous message (my first bug report). 

Thanks for the bug report.  The subject is fixed now.

Message #16 received at 47894 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: max.brieiev <at> gmail.com
Cc: Gregory Heytings <gregory <at> heytings.org>, 47894 <at> debbugs.gnu.org
Subject: Re: bug#47894: 28.0.50; isearch does not work if
 enable-recursive-minibuffers is on and some input method is set.
Date: Tue, 20 Apr 2021 23:00:53 +0300

> Some recent change made isearch to misbehave.
>
> isearch works normally when enable-recursive-minibuffers is off and no
> input method is set.
>
> However, when both enable-recursive-minibuffers is on and some input
> method is set, pressing C-s and then entering some text does not start a
> search. Successive presses of C-s lead to some garbled content being
> produced in minibuffer window (see screenshot).
>
> Steps to reproduce.
>
> 1. emacs -Q
> 2. M-x set-input-method RET programmer-dvorak
> 3. M-x customize-option RET enable-recursive-minibuffers
> Toggle the option into "On" state
> 4. Inside, for example, *scratch* buffer press C-s, then enter text to
> search. Observe that interactive search doesn't start. Also, successive
> presses of C-s produce some garbled text in minibuffer window as can be
> seen on screenshot below.

This is because of the recent change in ff796823e5
with the hope that it doesn't break other modes.
But your bug report helped to reveal that it
causes breakage.  So I had to revert it.

Gregory, could you please see if it can be improved
to not fail in the reported case?  Additionally,
on emacs-devel Zhiwei Chen said this:

  It failed to work when buffer is auto selected via
  `display-buffer-alist’.  When the point moves to the *Occur* buffer,
  typing “n”, “p” still sends “n”, “p” to isearch.

  (defun display-buffer-select (buffer alist)
    (let ((window (display-buffer-below-selected buffer alist)))
      (when (window-live-p window)
        (select-window window))))

  (setq display-buffer-alist '(("\\*Occur\\*" (display-buffer-select))))

Maybe this could be handled as well?

Message #19 received at 47894 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: max.brieiev <at> gmail.com, 47894 <at> debbugs.gnu.org
Subject: Re: bug#47894: 28.0.50;
 isearch does not work if enable-recursive-minibuffers
 is on and some input method is set.
Date: Tue, 20 Apr 2021 20:15:41 +0000

[Message part 1 (text/plain, inline)]

>
> This is because of the recent change in ff796823e5 with the hope that it 
> doesn't break other modes. But your bug report helped to reveal that it 
> causes breakage.  So I had to revert it.
>
> Gregory, could you please see if it can be improved to not fail in the 
> reported case?  Additionally, on emacs-devel Zhiwei Chen said this:
>
>  It failed to work when buffer is auto selected via
>  `display-buffer-alist’.  When the point moves to the *Occur* buffer,
>  typing “n”, “p” still sends “n”, “p” to isearch.
>
>  (defun display-buffer-select (buffer alist)
>    (let ((window (display-buffer-below-selected buffer alist)))
>      (when (window-live-p window)
>        (select-window window))))
>
>  (setq display-buffer-alist '(("\\*Occur\\*" (display-buffer-select))))
>
> Maybe this could be handled as well?
>

Thanks for the reminder; I had seen Zhiwei Chen's message, but not this 
bug.  I'll have a look.

Message #22 received at 47894 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: max.brieiev <at> gmail.com, 47894 <at> debbugs.gnu.org
Subject: Re: bug#47894: 28.0.50;
 isearch does not work if enable-recursive-minibuffers
 is on and some input method is set.
Date: Tue, 20 Apr 2021 21:51:38 +0000

[Message part 1 (text/plain, inline)]

>
> Thanks for the reminder; I had seen Zhiwei Chen's message, but not this 
> bug. I'll have a look.
>

This bug is rather strange.  When an input method is used, 
isearch-post-command-hook is executed twice, once in the current isearch 
buffer and once in the minibuffer.

When enable-recursive-minibuffers is unset, the execution of isearch-exit 
fails with "Command attempted to use minibuffer while in minibuffer", so 
the error is not visible (except in *Messages*).

When enable-recursive-minibuffers is set, the execution of isearch-exit 
does not fail, which triggers the current bug.

The immediate fix is attached, but I wonder if the cause is not elswhere, 
that is, if it is indeed intended that isearch-post-command-hook is 
executed in the minibuffer in such a case.

[Terminate-isearch-when-point-has-moved-to-another-bu.patch (text/x-diff, attachment)]

Message #25 received at 47894 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: max.brieiev <at> gmail.com, martin rudalics <rudalics <at> gmx.at>,
 47894 <at> debbugs.gnu.org
Subject: Re: bug#47894: 28.0.50;
 isearch does not work if enable-recursive-minibuffers
 is on and some input method is set.
Date: Tue, 20 Apr 2021 22:35:47 +0000

[Message part 1 (text/plain, inline)]

>
> Additionally, on emacs-devel Zhiwei Chen said this:
>
>  It failed to work when buffer is auto selected via
>  `display-buffer-alist’.  When the point moves to the *Occur* buffer,
>  typing “n”, “p” still sends “n”, “p” to isearch.
>
>  (defun display-buffer-select (buffer alist)
>    (let ((window (display-buffer-below-selected buffer alist)))
>      (when (window-live-p window)
>        (select-window window))))
>
>  (setq display-buffer-alist '(("\\*Occur\\*" (display-buffer-select))))
> 
> Maybe this could be handled as well?
>

And this one is strange, too, for two reasons:

- display-buffer-select is not a documented action, and does not even 
appear in the sources (even Google does not find it!), yet it works;

- After this action, point has moved, but (current-buffer) does not return 
the buffer where point is; (window-buffer (selected-window)) does.  It 
seems to me that at the top-level these two should always be equal; 
apparently they are not.

Again it's not clear to me whether the bug is here or elsewhere, but the 
attached patch fixes the original problem and the two bugs.

Cc'ing Martin, who may have some insights on the above two points.

[Terminate-isearch-when-point-has-moved-to-another-bu.patch (text/x-diff, attachment)]

Message #28 received at 47894 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: max.brieiev <at> gmail.com, martin rudalics <rudalics <at> gmx.at>,
 47894 <at> debbugs.gnu.org
Subject: Re: bug#47894: 28.0.50;
 isearch does not work if enable-recursive-minibuffers
 is on and some input method is set.
Date: Wed, 21 Apr 2021 06:21:12 +0000

[Message part 1 (text/plain, inline)]

>> Additionally, on emacs-devel Zhiwei Chen said this:
>>
>>  It failed to work when buffer is auto selected via
>>  `display-buffer-alist’.  When the point moves to the *Occur* buffer,
>>  typing “n”, “p” still sends “n”, “p” to isearch.
>>
>>  (defun display-buffer-select (buffer alist)
>>    (let ((window (display-buffer-below-selected buffer alist)))
>>      (when (window-live-p window)
>>        (select-window window))))
>>
>>  (setq display-buffer-alist '(("\\*Occur\\*" (display-buffer-select))))
>> 
>> Maybe this could be handled as well?
>
> And this one is strange, too, for two reasons:
>
> - display-buffer-select is not a documented action, and does not even 
> appear in the sources (even Google does not find it!), yet it works;
>

Whooops, I guess I was becoming a bit tired after too many hours of 
hacking, of course it's defined right above ;-)  Anyway, the other 
question is relevant :

>
> - After this action, point has moved, but (current-buffer) does not 
> return the buffer where point is; (window-buffer (selected-window)) 
> does.  It seems to me that at the top-level these two should always be 
> equal; apparently they are not.
>
> Again it's not clear to me whether the bug is here or elsewhere, but the 
> attached patch fixes the original problem and the two bugs.
>
> Cc'ing Martin, who may have some insights on the above two points.
>

Message #31 received at 47894 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Gregory Heytings <gregory <at> heytings.org>, Juri Linkov <juri <at> linkov.net>
Cc: max.brieiev <at> gmail.com, 47894 <at> debbugs.gnu.org
Subject: Re: bug#47894: 28.0.50; isearch does not work if
 enable-recursive-minibuffers is on and some input method is set.
Date: Wed, 21 Apr 2021 09:03:27 +0200

>>  It failed to work when buffer is auto selected via
>>  `display-buffer-alist’.  When the point moves to the *Occur* buffer,
>>  typing “n”, “p” still sends “n”, “p” to isearch.
>>
>>  (defun display-buffer-select (buffer alist)
>>    (let ((window (display-buffer-below-selected buffer alist)))
>>      (when (window-live-p window)
>>        (select-window window))))
>>
>>  (setq display-buffer-alist '(("\\*Occur\\*" (display-buffer-select))))
>>
>> Maybe this could be handled as well?
>>
>
> And this one is strange, too, for two reasons:
>
> - display-buffer-select is not a documented action, and does not even appear in the sources (even Google does not find it!), yet it works;

I don't understand you here.  It is defined above as a function based on
`display-buffer-below-selected' with the additional twist that it tries
to select the window chosen.  Which is arguably not the task of a
display buffer action function - `pop-to-buffer' should be used for that
purpose.

> - After this action, point has moved, but (current-buffer) does not return the buffer where point is; (window-buffer (selected-window)) does.  It seems to me that at the top-level these two should always be equal; apparently they are not.

They need not be equal - only command_loop_1 ascertains that once here

      /* Make sure the current window's buffer is selected.  */
      set_buffer_internal (XBUFFER (XWINDOW (selected_window)->contents));

but `display-buffer' can be called anywhere.  The major point is that,
due to the fact that `display-buffer' may pop up a new frame and the WM
will usually focus that frame, an application can _never_ be sure which
window will get selected and which buffer will be current after calling
`display-buffer' finished.

martin

Message #34 received at 47894 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: martin rudalics <rudalics <at> gmx.at>
Cc: max.brieiev <at> gmail.com, 47894 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#47894: 28.0.50;
 isearch does not work if enable-recursive-minibuffers
 is on and some input method is set.
Date: Wed, 21 Apr 2021 07:16:31 +0000

>> - display-buffer-select is not a documented action, and does not even 
>> appear in the sources (even Google does not find it!), yet it works;
>
> I don't understand you here.  It is defined above
>

Yes, I guess I was a bit tired when I wrote this ;-)  Sorry for the noise.

>> - After this action, point has moved, but (current-buffer) does not 
>> return the buffer where point is; (window-buffer (selected-window)) 
>> does.  It seems to me that at the top-level these two should always be 
>> equal; apparently they are not.
>
> They need not be equal - only command_loop_1 ascertains that once here
>
>      /* Make sure the current window's buffer is selected.  */
>      set_buffer_internal (XBUFFER (XWINDOW (selected_window)->contents));
>
> but `display-buffer' can be called anywhere.  The major point is that, 
> due to the fact that `display-buffer' may pop up a new frame and the WM 
> will usually focus that frame, an application can _never_ be sure which 
> window will get selected and which buffer will be current after calling 
> `display-buffer' finished.
>

Okay, thanks for the clarification.  IIUC the right way to determine what 
the "current buffer" is (from a user's point of view: in which buffer will 
"a" be added if I press "a") is what I do: (window-buffer 
(selected-window)) and not what I did: (current-buffer)?

Message #37 received at 47894 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: max.brieiev <at> gmail.com, 47894 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#47894: 28.0.50; isearch does not work if
 enable-recursive-minibuffers is on and some input method is set.
Date: Wed, 21 Apr 2021 09:42:55 +0200

> Okay, thanks for the clarification.  IIUC the right way to determine
> what the "current buffer" is (from a user's point of view: in which
> buffer will "a" be added if I press "a") is what I do: (window-buffer
> (selected-window)) and not what I did: (current-buffer)?

For users (eq (current-buffer) (window-buffer)) _should_ be invariant.
When and if an application temporarily violates that invariant, it
should reestablish it before the user can see it.  So if an application
calls `display-buffer' in a state where the invariant does not hold, it
should handle that case including the complication that `display-buffer'
might have selected another window.  And it goes without saying that a
display buffer action should never violate that invariant.

martin

Message #40 received at 47894 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: martin rudalics <rudalics <at> gmx.at>
Cc: max.brieiev <at> gmail.com, 47894 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#47894: 28.0.50;
 isearch does not work if enable-recursive-minibuffers
 is on and some input method is set.
Date: Wed, 21 Apr 2021 07:49:28 +0000

>> Okay, thanks for the clarification.  IIUC the right way to determine 
>> what the "current buffer" is (from a user's point of view: in which 
>> buffer will "a" be added if I press "a") is what I do: (window-buffer 
>> (selected-window)) and not what I did: (current-buffer)?
>
> For users (eq (current-buffer) (window-buffer)) _should_ be invariant. 
> When and if an application temporarily violates that invariant, it 
> should reestablish it before the user can see it.  So if an application 
> calls `display-buffer' in a state where the invariant does not hold, it 
> should handle that case including the complication that `display-buffer' 
> might have selected another window.  And it goes without saying that a 
> display buffer action should never violate that invariant.
>

I see.  So in this case the bug was elsewhere as I thought, it's 
display-buffer-select which was wrong (as you said it should have used 
pop-to-buffer) and not the code I added in isearch-post-command-hook. 
Anyway using (window-buffer (selected-window)) should not harm, and is an 
extra safety against display buffer actions doing something weird.

Message #43 received at 47894 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: max.brieiev <at> gmail.com, martin rudalics <rudalics <at> gmx.at>,
 47894 <at> debbugs.gnu.org
Subject: Re: bug#47894: 28.0.50; isearch does not work if
 enable-recursive-minibuffers is on and some input method is set.
Date: Wed, 21 Apr 2021 20:02:02 +0300

[Message part 1 (text/plain, inline)]

>> For users (eq (current-buffer) (window-buffer)) _should_ be
>> invariant. When and if an application temporarily violates that
>> invariant, it should reestablish it before the user can see it.  So if an
>> application calls `display-buffer' in a state where the invariant does
>> not hold, it should handle that case including the complication that
>> `display-buffer' might have selected another window.  And it goes without
>> saying that a display buffer action should never violate that invariant.
>
> I see.  So in this case the bug was elsewhere as I thought, it's
> display-buffer-select which was wrong (as you said it should have used
> pop-to-buffer) and not the code I added in
> isearch-post-command-hook. Anyway using (window-buffer (selected-window))
> should not harm, and is an extra safety against display buffer actions
> doing something weird.

As this bug report indicates, automatically exiting isearch does more harm.
So rather than forcibly exit isearch, we could select the original window
back, in the same vein as isearch-back-into-window in the same hook
moves point back to the old window boundaries:

[isearch-post-command-hook-select-window.patch (text/x-diff, inline)]

diff --git a/lisp/isearch.el b/lisp/isearch.el
index f1e6e3eba2..1dfb0c86fc 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -3052,6 +3057,8 @@ isearch-pre-command-hook
       (isearch-clean-overlays)))))
 
 (defun isearch-post-command-hook ()
+   (unless (eq (selected-window) (old-selected-window))
+     (select-window (old-selected-window)))
    (when isearch-pre-scroll-point
      (let ((ab-bel (isearch-string-out-of-window isearch-pre-scroll-point)))
        (if ab-bel

Message #46 received at 47894 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: max.brieiev <at> gmail.com, martin rudalics <rudalics <at> gmx.at>,
 47894 <at> debbugs.gnu.org
Subject: Re: bug#47894: 28.0.50;
 isearch does not work if enable-recursive-minibuffers
 is on and some input method is set.
Date: Wed, 21 Apr 2021 17:18:36 +0000

>>> For users (eq (current-buffer) (window-buffer)) _should_ be invariant. 
>>> When and if an application temporarily violates that invariant, it 
>>> should reestablish it before the user can see it.  So if an 
>>> application calls `display-buffer' in a state where the invariant does 
>>> not hold, it should handle that case including the complication that 
>>> `display-buffer' might have selected another window.  And it goes 
>>> without saying that a display buffer action should never violate that 
>>> invariant.
>>
>> I see.  So in this case the bug was elsewhere as I thought, it's 
>> display-buffer-select which was wrong (as you said it should have used 
>> pop-to-buffer) and not the code I added in isearch-post-command-hook. 
>> Anyway using (window-buffer (selected-window)) should not harm, and is 
>> an extra safety against display buffer actions doing something weird.
>
> As this bug report indicates, automatically exiting isearch does more 
> harm.
>

I guess it's a matter of interpretation here.  In this bug report isearch 
was automatically exited by error, because isearch-post-command-hook is 
executed inside the minibuffer (I'm not sure it should be).

On the contrary, what Zhiwei Chen asked is what the patch does: exit 
isearch when point has moved to another window at the request of the user.

>
> So rather than forcibly exit isearch, we could select the original 
> window back, in the same vein as isearch-back-into-window in the same 
> hook moves point back to the old window boundaries:
>

That would be the opposite of what Zhiwei Chen asked (twice), but I won't 
fight for him.

Message #49 received at 47894 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: max.brieiev <at> gmail.com, martin rudalics <rudalics <at> gmx.at>,
 47894 <at> debbugs.gnu.org
Subject: Re: bug#47894: 28.0.50; isearch does not work if
 enable-recursive-minibuffers is on and some input method is set.
Date: Wed, 21 Apr 2021 20:37:07 +0300

> On the contrary, what Zhiwei Chen asked is what the patch does: exit
> isearch when point has moved to another window at the request of the user.

Anyone who wants to exit isearch, needs to do this explicitly,
where the request of the user means typing a key that exits isearch.

>> So rather than forcibly exit isearch, we could select the original window
>> back, in the same vein as isearch-back-into-window in the same hook moves
>> point back to the old window boundaries:
>
> That would be the opposite of what Zhiwei Chen asked (twice), but I won't
> fight for him.

His example used the command isearch-occur.  This command
is exceptional - it doesn't exit intentionally, to be able
to show matches in another window without exiting isearch.

So the right customization for him is

  (advice-add 'isearch-occur :after
              (lambda (&rest _args)
                (isearch-done nil t)
                (isearch-clean-overlays)))

Message #52 received at 47894 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: max.brieiev <at> gmail.com, 47894 <at> debbugs.gnu.org
Subject: Re: bug#47894: 28.0.50; isearch does not work if
 enable-recursive-minibuffers is on and some input method is set.
Date: Wed, 21 Apr 2021 20:59:19 +0300

[Message part 1 (text/plain, inline)]

> So rather than forcibly exit isearch, we could select the original window back

I see two cases that could go wrong:

1. another window is selected by display-buffer-alist.
   This can be fixed by selecting an old window back.

2. another buffer is displayed in the same window
   by display-buffer-alist.

Isearch help commands solve the second problem by simply using
isearch--display-help-action that inhibits displaying other buffers
in the same window.

Instead of let-binding display-buffer-overriding-action
in all isearch help commands, we could set it temporarily
like we already temporarily set overriding-terminal-local-map:

[isearch--saved-overriding-action.patch (text/x-diff, inline)]

diff --git a/lisp/isearch.el b/lisp/isearch.el
index f1e6e3eba2..1dfb0c86fc 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -499,32 +499,28 @@ isearch--display-help-action
 (defun isearch-help-for-help ()
   "Display Isearch help menu."
   (interactive)
-  (let ((display-buffer-overriding-action isearch--display-help-action))
-    (isearch-help-for-help-internal))
+  (isearch-help-for-help-internal)
   (isearch-update))
 
 (defun isearch-describe-bindings ()
   "Show a list of all keys defined in Isearch mode, and their definitions.
 This is like `describe-bindings', but displays only Isearch keys."
   (interactive)
-  (let ((display-buffer-overriding-action isearch--display-help-action))
-    (with-help-window "*Help*"
-      (with-current-buffer standard-output
-	(princ "Isearch Mode Bindings:\n")
-	(princ (substitute-command-keys "\\{isearch-mode-map}"))))))
+  (with-help-window "*Help*"
+    (with-current-buffer standard-output
+      (princ "Isearch Mode Bindings:\n")
+      (princ (substitute-command-keys "\\{isearch-mode-map}")))))
 
 (defun isearch-describe-key ()
   "Display documentation of the function invoked by isearch key."
   (interactive)
-  (let ((display-buffer-overriding-action isearch--display-help-action))
-    (call-interactively 'describe-key))
+  (call-interactively 'describe-key)
   (isearch-update))
 
 (defun isearch-describe-mode ()
   "Display documentation of Isearch mode."
   (interactive)
-  (let ((display-buffer-overriding-action isearch--display-help-action))
-    (describe-function 'isearch-forward))
+  (describe-function 'isearch-forward)
   (isearch-update))
 
 (defalias 'isearch-mode-help 'isearch-describe-mode)
@@ -953,6 +949,7 @@ isearch-hidden
 (defvar isearch-input-method-function nil)
 
 (defvar isearch--saved-overriding-local-map nil)
+(defvar isearch--saved-overriding-action nil)
 
 ;; Minor-mode-alist changes - kind of redundant with the
 ;; echo area, but if isearching in multiple windows, it can be useful.
@@ -1278,6 +1280,8 @@ isearch-mode
   (setq	isearch-mode " Isearch")  ;; forward? regexp?
   (force-mode-line-update)
 
+  (setq isearch--saved-overriding-action display-buffer-overriding-action
+        display-buffer-overriding-action isearch--display-help-action)
   (setq overriding-terminal-local-map isearch-mode-map)
   (run-hooks 'isearch-mode-hook)
   ;; Remember the initial map possibly modified
@@ -1400,6 +1404,7 @@ isearch-done
   ;; Called by all commands that terminate isearch-mode.
   ;; If NOPUSH is non-nil, we don't push the string on the search ring.
   (setq overriding-terminal-local-map nil)
+  (setq display-buffer-overriding-action isearch--saved-overriding-action)
   ;; (setq pre-command-hook isearch-old-pre-command-hook) ; for lemacs
   (setq minibuffer-message-timeout isearch-original-minibuffer-message-timeout)
   (isearch-dehighlight)
@@ -3052,6 +3057,8 @@ isearch-pre-command-hook
       (isearch-clean-overlays)))))
 
 (defun isearch-post-command-hook ()
+   (unless (eq (selected-window) (old-selected-window))
+     (select-window (old-selected-window)))
    (when isearch-pre-scroll-point
      (let ((ab-bel (isearch-string-out-of-window isearch-pre-scroll-point)))
        (if ab-bel

Message #55 received at 47894 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: max.brieiev <at> gmail.com, 47894 <at> debbugs.gnu.org
Subject: Re: bug#47894: 28.0.50; isearch does not work if
 enable-recursive-minibuffers is on and some input method is set.
Date: Wed, 21 Apr 2021 22:23:59 +0300

tags 47894 fixed
close 47894 28.0.50
quit

> Isearch help commands solve the second problem by simply using
> isearch--display-help-action that inhibits displaying other buffers
> in the same window.
>
> Instead of let-binding display-buffer-overriding-action
> in all isearch help commands, we could set it temporarily
> like we already temporarily set overriding-terminal-local-map:

Actually, I withdraw my patches.  Better not to make any assumptions
about possible ways how isearch is used because of existence of
such complex cases like with input methods in this bug report
(fixed and closed).

By default, most commands that want to display a buffer
in another window, exit isearch automatically,
so there is no problem.  Only isearch help commands
and isearch-occur don't exit isearch before displaying
another buffer, thus they need to be treated individually.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.