From 3b875b89eb69b4733373d8a79c276bddf8356130 Mon Sep 17 00:00:00 2001
From: Liu Hui <liuhui1610@gmail.com>
Date: Tue, 7 Nov 2023 09:10:59 +0800
Subject: [PATCH] Improve read/append behavior of eshell history command

* lisp/eshell/em-hist.el (eshell-hist--new-items): New variable.
(eshell-hist-initialize): Initialize `eshell-hist--new-items' to 0.
(eshell/history): Change the behavior of 'history -a' to "append new
history in current buffer to history file".  Clarify the help text of
'history -r'.
(eshell-add-input-to-history): Increase counter of new history items.
(eshell-read-history): Respect eshell-hist-ignoredups option.
(eshell-write-history): If the optional argument APPEND is non-nil,
appending new history items rather than the whole history.
* test/lisp/eshell/em-hist-tests.el (em-hist-test/history-append):
(em-hist-test/history-read): New tests.
---
 lisp/eshell/em-hist.el            | 47 ++++++++++++++++++++-----------
 test/lisp/eshell/em-hist-tests.el | 34 ++++++++++++++++++++++
 2 files changed, 65 insertions(+), 16 deletions(-)

diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el
index 9d4b72b01df..775f32c6518 100644
--- a/lisp/eshell/em-hist.el
+++ b/lisp/eshell/em-hist.el
@@ -195,6 +195,9 @@ eshell-history-ring
 (defvar eshell-history-index nil)
 (defvar eshell-matching-input-from-input-string "")
 (defvar eshell-save-history-index nil)
+(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."
@@ -283,6 +286,7 @@ eshell-hist-initialize
 
   (make-local-variable 'eshell-history-index)
   (make-local-variable 'eshell-save-history-index)
+  (setq-local eshell-hist--new-items 0)
 
   (if (minibuffer-window-active-p (selected-window))
       (setq-local eshell-save-history-on-exit nil)
@@ -323,11 +327,11 @@ eshell/history
   (eshell-eval-using-options
    "history" args
    '((?r "read" nil read-history
-	 "read from history file to current history list")
+	 "clear current history list and read from history file to it")
      (?w "write" nil write-history
 	 "write current history list to history file")
      (?a "append" nil append-history
-	 "append current history list to history file")
+	 "append new history in current buffer to history file")
      (?h "help" nil nil "display this usage message")
      :usage "[n] [-rwa [filename]]"
      :post-usage
@@ -394,6 +398,8 @@ eshell-add-input-to-history
                (_                       ; Add if not already the latest entry
                 (or (ring-empty-p eshell-history-ring)
                     (not (string-equal (eshell-get-history 0) input))))))
+    (setq eshell-hist--new-items
+          (min eshell-history-size (1+ eshell-hist--new-items)))
     (eshell-put-history input))
   (setq eshell-save-history-index eshell-history-index)
   (setq eshell-history-index nil))
@@ -455,21 +461,28 @@ eshell-read-history
 		      (re-search-backward "^[ \t]*\\([^#\n].*\\)[ \t]*$"
 					  nil t))
 	    (let ((history (match-string 1)))
-	      (if (or (null ignore-dups)
-		      (ring-empty-p ring)
-		      (not (string-equal (ring-ref ring 0) history)))
-		  (ring-insert-at-beginning
-		   ring (subst-char-in-string ?\177 ?\n history))))
-	    (setq count (1+ count))))
+              (when (or (ring-empty-p ring)
+                        (null ignore-dups)
+                        (and (not (string-equal
+                                   (ring-ref ring (1- (ring-length ring)))
+                                   history))
+                             (not (and (eq ignore-dups 'erase)
+                                       (ring-member ring history)))))
+                (ring-insert-at-beginning
+		 ring (subst-char-in-string ?\177 ?\n history))
+                (setq count (1+ count))))))
 	(setq eshell-history-ring ring
-	      eshell-history-index nil))))))
+	      eshell-history-index nil
+              eshell-hist--new-items 0))))))
 
 (defun eshell-write-history (&optional filename append)
   "Writes the buffer's `eshell-history-ring' to a history file.
