bug#74287: [PATCH] Rework history Isearch for Eshell

[Pengji Zhang <me@pengjiz.com>]

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

Juri Linkov <juri@linkov.net> writes:

> (There is only one typo in the variable name
> 'eshell--history-isearch-messasge-overlay'.)

Thanks! Corrected in the attached updated patch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Updated patch --]
[-- Type: text/x-patch, Size: 20730 bytes --]

From 9df1d8eaf626c396b88fe616a6e89d5a8e7b963c Mon Sep 17 00:00:00 2001
From: Pengji Zhang <me@pengjiz.com>
Date: Thu, 5 Dec 2024 20:25:25 +0800
Subject: [PATCH] Rework history Isearch for Eshell

This is to make history Isearch for Eshell similar to that of
'comint-mode', by hooking into Isearch properly instead of
defining new commands to emulate Isearch.

* lisp/eshell/em-hist.el (eshell-history-isearch): New user
option.
(eshell-goto-history, eshell--isearch-setup)
(eshell-history-isearch-end, eshell-history-isearch-search)
(eshell-history-isearch-message, eshell-history-isearch-wrap)
(eshell-history-isearch-push-state): New functions.
(eshell-isearch-backward-regexp, eshell-isearch-forward-regexp):
New commands.
(eshell--history-isearch-messasge-overlay)
(eshell--stored-incomplete-input): New internal variables.
(eshell-hist-mode-map): Bind 'M-r' to
'eshell-isearch-backward-regexp' and free 'M-s' binding for
normal in-buffer search commands.
(eshell-isearch-backward, eshell-isearch-forward): Use the new
way to start searching.
(eshell-hist-initialize): Use the new Isearch setup function.
(eshell-previous-matching-input): Use 'eshell-goto-history'.
Also inhibit messages when searching.
(eshell-isearch-map, eshell-isearch-repeat-backward)
(eshell-isearch-abort, eshell-isearch-delete-char)
(eshell-isearch-return, eshell-isearch-cancel)
(eshell-isearch-repeat-forward, eshell-test-imatch)
(eshell-return-to-prompt, eshell-prepare-for-search): Remove.
These are for the old history Isearch implementation.

* doc/misc/eshell.texi (History): Document changes.
* etc/NEWS: Annouce changes.
---
 doc/misc/eshell.texi   |  15 +-
 etc/NEWS               |  23 +++
 lisp/eshell/em-hist.el | 322 ++++++++++++++++++++++++-----------------
 3 files changed, 223 insertions(+), 137 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index ee4d0ca09c8..701137ea1b4 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -2661,10 +2661,10 @@ History
 
 @table @kbd
 @kindex M-r
-@kindex M-s
 @item M-r
-@itemx M-s
-History I-search.
+History I-search.  @kbd{M-r} starts an incremental search in input
+history.  While searching, type @kbd{C-r} to move to the previous match,
+and @kbd{C-s} to move to the next match in the input history.
 
 @kindex M-p
 @kindex M-n
@@ -2675,6 +2675,15 @@ History
 previous or next line that begins with that string.
 @end table
 
+@vindex eshell-history-isearch
+If you would like to use the default Isearch key-bindings to search
+through input history, you may customize @code{eshell-history-isearch}
+to @code{t}.  That makes, for example, @kbd{C-r} and @kbd{C-M-r} in an
+Eshell buffer search in input history only.  In addition, if the value
+of @code{eshell-history-isearch} is @code{dwim}, those commands search
+in the history when the point is after the last prompt, and search in
+the buffer when the point is before or within the last prompt.
+
 @node Extension modules
 @chapter Extension modules
 Eshell provides a facility for defining extension modules so that they
diff --git a/etc/NEWS b/etc/NEWS
index d1c7303f976..c1df6f78d9f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -288,6 +288,29 @@ This hook runs after an Eshell session has been fully initialized,
 immediately before running 'eshell-post-command-hook' for the first
 time.
 
