Re: Simple isearch concerns

[Juri Linkov]

>> Is this overriding map there just so that `isearch-mode-map' takes
>> precedence over other minor mode maps?  If so, couldn't this be achieved
>> by manipulating minor-mode-map-alist to put Isearch's map at the top?
>> Maybe that can be done buffer-locally.
>
> According to (info "(elisp) Searching Keymaps") the next keymap to try is
> `overriding-local-map`.  It can be set buffer-locally.

Actually, overriding-local-map can't be used, because it prioritizes
global-map over local maps, so e.g. in Dired typing C-o exits isearch
and calls open-line instead of dired-display-file.

According to (info "(elisp) Searching Keymaps"), the next available keymap is
emulation-mode-map-alists.  The following patch added a single line:

  (add-to-list 'emulation-mode-map-alists `((isearch-mode . ,isearch-mode-map)))

then removed all mentions of overriding-terminal-local-map,
removed all 156 lines of the monstrous macro with-isearch-suspended,
added a single line to the end of isearch-edit-string:

  (isearch-search-and-update)

and then M-e edits the search string without exiting isearch.  And it's
trivial to use isearch commands without exiting the minibuffer with e.g.

  (with-minibuffer-selected-window (isearch-repeat-forward))

And everything works fine.  It's too good to be true,
but I don't see where's the catch.

diff --git a/lisp/isearch.el b/lisp/isearch.el
index 9f3cfd70fb..bbf091fd0e 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -668,6 +668,8 @@ isearch-mode-map
     map)
   "Keymap for `isearch-mode'.")
 
