bug#47894: 28.0.50; y.oy

bug#47894: 28.0.50; y.oy

[max.brieiev]

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

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.


[-- Attachment #2: isearch_screenshot --]
[-- Type: image/png, Size: 11669 bytes --]

[-- Attachment #3: Type: text/plain, Size: 4271 bytes --]



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))

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

[max.brieiev]

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

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

[Juri Linkov]

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.

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

[Juri Linkov]

> 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?

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

[Gregory Heytings]

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


>
> 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.

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

[Gregory Heytings]

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


>
> 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.

[-- Attachment #2: Type: text/x-diff, Size: 1492 bytes --]

From 32456fad0d683db7a154220b1b4a57946c84b410 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Tue, 20 Apr 2021 21:42:32 +0000
Subject: [PATCH] Terminate isearch when point has moved to another buffer

* lisp/isearch.el (isearch-post-command-hook): Terminate isearch
when the command just executed has moved point to another buffer.
https://lists.gnu.org/archive/html/emacs-devel/2021-04/msg00309.html

This improves commit ff796823e5 for the case when an input method is
used and enable-recursive-minibuffers is set (Bug#47894).
---
 lisp/isearch.el | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lisp/isearch.el b/lisp/isearch.el
index 69fdc9df6d..8123da90c9 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -3057,9 +3057,10 @@ isearch-post-command-hook
          (isearch-search-and-update)))
      (setq isearch-pre-move-point nil))
   ;; Terminate the search if point has moved to another buffer.
-  (unless (eq isearch--current-buffer (current-buffer))
-    (when (buffer-live-p isearch--current-buffer)
-      (with-current-buffer isearch--current-buffer (isearch-exit))))
+  (unless (minibufferp (current-buffer))
+    (unless (eq isearch--current-buffer (current-buffer))
+      (when (buffer-live-p isearch--current-buffer)
+        (with-current-buffer isearch--current-buffer (isearch-exit)))))
   (force-mode-line-update))
 
 (defun isearch-quote-char (&optional count)
-- 
2.30.2

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

[Gregory Heytings]

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


>
> 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.

[-- Attachment #2: Type: text/x-diff, Size: 1686 bytes --]

From fc1399ffbbc1385536ce03237813092c6a804adc Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Tue, 20 Apr 2021 22:13:27 +0000
Subject: [PATCH] Terminate isearch when point has moved to another buffer

* lisp/isearch.el (isearch-post-command-hook): Terminate isearch
when the command just executed has moved point to another buffer.
https://lists.gnu.org/archive/html/emacs-devel/2021-04/msg00309.html

This improves commit ff796823e5 for the cases when an input method is
used and enable-recursive-minibuffers is set (Bug#47894), and when a
buffer is automatically selected with a display-buffer-select in
display-buffer-alist (see
https://lists.gnu.org/archive/html/emacs-devel/2021-04/msg00458.html )
---
 lisp/isearch.el | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lisp/isearch.el b/lisp/isearch.el
index 69fdc9df6d..d4763f21f9 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -3057,9 +3057,10 @@ isearch-post-command-hook
          (isearch-search-and-update)))
      (setq isearch-pre-move-point nil))
   ;; Terminate the search if point has moved to another buffer.
-  (unless (eq isearch--current-buffer (current-buffer))
-    (when (buffer-live-p isearch--current-buffer)
-      (with-current-buffer isearch--current-buffer (isearch-exit))))
+  (unless (minibufferp (current-buffer))
+    (unless (eq isearch--current-buffer (window-buffer (selected-window)))
+      (when (buffer-live-p isearch--current-buffer)
+        (with-current-buffer isearch--current-buffer (isearch-exit)))))
   (force-mode-line-update))
 
 (defun isearch-quote-char (&optional count)
-- 
2.30.2

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

[Gregory Heytings]

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


>> 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.
>

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

[martin rudalics]

 >>  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

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

[Gregory Heytings]

>> - 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)?

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

[martin rudalics]

 > 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

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

[Gregory Heytings]

>> 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.

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

[Juri Linkov]

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

>> 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:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: isearch-post-command-hook-select-window.patch --]
[-- Type: text/x-diff, Size: 483 bytes --]

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

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

[Gregory Heytings]

>>> 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.

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

[Juri Linkov]

> 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)))

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

[Juri Linkov]

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

> 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:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: isearch--saved-overriding-action.patch --]
[-- Type: text/x-diff, Size: 3220 bytes --]

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

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

[Juri Linkov]

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.