++++
+*** Improved history Isearch.
+History Isearch in Eshell is reworked.  Two new commands
+'eshell-isearch-backward-regexp' and 'eshell-isearch-forward-regexp' are
+added for incrementally searching through the input history.
+'eshell-isearch-backward-regexp' is bound to 'M-r' by default, and 'M-s'
+is freed for normal search commands.  If you would like to restore the
+previous key-bindings for the non-incremental search commands, put in
+your configuration:
+
+    (keymap-set eshell-hist-mode-map "M-r"
+                #'eshell-previous-matching-input)
+    (keymap-set eshell-hist-mode-map "M-s"
+                #'eshell-next-matching-input)
+
++++
+*** New user option 'eshell-history-isearch'
+When 'eshell-history-isearch' is nil (the default), Isearch commands
+search in the buffer contents.  If you customize it to t, those commands
+only search in input history.  If you customize it to the symbol 'dwim',
+those commands search in input history only when the point is after the
+last prompt.
+
 ** SHR
 
 +++
diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el
index fffd611c06f..c85e7704eca 100644
--- a/lisp/eshell/em-hist.el
+++ b/lisp/eshell/em-hist.el
@@ -34,7 +34,6 @@
 ;; Also, most of `comint-mode's keybindings are accepted:
 ;;
 ;;   M-r     ; search backward for a previous command by regexp
-;;   M-s     ; search forward for a previous command by regexp
 ;;   M-p     ; access the last command entered, repeatable
 ;;   M-n     ; access the first command entered, repeatable
 ;;
@@ -132,6 +131,17 @@ eshell-input-filter
                 (function :tag "Other function"))
   :risky t)
 
+(defcustom eshell-history-isearch nil
+  "Non-nil to Isearch in input history only.
+If t, usual Isearch keys like \\[isearch-forward] in Eshell search in
+the input history only.  If `dwim', Isearch in the input history when
+point is at the command line, otherwise search in the current Eshell
+buffer."
+  :type '(choice (const :tag "Don't search in input history" nil)
+                 (const :tag "Search histroy when point is on command line" dwim)
+                 (const :tag "Always search in input history" t))
+  :version "31.1")
+
 (defun eshell-hist--update-keymap (symbol value)
   "Update `eshell-hist-mode-map' for `eshell-hist-match-partial'."
   ;; Don't try to set this before it is bound.  See below.
@@ -204,25 +214,17 @@ eshell-save-history-index
 (defvar eshell-hist--new-items nil
   "The number of new history items that have not been written to
 file.  This variable is local in each eshell buffer.")
-
-(defvar-keymap eshell-isearch-map
-  :doc "Keymap used in isearch in Eshell."
-  :parent isearch-mode-map
-  "C-m"         #'eshell-isearch-return
-  "C-r"         #'eshell-isearch-repeat-backward
-  "C-s"         #'eshell-isearch-repeat-forward
-  "C-g"         #'eshell-isearch-abort
-  "<backspace>" #'eshell-isearch-delete-char
-  "<delete>"    #'eshell-isearch-delete-char
-  "C-c C-c"     #'eshell-isearch-cancel)
+(defvar-local eshell--history-isearch-message-overlay nil
+  "Overlay for Isearch message when searching through input history.")
+(defvar-local eshell--stored-incomplete-input nil
+  "Stored input for history cycling.")
 
 (defvar-keymap eshell-hist-mode-map
   "<up>"     #'eshell-previous-matching-input-from-input
   "<down>"   #'eshell-next-matching-input-from-input
   "C-<up>"   #'eshell-previous-input
   "C-<down>" #'eshell-next-input
-  "M-r"      #'eshell-previous-matching-input
-  "M-s"      #'eshell-next-matching-input
+  "M-r"      #'eshell-isearch-backward-regexp
   "C-c M-r"  #'eshell-previous-matching-input-from-input
   "C-c M-s"  #'eshell-next-matching-input-from-input
   "C-c C-l"  #'eshell-list-history
@@ -261,20 +263,9 @@ eshell-hist-initialize
 	   (not eshell-non-interactive-p))
       (let ((rebind-alist eshell-rebind-keys-alist))
         (setq-local eshell-rebind-keys-alist
-	      (append rebind-alist eshell-hist-rebind-keys-alist))
-        (setq-local search-invisible t)
-        (setq-local search-exit-option t)
-	(add-hook 'isearch-mode-hook
-                  (lambda ()
-                    (if (>= (point) eshell-last-output-end)
-                        (setq overriding-terminal-local-map
-                              eshell-isearch-map)))
-                  nil t)
-	(add-hook 'isearch-mode-end-hook
-                  (lambda ()
-                    (setq overriding-terminal-local-map nil))
-                  nil t))
+	            (append rebind-alist eshell-hist-rebind-keys-alist)))
     (eshell-hist-mode))
