master 8b3969006fe 1/2: Improve read/append behavior of eshell history command

[Jim Porter]

branch: master
commit 8b3969006fe6095178eea38df096d73bdd460a15
Author: Liu Hui <liuhui1610@gmail.com>
Commit: Jim Porter <jporterbugs@gmail.com>

    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 (bug#66768).
---
 lisp/eshell/em-hist.el            | 49 ++++++++++++++++++++++++++-------------
 test/lisp/eshell/em-hist-tests.el | 34 +++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 16 deletions(-)

diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el
index 9d4b72b01df..cf03f8399a6 100644
--- a/lisp/eshell/em-hist.el
+++ b/lisp/eshell/em-hist.el
@@ -195,6 +195,9 @@ element, regardless of any text on the command line.  In 
that case,
 (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 @@ Returns nil if INPUT is prepended by blank space, otherwise 
non-nil."
 
   (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 @@ Returns nil if INPUT is prepended by blank space, 
otherwise non-nil."
   (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 @@ input."
                (_                       ; 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,30 @@ line, with the most recent command last.  See also
                      (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 non-nil, then append new
+history items to the history file.  Otherwise, overwrite the
+contents of the file with `eshell-history-ring' (so long as it is
+not empty).
 
 Useful within process sentinels.
 
@@ -480,13 +495,14 @@ See also `eshell-read-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 +515,8 @@ See also `eshell-read-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 @@
                    (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))