GNU bug report logs - #63362
Bug+fix for eshell-hist-ignore-dups 'erase

Previous Next

Package: emacs;

Reported by: Alexander Kozhevnikov <mentalisttraceur <at> gmail.com>

Date: Mon, 8 May 2023 07:24:05 UTC

Severity: normal

Merged with 63360

Fixed in version 30.1

Done: Jim Porter <jporterbugs <at> gmail.com>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 63362 in the body.
You can then email your comments to 63362 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#63362; Package emacs. (Mon, 08 May 2023 07:24:05 GMT) Full text and rfc822 format available.

Acknowledgement sent to Alexander Kozhevnikov <mentalisttraceur <at> gmail.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Mon, 08 May 2023 07:24:05 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Alexander Kozhevnikov <mentalisttraceur <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: Bug+fix for eshell-hist-ignore-dups 'erase
Date: Sun, 7 May 2023 18:38:42 +0000
Ah! The second bug (the case where there are zero history entries) is
due to the immediately surrounding lines:

                 (unless (ring-empty-p eshell-history-ring)
                   ...
                   t)

Same exact steps to reproduce, just don't add one entry to the history
ring - leave it empty.

The cause is that these lines are inside a big condition of a big
`when` - but that "condition" is complecting two things:

1. the actual predicate - should we add this input to the history ring? and
2. the history management side-effects that need to happen if that
predicate is true, before the history addition is made.

So the key observation is that this `(unless ...)` is part of the
surrounding `(and ...)` but is not actually there to influence the
condition! It's there to catch a case which requires a different
side-effect. But when the inner `(unless ...)` was added, the `t` got
accidentally/wrongly scooped into the `(unless ...)` along with the
side-effect.

I think the ideal fix here is a refactor that makes the big picture
clearer (I can provide one if asked, but that would almost certainly
have enough creative substance to require copyright assignment, and
would need to wait on the paperwork). But a good-enough, minimally
disruptive fix that is too mechanical and small to be copyrightable is
just to change the `(unless ... t)` to a `(progn (unless ...) t)`:

                 (progn
                   (unless (ring-empty-p eshell-history-ring)
                   ...)
                 t)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#63362; Package emacs. (Mon, 15 May 2023 04:47:02 GMT) Full text and rfc822 format available.

Message #8 received at 63362 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Alexander Kozhevnikov <mentalisttraceur <at> gmail.com>, 63362 <at> debbugs.gnu.org
Subject: Re: bug#63362: Bug+fix for eshell-hist-ignore-dups 'erase
Date: Sun, 14 May 2023 21:46:32 -0700
On 5/7/2023 11:38 AM, Alexander Kozhevnikov wrote:
> I think the ideal fix here is a refactor that makes the big picture
> clearer (I can provide one if asked, but that would almost certainly
> have enough creative substance to require copyright assignment, and
> would need to wait on the paperwork).

Thanks for this analysis. I'll try and take a look at this in more 
detail, and perhaps refactor things a bit in this part of the code as 
you suggest. (I've wanted to clean up the Eshell history logic a bit 
already, since I'd like to make it a bit more usable when you have 
multiple Eshell buffers.)

First step though is for me to write up a few regression tests, I think. 
That should make it easier to do some refactoring without breaking anything.




Merged 63360 63362. Request was from Jim Porter <jporterbugs <at> gmail.com> to control <at> debbugs.gnu.org. (Thu, 24 Aug 2023 00:19:02 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Thu, 21 Sep 2023 11:24:08 GMT) Full text and rfc822 format available.

This bug report was last modified 188 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.