+  (add-hook 'isearch-mode-hook #'eshell--isearch-setup nil t)
 
   (make-local-variable 'eshell-history-size)
   (or eshell-history-size
@@ -384,6 +375,23 @@ eshell-get-history
   "Get an input line from the history ring."
   (ring-ref (or ring eshell-history-ring) index))
 
+(defun eshell-goto-history (pos)
+  "Replace command line with the element at POS of history ring.
+Also update `eshell-history-index'.  As a special case, if POS is nil
+and `eshell--stored-incomplete-input' is a non-empty string, restore the
+saved input."
+  (when (null eshell-history-index)
+    (setq eshell--stored-incomplete-input
+          (buffer-substring-no-properties eshell-last-output-end
+                                          (point-max))))
+  (setq eshell-history-index pos)
+  ;; Can't use kill-region as it sets this-command
+  (delete-region eshell-last-output-end (point-max))
+  (if (and pos (not (ring-empty-p eshell-history-ring)))
+      (insert-and-inherit (eshell-get-history pos))
+    (when (> (length eshell--stored-incomplete-input) 0)
+      (insert-and-inherit eshell--stored-incomplete-input))))
+
 (defun eshell-add-input-to-history (input)
   "Add the string INPUT to the history ring.
 Input is entered into the input history ring, if the value of
@@ -897,12 +905,12 @@ eshell-previous-matching-input
     ;; Has a match been found?
     (if (null pos)
 	(error "Not found")
-      (setq eshell-history-index pos)
-      (unless (minibuffer-window-active-p (selected-window))
-	(message "History item: %d" (- (ring-length eshell-history-ring) pos)))
-      ;; Can't use kill-region as it sets this-command
-      (delete-region eshell-last-output-end (point))
-      (insert-and-inherit (eshell-get-history pos)))))
+      (eshell-goto-history pos)
+      (unless (or (minibuffer-window-active-p (selected-window))
+                  ;; No messages for Isearch because it will show the
+                  ;; same messages (and more).
+                  isearch-mode)
+	(message "History item: %d" (- (ring-length eshell-history-ring) pos))))))
 
 (defun eshell-next-matching-input (regexp arg)
   "Search forwards through input history for match for REGEXP.
@@ -937,114 +945,160 @@ eshell-next-matching-input-from-input
   (interactive "p")
   (eshell-previous-matching-input-from-input (- arg)))
 
