From: Robin Campbell Joy <emacs@robinjoy.net>
To: Jim Porter <jporterbugs@gmail.com>
Cc: 71107@debbugs.gnu.org, eli@gnu.org
Subject: bug#71107: 29.3; eshell-hist/Incorrect history handling with eshell-hist-ignoredups 'erase
Date: Fri, 24 May 2024 14:29:59 +0200 [thread overview]
Message-ID: <CADzxVkFjcR5saOD-KKNjOu59Tvw-CPXNbUheZTc4870DZ+9tbQ@mail.gmail.com> (raw)
In-Reply-To: <634c838e-9a20-816f-d786-d01ccf75b5e1@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 803 bytes --]
On Fri, 24 May 2024 at 01:33, Jim Porter <jporterbugs@gmail.com> wrote:
> On 5/22/2024 11:22 PM, Robin Campbell Joy wrote:
> > Thanks, not sure why I didn't find this when looking for an existing
> > bug, should have checked master first though.
> >
> > Attached the rebased patch. Thanks for fixing this.
>
> Thanks for the updated patch. From a visual inspection, this all looks
> good to me.
>
> Just two small things and then I think this is good to merge:
>
> 1) Could you add a commit message in the usual style (you can look at
> the other Emacs commits to get an idea of what these look like), and
> then generate the patch file with `git format-patch master`? This'll
> make it easier for me to apply the patch without having to write up a
> commit message myself.
>
Sure, hope this will do.
[-- Attachment #1.2: Type: text/html, Size: 1205 bytes --]
[-- Attachment #2: 0001-Erase-existing-duplicates-in-eshell-history-ring.patch --]
[-- Type: application/octet-stream, Size: 3126 bytes --]
From 5080ad2c1c706c4110ee3a57422f1bcf998cfbbf Mon Sep 17 00:00:00 2001
From: Robin Joy <emacs@robinjoy.net>
Date: Fri, 24 May 2024 14:26:39 +0200
Subject: [PATCH] Erase existing duplicates in eshell-history-ring
Erase all existing duplicates instead of just the last duplicate entry
when `eshell-hist-ignoredups` is set to 'erase. Multiple duplicates can
exist in case `eshell-hist-ignoredups` was set to something else than
'erase in the past or if the history file contains duplicates.
* lisp/eshell/em-hist.el (eshell-add-input-to-history): Remove all
duplicates from history ring.
* test/lisp/eshell/em-hist-tests.el (erase-existing-dups): New test.
---
lisp/eshell/em-hist.el | 8 +++-----
test/lisp/eshell/em-hist-tests.el | 17 +++++++++++++++++
2 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el
index 21029eae1bc..b171a2850ff 100644
--- a/lisp/eshell/em-hist.el
+++ b/lisp/eshell/em-hist.el
@@ -398,11 +398,9 @@ eshell-add-input-to-history
(pcase eshell-hist-ignoredups
('nil t) ; Always add to history
('erase ; Add, removing any old occurrences
- (when-let ((old-index (ring-member eshell-history-ring input)))
- ;; Remove the old occurrence of this input so we can
- ;; add it to the end. FIXME: Should we try to
- ;; remove multiple old occurrences, e.g. if the user
- ;; recently changed to using `erase'?
+ (while-let ((old-index (ring-member eshell-history-ring input)))
+ ;; Remove the old occurrences of this input so we can
+ ;; add it to the end.
(ring-remove eshell-history-ring old-index))
t)
(_ ; Add if not already the latest entry
diff --git a/test/lisp/eshell/em-hist-tests.el b/test/lisp/eshell/em-hist-tests.el
index a4e1e01b124..40e6f90478d 100644
--- a/test/lisp/eshell/em-hist-tests.el
+++ b/test/lisp/eshell/em-hist-tests.el
@@ -163,6 +163,23 @@ em-hist-test/add-to-history/erase-dups
(should (equal (ring-elements eshell-history-ring)
'("echo hi" "echo bye"))))))
+(ert-deftest em-hist-test/add-to-history/erase-existing-dups ()
+ "Test adding to history, erasing any old dups after switching to 'erase."
+ (let ((eshell-hist-ignoredups nil))
+ (with-temp-eshell
+ (eshell-insert-command "echo hi")
+ (eshell-insert-command "echo bye")
+ (eshell-insert-command "echo bye")
+ (eshell-insert-command "echo hi")
+ (eshell-insert-command "echo bye")
+ (setq eshell-hist-ignoredups 'erase)
+ (eshell-insert-command "echo hi")
+ (should (equal (ring-elements eshell-history-ring)
+ '("echo hi" "echo bye" "echo bye" "echo bye")))
+ (eshell-insert-command "echo bye")
+ (should (equal (ring-elements eshell-history-ring)
+ '("echo bye" "echo hi"))))))
+
(provide 'em-hist-test)
;;; em-hist-tests.el ends here
--
2.45.1
next prev parent reply other threads:[~2024-05-24 12:29 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2024-05-25 2:35 ` Jim Porter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CADzxVkFjcR5saOD-KKNjOu59Tvw-CPXNbUheZTc4870DZ+9tbQ@mail.gmail.com \
--to=emacs@robinjoy.net \
--cc=71107@debbugs.gnu.org \
--cc=eli@gnu.org \
--cc=jporterbugs@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.