bug#30187: M-e should restore isearch correctly in special modes

bug#30187: M-e should restore isearch correctly in special modes

[Juri Linkov]

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

There was an old problem that ‘isearch-edit-string’ incorrectly
restored a previous isearch mode in such commands as
‘comint-history-isearch-backward’ and ‘dired-isearch-filenames’
because they let-bind variables that affects the search parameters,
e.g. ‘(let ((dired-isearch-filenames t)))’ only at the starting
isearch, but when ‘M-e’ (isearch-edit-string) exits isearch,
and later starts again, these let-binding are not in effect
and thus restore the wrong search state.

All imaginable solutions were very bad.  But now I found a solution
that at least not bad.  It changes the global value of customizable
variables for the time while isearch is active (including the time
when isearch is suspended temporarily), and restores the original value
afterwards.


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

diff --git a/lisp/comint.el b/lisp/comint.el
index a79e34b..8dba317 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -1434,14 +1434,14 @@ comint-history-isearch
 (defun comint-history-isearch-backward ()
   "Search for a string backward in input history using Isearch."
   (interactive)
-  (let ((comint-history-isearch t))
-    (isearch-backward nil t)))
+  (setq comint-history-isearch t)
+  (isearch-backward nil t))
 
 (defun comint-history-isearch-backward-regexp ()
   "Search for a regular expression backward in input history using Isearch."
   (interactive)
-  (let ((comint-history-isearch t))
-    (isearch-backward-regexp nil t)))
+  (setq comint-history-isearch t)
+  (isearch-backward-regexp nil t))
 
 (defvar-local comint-history-isearch-message-overlay nil)
 
@@ -1472,7 +1472,9 @@ comint-history-isearch-end
   (setq isearch-message-function nil)
   (setq isearch-wrap-function nil)
   (setq isearch-push-state-function nil)
-  (remove-hook 'isearch-mode-end-hook 'comint-history-isearch-end t))
+  (remove-hook 'isearch-mode-end-hook 'comint-history-isearch-end t)
+  (unless isearch-suspended
+    (custom-reevaluate-setting 'comint-history-isearch)))
 
 (defun comint-goto-input (pos)
   "Put input history item of the absolute history position POS."
diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index 223b254..55b68a3 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -2766,7 +2766,9 @@ dired-isearch-filenames-end
   "Clean up the Dired file name search after terminating isearch."
   (define-key isearch-mode-map "\M-sff" nil)
   (dired-isearch-filenames-mode -1)
-  (remove-hook 'isearch-mode-end-hook 'dired-isearch-filenames-end t))
+  (remove-hook 'isearch-mode-end-hook 'dired-isearch-filenames-end t)
+  (unless isearch-suspended
+    (custom-reevaluate-setting 'dired-isearch-filenames)))
 
 (defun dired-isearch-filter-filenames (beg end)
   "Test whether some part of the current search match is inside a file name.
@@ -2779,15 +2781,15 @@ dired-isearch-filter-filenames
 (defun dired-isearch-filenames ()
   "Search for a string using Isearch only in file names in the Dired buffer."
   (interactive)
-  (let ((dired-isearch-filenames t))
-    (isearch-forward nil t)))
+  (setq dired-isearch-filenames t)
+  (isearch-forward nil t))
 
 ;;;###autoload
 (defun dired-isearch-filenames-regexp ()
   "Search for a regexp using Isearch only in file names in the Dired buffer."
   (interactive)
-  (let ((dired-isearch-filenames t))
-    (isearch-forward-regexp nil t)))
+  (setq dired-isearch-filenames t)
+  (isearch-forward-regexp nil t))
 
 \f
 ;; Functions for searching in tags style among marked files.
diff --git a/lisp/isearch.el b/lisp/isearch.el
index 3725779..b131437 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -1233,6 +1233,8 @@ isearch-new-regexp-function
 (define-obsolete-variable-alias 'isearch-new-word
   'isearch-new-regexp-function "25.1")
 
+(defvar isearch-suspended nil)
+
 (defmacro with-isearch-suspended (&rest body)
   "Exit Isearch mode, run BODY, and reinvoke the pending search.
 You can update the global isearch variables by setting new values to
@@ -1299,6 +1301,8 @@ with-isearch-suspended
 	       isearch-original-minibuffer-message-timeout)
 	      old-point old-other-end)
 
+          (setq isearch-suspended t)
+
 	  ;; Actually terminate isearching until editing is done.
 	  ;; This is so that the user can do anything without failure,
 	  ;; like switch buffers and start another isearch, and return.
@@ -1313,6 +1317,8 @@ with-isearch-suspended
 	  (unwind-protect
 	      (progn ,@body)
 
+            (setq isearch-suspended nil)
+
 	    ;; Always resume isearching by restarting it.
 	    (isearch-mode isearch-forward
 			  isearch-regexp
@@ -1374,6 +1380,7 @@ with-isearch-suspended
 		  (message "")))))
 
     (quit  ; handle abort-recursive-edit
+     (setq isearch-suspended nil)
      (isearch-abort)  ;; outside of let to restore outside global values
      )))
 