-The name of the file is given by the variable
-`eshell-history-file-name'.  The original contents of the file are
-lost if `eshell-history-ring' is not empty.  If
-`eshell-history-file-name' is nil this function does nothing.
+If the optional argument FILENAME is nil, the value of
+`eshell-history-file-name' is used.  This function does nothing
+if the value resolves to nil.  If the optional argument APPEND is
+omitted or nil, the original contents of the file are lost if
+`eshell-history-ring' is not empty.  A non-nil value means to
+append new history items to the history file.
 
 Useful within process sentinels.
 
@@ -480,13 +493,14 @@ eshell-write-history
      ((or (null file)
 	  (equal file "")
 	  (null eshell-history-ring)
-	  (ring-empty-p eshell-history-ring))
+	  (ring-empty-p eshell-history-ring)
+          (and append (= eshell-hist--new-items 0)))
       nil)
      ((not (file-writable-p resolved-file))
       (message "Cannot write history file %s" resolved-file))
      (t
       (let* ((ring eshell-history-ring)
-	     (index (ring-length ring)))
+	     (index (if append eshell-hist--new-items (ring-length ring))))
 	;; Write it all out into a buffer first.  Much faster, but
 	;; messier, than writing it one line at a time.
 	(with-temp-buffer
@@ -499,7 +513,8 @@ eshell-write-history
 	      (subst-char-in-region start (1- (point)) ?\n ?\177)))
 	  (eshell-with-private-file-modes
 	   (write-region (point-min) (point-max) resolved-file append
-			 'no-message))))))))
+			 'no-message)))
+        (setq eshell-hist--new-items 0))))))
 
 (defun eshell-list-history ()
   "List in help buffer the buffer's input history."
diff --git a/test/lisp/eshell/em-hist-tests.el b/test/lisp/eshell/em-hist-tests.el
index 0f143355115..4851bdc50b2 100644
--- a/test/lisp/eshell/em-hist-tests.el
+++ b/test/lisp/eshell/em-hist-tests.el
@@ -41,6 +41,40 @@ em-hist-test/write-readonly-history
                    (propertize "echo bar" 'read-only t))
       (eshell-write-history histfile))))
 
+(ert-deftest em-hist-test/history-append ()
+  "Test 'history -a'."
+  (ert-with-temp-file histfile
+    (with-temp-eshell
+     (let ((eshell-history-file-name histfile))
+       (eshell-insert-command "echo hi")
+       (eshell-insert-command "history -w")
+       (eshell-insert-command "history -a")
+       (eshell-insert-command "echo bye")
+       (eshell-insert-command "history -a")
+       (eshell-insert-command "history -r")
+       (should (equal (ring-elements eshell-history-ring)
+                      '("history -a" "echo bye"
+                        "history -a" "history -w" "echo hi")))))))
+
+(ert-deftest em-hist-test/history-read ()
+  "Test 'history -r'."
+  (ert-with-temp-file histfile
+    (with-temp-eshell
+     (let ((eshell-history-file-name histfile))
+       (eshell-insert-command "echo hi")
+       (eshell-insert-command "echo bye")
+       (eshell-insert-command "echo bye")
+       (eshell-insert-command "echo hi")
+       (eshell-insert-command "history -w")
+       (let ((eshell-hist-ignoredups t))
+         (eshell-insert-command "history -r")
+         (should (equal (ring-elements eshell-history-ring)
+                        '("history -w" "echo hi" "echo bye" "echo hi"))))
+       (let ((eshell-hist-ignoredups 'erase))
+         (eshell-insert-command "history -r")
+         (should (equal (ring-elements eshell-history-ring)
+                        '("history -w" "echo hi" "echo bye"))))))))
+
 (ert-deftest em-hist-test/add-to-history/allow-dups ()
   "Test adding to history, allowing dups."
   (let ((eshell-hist-ignoredups nil))
-- 
2.25.1

