unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#71107: 29.3; eshell-hist/Incorrect history handling with eshell-hist-ignoredups 'erase
@ 2024-05-22  8:21 Robin Campbell Joy
  2024-05-23  4:35 ` Jim Porter
  0 siblings, 1 reply; 8+ messages in thread
From: Robin Campbell Joy @ 2024-05-22  8:21 UTC (permalink / raw)
  To: 71107


[-- Attachment #1.1: Type: text/plain, Size: 525 bytes --]

Severity: minor
Tags: patch

In case of eshell-hist-ingoredups set to 'erase, history handling isn't
working correctly.

1) If the input ring is empty, no elements are added to the ring.
2) If elements are on the ring, but the element added to the ring is not
yet in the ring, the last element is deleted.
3) When switching to 'erase with multiple duplicates in the ring, only
the last equal element is removed.

emacs -Q
(require 'eshell)
(setq eshell-hist-ignoredups 'erase)
(eshell)
$ echo foo
foo
M-p -> Empty input ring

[-- Attachment #1.2: Type: text/html, Size: 628 bytes --]

[-- Attachment #2: eshell-hist-ignoredups.patch --]
[-- Type: application/octet-stream, Size: 4140 bytes --]

diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el
index 4081f2a7f85..e7665de3ab7 100644
--- a/lisp/eshell/em-hist.el
+++ b/lisp/eshell/em-hist.el
@@ -385,10 +385,12 @@ eshell-add-input-to-history
              (if (eq eshell-hist-ignoredups 'erase)
                  ;; Remove any old occurrences of the input, and put
                  ;; the new one at the end.
-                 (unless (ring-empty-p eshell-history-ring)
-                   (ring-remove eshell-history-ring
-	                        (ring-member eshell-history-ring input))
-                   t)
+                 (or (unless (ring-empty-p eshell-history-ring)
+                       (let (index)
+                         (while (setq index (ring-member eshell-history-ring input))
+                           (ring-remove eshell-history-ring index)))
+                       t)
+                     t)
                ;; Always add...
                (or (null eshell-hist-ignoredups)
                    ;; ... or add if it's not already present at the
diff --git a/test/lisp/eshell/em-hist-tests.el b/test/lisp/eshell/em-hist-tests.el
index d325e3a6402..04d7b1a0670 100644
--- a/test/lisp/eshell/em-hist-tests.el
+++ b/test/lisp/eshell/em-hist-tests.el
@@ -33,6 +33,51 @@ eshell-write-readonly-history
                    (propertize "echo bar" 'read-only t))
       (eshell-write-history histfile))))
 
+(ert-deftest eshell-add-history-element-to-empty-ring-with-erase-dups ()
+  "Test that we add a new history item to an empty history ring with `eshell-hist-ignoredups' set to 'erase."
+  (ert-with-test-buffer (:name "eshell-hist-test")
+    (let ((eshell-history-ring (make-ring 2))
+          (eshell-hist-ignoredups 'erase))
+      (eshell-add-input-to-history "echo foo")
+      (should (= (ring-length eshell-history-ring) 1))
+      (should (= (ring-member eshell-history-ring "echo foo") 0)))))
+
+(ert-deftest eshell-add-history-element-to-non-empty-ring-with-erase-dups-erases-duplicate ()
+  "Test that when we add an existing history item to a non empty history ring with `eshell-hist-ignoredups' set to 'erase the existing element is erased."
+  (ert-with-test-buffer (:name "eshell-hist-test")
+    (let ((eshell-history-ring (make-ring 2))
+          (eshell-hist-ignoredups 'erase))
+      (eshell-add-input-to-history "echo foo")
+      (eshell-add-input-to-history "echo foo")
+      (should (= (ring-length eshell-history-ring) 1))
+      (should (= (ring-member eshell-history-ring "ls") 0)))))
+
+(ert-deftest eshell-add-history-element-to-non-empty-ring-with-erase-dups-erases-duplicates ()
+  "Test that when we add an existing history item to a non empty history ring with `eshell-hist-ignoredups' set to 'erase the existing elements are erased."
+  (ert-with-test-buffer (:name "eshell-hist-test")
+    (let ((eshell-history-ring (make-ring 5))
+          (eshell-hist-ignoredups nil))
+      (eshell-add-input-to-history "echo foo")
+      (eshell-add-input-to-history "echo bar")
+      (eshell-add-input-to-history "echo foo")
+      (should (= (ring-length eshell-history-ring) 3))
+      (setq eshell-hist-ignoredups 'erase)
+      (eshell-add-input-to-history "echo foo")
+      (should (= (ring-length eshell-history-ring) 2))
+      (should (= (ring-member eshell-history-ring "echo foo") 0))
+      (should (= (ring-member eshell-history-ring "echo bar") 1))))))
+
+(ert-deftest eshell-add-history-element-to-non-empty-ring-with-erase-dups-keeps-non-duplicates ()
+  "Test that when we add a new  history item to a non empty history ring with `eshell-hist-ignoredups' set to 'erase the existing elements are not erased."
+  (ert-with-test-buffer (:name "eshell-hist-test")
+    (let ((eshell-history-ring (make-ring 2))
+          (eshell-hist-ignoredups 'erase))
+      (eshell-add-input-to-history "echo foo")
+      (eshell-add-input-to-history "echo bar")
+      (should (= (ring-length eshell-history-ring) 2))
+      (should (= (ring-member eshell-history-ring "echo foo") 1))
+      (should (= (ring-member eshell-history-ring "echo bar") 0)))))
+
 (provide 'em-hist-test)
 
 ;;; em-hist-tests.el ends here

^ permalink raw reply related	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-05-25  2:38 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-22  8:21 bug#71107: 29.3; eshell-hist/Incorrect history handling with eshell-hist-ignoredups 'erase Robin Campbell Joy
2024-05-23  4:35 ` Jim Porter
2024-05-23  6:22   ` Robin Campbell Joy
2024-05-23 23:33     ` Jim Porter
2024-05-24  5:55       ` Eli Zaretskii
2024-05-25  2:38         ` Jim Porter
2024-05-24 12:29       ` Robin Campbell Joy
2024-05-25  2:35         ` Jim Porter

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).