+(add-to-list 'emulation-mode-map-alists `((isearch-mode . ,isearch-mode-map)))
+
 (easy-menu-define isearch-menu-bar-map  isearch-mode-map
   "Menu for `isearch-mode'."
   '("Isearch"
@@ -952,8 +954,6 @@ isearch-hidden
 ;; The value of input-method-function when isearch is invoked.
 (defvar isearch-input-method-function nil)
 
-(defvar isearch--saved-overriding-local-map nil)
-
 ;; Minor-mode-alist changes - kind of redundant with the
 ;; echo area, but if isearching in multiple windows, it can be useful.
 ;; Also, clicking the mode-line indicator pops up
@@ -1295,11 +1295,7 @@ isearch-mode
   (setq        isearch-mode " Isearch")  ;; forward? regexp?
   (force-mode-line-update)
 
-  (setq overriding-terminal-local-map isearch-mode-map)
   (run-hooks 'isearch-mode-hook)
-  ;; Remember the initial map possibly modified
-  ;; by external packages in isearch-mode-hook.  (Bug#16035)
-  (setq isearch--saved-overriding-local-map overriding-terminal-local-map)
 
   ;; Pushing the initial state used to be before running isearch-mode-hook,
   ;; but a hook might set `isearch-push-state-function' used in
@@ -1308,10 +1304,10 @@ isearch-mode
 
   (isearch-update)
 
-  (add-hook 'pre-command-hook 'isearch-pre-command-hook)
-  (add-hook 'post-command-hook 'isearch-post-command-hook)
-  (add-hook 'mouse-leave-buffer-hook 'isearch-mouse-leave-buffer)
-  (add-hook 'kbd-macro-termination-hook 'isearch-done)
+  (add-hook 'pre-command-hook 'isearch-pre-command-hook nil t)
+  (add-hook 'post-command-hook 'isearch-post-command-hook nil t)
+  (add-hook 'mouse-leave-buffer-hook 'isearch-mouse-leave-buffer nil t)
+  (add-hook 'kbd-macro-termination-hook 'isearch-done nil t)
 
   ;; isearch-mode can be made modal (in the sense of not returning to
   ;; the calling function until searching is completed) by entering
@@ -1406,10 +1402,11 @@ isearch-done
                                      ,isearch-message
                                      ',isearch-case-fold-search)))
 
-  (remove-hook 'pre-command-hook 'isearch-pre-command-hook)
-  (remove-hook 'post-command-hook 'isearch-post-command-hook)
-  (remove-hook 'mouse-leave-buffer-hook 'isearch-mouse-leave-buffer)
-  (remove-hook 'kbd-macro-termination-hook 'isearch-done)
+  (remove-hook 'pre-command-hook 'isearch-pre-command-hook t)
+  (remove-hook 'post-command-hook 'isearch-post-command-hook t)
+  (remove-hook 'mouse-leave-buffer-hook 'isearch-mouse-leave-buffer t)
+  (remove-hook 'kbd-macro-termination-hook 'isearch-done t)
+
   (when (buffer-live-p isearch--current-buffer)
     (with-current-buffer isearch--current-buffer
       (setq isearch--current-buffer nil)
@@ -1417,7 +1414,6 @@ 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 pre-command-hook isearch-old-pre-command-hook) ; for lemacs
   (setq minibuffer-message-timeout isearch-original-minibuffer-message-timeout)
   (isearch-dehighlight)
@@ -1792,32 +1788,32 @@ isearch-edit-string
 \\[isearch-reverse-exit-minibuffer] to resume isearching backward.
 \\[isearch-complete-edit] to complete the search string using the search ring."
   (interactive)
-  (with-isearch-suspended
-   (let* ((message-log-max nil)
-         ;; Don't add a new search string to the search ring here
-         ;; in `read-from-minibuffer'. It should be added only
-         ;; by `isearch-update-ring' called from `isearch-done'.
-         (history-add-new-input nil)
-         ;; Binding minibuffer-history-symbol to nil is a work-around
-         ;; for some incompatibility with gmhist.
-         (minibuffer-history-symbol)
-         ;; Search string might have meta information on text properties.
-         (minibuffer-allow-text-properties t))
-     (setq isearch-new-string
-          (read-from-minibuffer
-           (isearch-message-prefix nil isearch-nonincremental)
-           (cons isearch-string (1+ (or (isearch-fail-pos)
-                                        (length isearch-string))))
-           minibuffer-local-isearch-map nil
-           (if isearch-regexp
-               (cons 'regexp-search-ring
-                     (1+ (or regexp-search-ring-yank-pointer -1)))
-             (cons 'search-ring
-                   (1+ (or search-ring-yank-pointer -1))))
-           nil t)
-          isearch-new-message
-          (mapconcat 'isearch-text-char-description
-                     isearch-new-string "")))))
+  (let* ((message-log-max nil)
+        ;; Don't add a new search string to the search ring here
+        ;; in `read-from-minibuffer'. It should be added only
+        ;; by `isearch-update-ring' called from `isearch-done'.
+        (history-add-new-input nil)
+        ;; Binding minibuffer-history-symbol to nil is a work-around
+        ;; for some incompatibility with gmhist.
+        (minibuffer-history-symbol)
+        ;; Search string might have meta information on text properties.
+        (minibuffer-allow-text-properties t))
+    (setq isearch-string
+         (read-from-minibuffer
+          (isearch-message-prefix nil isearch-nonincremental)
+          (cons isearch-string (1+ (or (isearch-fail-pos)
+                                       (length isearch-string))))
+          minibuffer-local-isearch-map nil
+          (if isearch-regexp
+              (cons 'regexp-search-ring
+                    (1+ (or regexp-search-ring-yank-pointer -1)))
+            (cons 'search-ring
+                  (1+ (or search-ring-yank-pointer -1))))
+          nil t)
+         isearch-message
+         (mapconcat 'isearch-text-char-description
+                    isearch-string ""))
+    (isearch-search-and-update)))
 
 (defun isearch-nonincremental-exit-minibuffer ()
   (interactive)
@@ -2599,7 +2595,7 @@ isearch-mouse-2
 is bound to outside of Isearch."
   (interactive "e")
   (let ((w (posn-window (event-start click)))
-        (binding (let ((overriding-terminal-local-map nil)
+        (binding (let (
                        ;; Key search depends on mode (bug#47755)
                        (isearch-mode nil))
                    (key-binding (this-command-keys-vector) t))))
@@ -2715,16 +2711,16 @@ isearch-char-by-name
 Completion is available like in `read-char-by-name' used by `insert-char'.
 With argument, add COUNT copies of the character."
   (interactive "p")
-  (with-isearch-suspended
-   (let ((char (read-char-by-name "Add character to search (Unicode name or 
hex): ")))
-     (when char
-       (let ((string (if (and (integerp count) (> count 1))
-                        (make-string count char)
-                      (char-to-string char))))
-        (setq isearch-new-string (concat isearch-string string)
-              isearch-new-message (concat isearch-message
-                                          (mapconcat 
'isearch-text-char-description
-                                                     string ""))))))))
+  (let ((char (read-char-by-name "Add character to search (Unicode name or 
hex): ")))
+    (when char
+      (let ((string (if (and (integerp count) (> count 1))
+                       (make-string count char)
+                     (char-to-string char))))
+       (setq isearch-string (concat isearch-string string)
+             isearch-message (concat isearch-message
+                                     (mapconcat 'isearch-text-char-description
+                                                string ""))))))
+  (isearch-search-and-update))
 
 (defun isearch-search-and-update ()
   "Do the search and update the display."
@@ -3016,9 +3012,6 @@ isearch-pre-command-hook
   (let* ((key (this-single-command-keys))
         (main-event (aref key 0)))
     (cond
-     ;; Don't exit Isearch if we're in the middle of some
-     ;; `set-transient-map' thingy like `universal-argument--mode'.
-     ((not (eq overriding-terminal-local-map 
isearch--saved-overriding-local-map)))
      ;; Don't exit Isearch for isearch key bindings.
      ((or (commandp (lookup-key isearch-mode-map key nil))
           (commandp