bug#72097: 30.0.60; [PATCH] Don't add to Eshell history when aborting 'eshell-command'

bug#72097: 30.0.60; [PATCH] Don't add to Eshell history when aborting 'eshell-command'

[Jim Porter]

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

Steps to reproduce:

   emacs -Q
   M-x eshell-command RET oops C-g
   M-x eshell
   <up>

This will show the most-recent command in the Eshell history as "oops". 
That's because it was adding to the history in 'minibuffer-exit-hook', 
which runs regardless of *how* you exit the minibuffer. This regressed 
from 093a360251, and I misunderstood the conditions where 
'minibuffer-exit-hook' runs.

Attached is a patch to fix this. Is this ok for the release branch?

[-- Attachment #2: 0001-Don-t-save-to-history-from-eshell-command-when-abort.patch --]
[-- Type: text/plain, Size: 3718 bytes --]

From 29f4967dd877c19b827fb44cb1c01944d4012995 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 13 Jul 2024 11:43:42 -0700
Subject: [PATCH] Don't save to history from 'eshell-command' when aborting

* lisp/eshell/eshell.el (eshell-add-input-to-history)
(eshell--save-history): Declare.
(eshell-command-mode-exit): New function...
(eshell-command-mode): ... use it.

* lisp/eshell/em-hist.el (eshell-hist-initialize): Don't handle
minibuffer logic here.  Always read history file (this ensures that
'eshell-command' can see the history, too).
(eshell-add-command-to-history): Remove.
---
 lisp/eshell/em-hist.el | 20 ++------------------
 lisp/eshell/eshell.el  | 20 ++++++++++++++++----
 2 files changed, 18 insertions(+), 22 deletions(-)

diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el
index 9ffddfb611f..fffd611c06f 100644
--- a/lisp/eshell/em-hist.el
+++ b/lisp/eshell/em-hist.el
@@ -295,12 +295,8 @@ eshell-hist-initialize
   (setq-local eshell-hist--new-items 0)
 
   (setq-local eshell-history-ring nil)
-  (if (minibuffer-window-active-p (selected-window))
-      (progn
-        (setq-local eshell-history-append t)
-        (add-hook 'minibuffer-exit-hook #'eshell-add-command-to-history nil t))
-    (if eshell-history-file-name
-	(eshell-read-history nil t)))
+  (when eshell-history-file-name
+    (eshell-read-history nil t))
 
   (unless eshell-history-ring
     (setq eshell-history-ring (make-ring eshell-history-size)))
@@ -411,18 +407,6 @@ eshell-add-input-to-history
   (setq eshell-save-history-index eshell-history-index)
   (setq eshell-history-index nil))
 
-(defun eshell-add-command-to-history ()
-  "Add the command entered at `eshell-command's prompt to the history ring.
-The command is added to the input history ring, if the value of
-variable `eshell-input-filter' returns non-nil when called on the
-command.
-
-This function is supposed to be called from the minibuffer, presumably
-as a `minibuffer-exit-hook'."
-  (eshell-add-input-to-history
-   (buffer-substring (minibuffer-prompt-end) (point-max)))
-  (eshell--save-history))
-
 (defun eshell-add-to-history ()
   "Add last Eshell command to the history ring.
 The command is entered into the input history ring, if the value of
diff --git a/lisp/eshell/eshell.el b/lisp/eshell/eshell.el
index b7be3dd1643..d60101d51e1 100644
--- a/lisp/eshell/eshell.el
+++ b/lisp/eshell/eshell.el
@@ -284,14 +284,26 @@ eshell
       (eshell-mode))
     buf))
 
+(declare-function eshell-add-input-to-history "em-hist" (input))
+(declare-function eshell--save-history "em-hist" ())
+
+(defun eshell-command-mode-exit ()
+  "Exit the `eshell-commad-mode' minibuffer and save Eshell history."
+  (interactive)
+  (when (eshell-using-module 'eshell-hist)
+    (eshell-add-input-to-history
+     (buffer-substring (minibuffer-prompt-end) (point-max)))
+    (eshell--save-history))
+  (exit-minibuffer))
+
 (define-minor-mode eshell-command-mode
   "Minor mode for `eshell-command' input.
 \\{eshell-command-mode-map}"
   :keymap (let ((map (make-sparse-keymap)))
-            (define-key map [(control ?g)] 'abort-recursive-edit)
-            (define-key map [(control ?m)] 'exit-minibuffer)
-            (define-key map [(control ?j)] 'exit-minibuffer)
-            (define-key map [(meta control ?m)] 'exit-minibuffer)
+            (define-key map [(control ?g)] #'abort-recursive-edit)
+            (define-key map [(control ?m)] #'eshell-command-mode-exit)
+            (define-key map [(control ?j)] #'eshell-command-mode-exit)
+            (define-key map [(meta control ?m)] #'eshell-command-mode-exit)
             map))
 
 (define-obsolete-function-alias 'eshell-return-exits-minibuffer
-- 
2.25.1

bug#72097: 30.0.60; [PATCH] Don't add to Eshell history when aborting 'eshell-command'

[Eli Zaretskii]

> Date: Sat, 13 Jul 2024 11:51:46 -0700
> From: Jim Porter <jporterbugs@gmail.com>
> 
>    emacs -Q
>    M-x eshell-command RET oops C-g
>    M-x eshell
>    <up>
> 
> This will show the most-recent command in the Eshell history as "oops". 
> That's because it was adding to the history in 'minibuffer-exit-hook', 
> which runs regardless of *how* you exit the minibuffer. This regressed 
> from 093a360251, and I misunderstood the conditions where 
> 'minibuffer-exit-hook' runs.
> 
> Attached is a patch to fix this. Is this ok for the release branch?

Yes, thanks.

bug#72097: 30.0.60; [PATCH] Don't add to Eshell history when aborting 'eshell-command'

[Jim Porter]

On 7/13/2024 9:42 PM, Eli Zaretskii wrote:
>> Attached is a patch to fix this. Is this ok for the release branch?
> 
> Yes, thanks.

Thanks. Merged to the release branch as 3407e274999, and closing this bug.