bug#30187: M-e should restore isearch correctly in special modes

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Sat, 20 Jan 2018 23:10:16 +0200
> 
> All imaginable solutions were very bad.  But now I found a solution
> that at least not bad.  It changes the global value of customizable
> variables for the time while isearch is active (including the time
> when isearch is suspended temporarily), and restores the original value
> afterwards.

What if the user QUITs out of the search -- will the original value be
restored?

Thanks.

bug#30187: M-e should restore isearch correctly in special modes

[Juri Linkov]

>> All imaginable solutions were very bad.  But now I found a solution
>> that at least not bad.  It changes the global value of customizable
>> variables for the time while isearch is active (including the time
>> when isearch is suspended temporarily), and restores the original value
>> afterwards.
>
> What if the user QUITs out of the search -- will the original value be
> restored?

Yes, the original value is restored thanks to ‘condition-case’ with
‘quit’ in ‘with-isearch-suspended’ that guarantees that ‘isearch-abort’
is always called.

bug#30187: M-e should restore isearch correctly in special modes

[Juri Linkov]

>>> All imaginable solutions were very bad.  But now I found a solution
>>> that at least not bad.  It changes the global value of customizable
>>> variables for the time while isearch is active (including the time
>>> when isearch is suspended temporarily), and restores the original value
>>> afterwards.
>>
>> What if the user QUITs out of the search -- will the original value be
>> restored?
>
> Yes, the original value is restored thanks to ‘condition-case’ with
> ‘quit’ in ‘with-isearch-suspended’ that guarantees that ‘isearch-abort’
> is always called.

Done.

bug#30187: M-e should restore isearch correctly in special modes

[Juri Linkov]

> There was an old problem that ‘isearch-edit-string’ incorrectly
> restored a previous isearch mode in such commands as
> ‘comint-history-isearch-backward’ and ‘dired-isearch-filenames’
> because they let-bind variables that affects the search parameters,
> e.g. ‘(let ((dired-isearch-filenames t)))’ only at the starting
> isearch, but when ‘M-e’ (isearch-edit-string) exits isearch,
> and later starts again, these let-binding are not in effect
> and thus restore the wrong search state.

There is another problem with ‘comint-history-isearch-backward’:
when it is called at the end of an “*Async Shell Command*” buffer
it should not activate comint-history isearch, only a normal isearch
should be started.  I tried different approaches, but only workable
solution is to check whether the shell prompt is empty as it is
in all “*Async Shell Command*” buffers:

diff --git a/lisp/comint.el b/lisp/comint.el
index 8dba317..b7179e2 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -1448,10 +1448,17 @@ comint-history-isearch-message-overlay
 (defun comint-history-isearch-setup ()
   "Set up a comint for using Isearch to search the input history.
 Intended to be added to `isearch-mode-hook' in `comint-mode'."
-  (when (or (eq comint-history-isearch t)
-	    (and (eq comint-history-isearch 'dwim)
-		 ;; Point is at command line.
-		 (comint-after-pmark-p)))
+  (when (and (get-buffer-process (current-buffer))
+	     (or (eq comint-history-isearch t)
+		 (and (eq comint-history-isearch 'dwim)
+		      ;; Point is at command line.
+		      (comint-after-pmark-p)
+		      ;; Prompt is not empty like in Async Shell Command buffers
+		      (not (eq (save-excursion
+				 (goto-char (comint-line-beginning-position))
+				 (forward-line 0)
+				 (point))
+			       (comint-line-beginning-position))))))
     (setq isearch-message-prefix-add "history ")
     (setq-local isearch-search-fun-function
                 #'comint-history-isearch-search)