-(defun eshell-test-imatch ()
-  "If isearch match good, put point at the beginning and return non-nil."
-  (if (get-text-property (point) 'history)
-      (progn (beginning-of-line) t)
-    (let ((before (point)))
-      (beginning-of-line)
-      (if (and (not (bolp))
-	       (<= (point) before))
-	  t
-	(if isearch-forward
-	    (progn
-	      (end-of-line)
-	      (forward-char))
-	  (beginning-of-line)
-	  (backward-char))))))
-
-(defun eshell-return-to-prompt ()
-  "Once a search string matches, insert it at the end and go there."
-  (setq isearch-other-end nil)
-  (let ((found (eshell-test-imatch)) before)
-    (while (and (not found)
-		(setq before
-		      (funcall (if isearch-forward
-				   're-search-forward
-				 're-search-backward)
-			       isearch-string nil t)))
-      (setq found (eshell-test-imatch)))
-    (if (not found)
-	(progn
-	  (goto-char eshell-last-output-end)
-	  (delete-region (point) (point-max)))
-      (setq before (point))
-      (let ((text (buffer-substring-no-properties
-		   (point) (line-end-position)))
-	    (orig (marker-position eshell-last-output-end)))
-	(goto-char eshell-last-output-end)
-	(delete-region (point) (point-max))
-	(when (and text (> (length text) 0))
-	  (insert text)
-	  (put-text-property (1- (point)) (point)
-			     'last-search-pos before)
-	  (set-marker eshell-last-output-end orig)
-	  (goto-char eshell-last-output-end))))))
-
-(defun eshell-prepare-for-search ()
-  "Make sure the old history file is at the beginning of the buffer."
-  (unless (get-text-property (point-min) 'history)
-    (save-excursion
-      (goto-char (point-min))
-      (let ((end (copy-marker (point) t)))
-	(insert-file-contents eshell-history-file-name)
-	(set-text-properties (point-min) end
-			     '(history t invisible t))))))
+(defun eshell--isearch-setup ()
+  "Set up Isearch to search the input history.
+Intended to be added to `isearch-mode-hook' in an Eshell buffer."
+  (when (and
+         ;; Eshell is busy running a foreground process
+         (not eshell-foreground-command)
+         (or (eq eshell-history-isearch t)
+             (and (eq eshell-history-isearch 'dwim)
+                  (>= (point) eshell-last-output-end))))
+    (setq isearch-message-prefix-add "history ")
+    (setq-local isearch-lazy-count nil)
+    (setq-local isearch-search-fun-function #'eshell-history-isearch-search
+                isearch-message-function #'eshell-history-isearch-message
+                isearch-wrap-function #'eshell-history-isearch-wrap
+                isearch-push-state-function #'eshell-history-isearch-push-state)
+    (add-hook 'isearch-mode-end-hook #'eshell-history-isearch-end nil t)))
+
+(defun eshell-history-isearch-end ()
+  "Clean up after terminating history Isearch."
+  (when (overlayp eshell--history-isearch-message-overlay)
+    (delete-overlay eshell--history-isearch-message-overlay))
+  (setq isearch-message-prefix-add nil)
+  (kill-local-variable 'isearch-lazy-count)
+  (setq-local isearch-search-fun-function #'isearch-search-fun-default
+              isearch-message-function nil
+              isearch-wrap-function nil
+              isearch-push-state-function nil)
+  (remove-hook 'isearch-mode-end-hook #'eshell-history-isearch-end t)
+  (setq isearch-opoint (point))
+  (unless isearch-suspended
+    (custom-reevaluate-setting 'eshell-history-isearch)))
+
+(defun eshell-history-isearch-search ()
+  "Return search function for Isearch in input history."
+  (lambda (string bound noerror)
+    (let ((search-fun (isearch-search-fun-default))
+          (found nil))
+      ;; Avoid highlighting matches in and before the last prompt
+      (when (and bound isearch-forward
+                 (< (point) eshell-last-output-end))
+        (goto-char eshell-last-output-end))
+      (or
+       ;; First search in the initial input
+       (funcall search-fun string
+                (if isearch-forward bound eshell-last-output-end)
+                noerror)
+       ;; Then search in the input history: put next/previous history
+       ;; element in the command line successively, then search the
+       ;; string in the command line.  Do this only when not
+       ;; lazy-highlighting (`bound' is nil).
+       (unless bound
+         (condition-case nil
+             (progn
+               (while (not found)
+                 (cond (isearch-forward
+                        ;; Signal an error explicitly to break
+                        (when (or (null eshell-history-index)
+                                  (eq eshell-history-index 0))
+                          (error "End of history; no next item"))
+                        (eshell-next-input 1)
+                        (goto-char eshell-last-output-end))
+                       (t
+                        ;; Signal an error explicitly to break
+                        (when (eq eshell-history-index
+                                  (1- (ring-length eshell-history-ring)))
+                          (error "Beginning of history; no preceding item"))
+                        (eshell-previous-input 1)
+                        (goto-char (point-max))))
+                 (setq isearch-barrier (point)
+                       isearch-opoint (point))
+                 ;; After putting an history element in the command
+                 ;; line, search the string in them.
+                 (setq found (funcall search-fun string
+                                      (unless isearch-forward
+                                        eshell-last-output-end)
+                                      noerror)))
+               (point))
+           ;; Return when no next/preceding element error signaled
+           (error nil)))))))
+
+(defun eshell-history-isearch-message (&optional c-q-hack ellipsis)
+  "Display the input history search prompt.
+If there are no search errors, this function displays an overlay with
+the Isearch prompt which replaces the original Eshell prompt.
+Otherwise, it displays the standard Isearch message returned from the
+function `isearch-message'."
+  (if (not (and isearch-success (not isearch-error)))
+      ;; Use standard message function (which displays a message in the
+      ;; echo area) when not in command line, or search fails or has
+      ;; errors (like incomplete regexp).
+      (isearch-message c-q-hack ellipsis)
+    ;; Otherwise, use an overlay over the Eshell prompt.
+    (if (overlayp eshell--history-isearch-message-overlay)
+        (move-overlay eshell--history-isearch-message-overlay
+                      (save-excursion
+                        (goto-char eshell-last-output-end)
+                        (forward-line 0)
+                        (point))
+                      eshell-last-output-end)
+      (setq eshell--history-isearch-message-overlay
+            (make-overlay (save-excursion
+                            (goto-char eshell-last-output-end)
+                            (forward-line 0)
+                            (point))
+                          eshell-last-output-end))
+      (overlay-put eshell--history-isearch-message-overlay 'evaporate t))
+    (overlay-put eshell--history-isearch-message-overlay
+                 'display (isearch-message-prefix ellipsis
+                                                  isearch-nonincremental))
+    (if (and eshell-history-index (not ellipsis))
+        (message "History item: %d" (- (ring-length eshell-history-ring)
+                                       eshell-history-index))
+      (message ""))))
+
+(defun eshell-history-isearch-wrap ()
+  "Wrap the input history search."
+  (if isearch-forward
+      (eshell-goto-history (1- (ring-length eshell-history-ring)))
+    (eshell-goto-history nil))
+  (goto-char (if isearch-forward eshell-last-output-end (point-max))))
+
+(defun eshell-history-isearch-push-state ()
+  "Save a function restoring the state of input history search.
+Save `eshell-history-index' to the additional state parameter in the
+search status stack."
+  (let ((index eshell-history-index))
+    (lambda (_cmd)
+      (eshell-goto-history index))))
 
 (defun eshell-isearch-backward (&optional invert)
-  "Do incremental regexp search backward through past commands."
-  (interactive)
-  (let ((inhibit-read-only t))
-    (eshell-prepare-for-search)
-    (goto-char (point-max))
-    (set-marker eshell-last-output-end (point))
-    (delete-region (point) (point-max)))
-  (isearch-mode invert t 'eshell-return-to-prompt))
-
-(defun eshell-isearch-repeat-backward (&optional invert)
-  "Do incremental regexp search backward through past commands."
-  (interactive)
-  (let ((old-pos (get-text-property (1- (point-max))
-				    'last-search-pos)))
-    (when old-pos
-      (goto-char old-pos)
-      (if invert
-	  (end-of-line)
-	(backward-char)))
-    (setq isearch-forward invert)
-    (isearch-search-and-update)))
+  "Do incremental search backward through past commands."
+  (interactive nil eshell-mode)
+  (setq eshell-history-isearch t)
+  (if invert
+      (isearch-forward nil t)
+    (isearch-backward nil t)))
 
 (defun eshell-isearch-forward ()
-  "Do incremental regexp search backward through past commands."
-  (interactive)
+  "Do incremental search forward through past commands."
+  (interactive nil eshell-mode)
   (eshell-isearch-backward t))
 
-(defun eshell-isearch-repeat-forward ()
+(defun eshell-isearch-backward-regexp (&optional invert)
   "Do incremental regexp search backward through past commands."
-  (interactive)
-  (eshell-isearch-repeat-backward t))
-
-(defun eshell-isearch-cancel ()
-  (interactive)
-  (goto-char eshell-last-output-end)
-  (delete-region (point) (point-max))
-  (call-interactively 'isearch-cancel))
-
-(defun eshell-isearch-abort ()
-  (interactive)
-  (goto-char eshell-last-output-end)
-  (delete-region (point) (point-max))
-  (call-interactively 'isearch-abort))
-
-(defun eshell-isearch-delete-char ()
-  (interactive)
-  (save-excursion
-  (isearch-delete-char)))
-
-(defun eshell-isearch-return ()
-  (interactive)
-  (isearch-done)
-  (eshell-send-input))
+  (interactive nil eshell-mode)
+  (setq eshell-history-isearch t)
+  (if invert
+      (isearch-forward-regexp nil t)
+    (isearch-backward-regexp nil t)))
+
+(defun eshell-isearch-forward-regexp ()
+  "Do incremental regexp search forward through past commands."
+  (interactive nil eshell-mode)
+  (eshell-isearch-backward-regexp t))
 
 (defun em-hist-unload-function ()
   (remove-hook 'kill-emacs-hook 'eshell-save-some-history))
-- 
2.47.1