GNU bug report logs - #66700
30.0.50; Some history is missing with multiple eshell buffers

Previous Next

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

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

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

From: Liu Hui <liuhui1610 <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; Some history is missing with multiple eshell buffers
Date: Mon, 23 Oct 2023 20:14:27 +0800

Hi,

Eshell saves history by default when quitting the buffer or emacs, but
some history is missing when there are multiple eshell buffers. For
example,

1. emacs -Q
2. M-x eshell, and type foo
3. C-u M-x eshell, and type bar
4. quit emacs

I would expect both commands are saved, but in fact only one command
is saved in the file eshell-history-file-name. I think the reason is
the history file is always overwritten when an eshell buffer tries to
save the history, so maybe eshell should save commands by appending
to the file?

Best

--
Liu Hui

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

From: Liu Hui <liuhui1610 <at> gmail.com>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: bug-gnu-emacs <at> gnu.org
Subject: Re: 30.0.50; Some history is missing with multiple eshell buffers
Date: Tue, 14 Nov 2023 12:40:00 +0800

Hi Jim,

> Eshell saves history by default when quitting the buffer or emacs, but
> some history is missing when there are multiple eshell buffers. For
> example,
>
> 1. emacs -Q
> 2. M-x eshell, and type foo
> 3. C-u M-x eshell, and type bar
> 4. quit emacs
>
> I would expect both commands are saved, but in fact only one command
> is saved in the file eshell-history-file-name. I think the reason is
> the history file is always overwritten when an eshell buffer tries to
> save the history, so maybe eshell should save commands by appending
> to the file?

Since Eshell can append new history now, I think it makes sense to
change the default behavior from overwriting to appending, i.e., using
'(eshell-write-history nil t)' in eshell-exit-hook and
eshell-save-some-history. WDYT?

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Liu Hui <liuhui1610 <at> gmail.com>
Cc: jporterbugs <at> gmail.com, 66700 <at> debbugs.gnu.org
Subject: Re: bug#66700: 30.0.50;
 Some history is missing with multiple eshell buffers
Date: Tue, 14 Nov 2023 14:32:10 +0200

> Cc: 66700 <at> debbugs.gnu.org
> From: Liu Hui <liuhui1610 <at> gmail.com>
> Date: Tue, 14 Nov 2023 12:40:00 +0800
> 
> Since Eshell can append new history now, I think it makes sense to
> change the default behavior from overwriting to appending, i.e., using
> '(eshell-write-history nil t)' in eshell-exit-hook and
> eshell-save-some-history. WDYT?

IMO, this is too soon to change the default behavior in incompatible
ways.  We need to wait for a while, perhaps at least one major
release, before we do that.

Thanks.

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Eli Zaretskii <eliz <at> gnu.org>, Liu Hui <liuhui1610 <at> gmail.com>,
 jporterbugs <at> gmail.com,  66700 <at> debbugs.gnu.org
Subject: Re: bug#66700: 30.0.50; Some history is missing with multiple
 eshell buffers
Date: Tue, 14 Nov 2023 12:43:45 +0000

Hello,

On Tue 14 Nov 2023 at 02:32pm +02, Eli Zaretskii wrote:

>> Cc: 66700 <at> debbugs.gnu.org
>> From: Liu Hui <liuhui1610 <at> gmail.com>
>> Date: Tue, 14 Nov 2023 12:40:00 +0800
>>
>> Since Eshell can append new history now, I think it makes sense to
>> change the default behavior from overwriting to appending, i.e., using
>> '(eshell-write-history nil t)' in eshell-exit-hook and
>> eshell-save-some-history. WDYT?
>
> IMO, this is too soon to change the default behavior in incompatible
> ways.  We need to wait for a while, perhaps at least one major
> release, before we do that.

This is fair enough.  It would be really useful to have the default
behaviour now for those of used to it from other shells, though -- Liu,
perhaps you would be interested in adding a defcustom?

-- 
Sean Whitton

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Sean Whitton <spwhitton <at> spwhitton.name>, Eli Zaretskii <eliz <at> gnu.org>,
 Liu Hui <liuhui1610 <at> gmail.com>, 66700 <at> debbugs.gnu.org
Subject: Re: bug#66700: 30.0.50; Some history is missing with multiple eshell
 buffers
Date: Tue, 14 Nov 2023 09:15:07 -0800

On 11/14/2023 4:43 AM, Sean Whitton wrote:
> On Tue 14 Nov 2023 at 02:32pm +02, Eli Zaretskii wrote:
>>
>> IMO, this is too soon to change the default behavior in incompatible
>> ways.  We need to wait for a while, perhaps at least one major
>> release, before we do that.
> 
> This is fair enough.  It would be really useful to have the default
> behaviour now for those of used to it from other shells, though -- Liu,
> perhaps you would be interested in adding a defcustom?

Yeah, I think a defcustom would be good here. That would also make it 
easier for us to test out the new code and find any bugs.

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

From: Liu Hui <liuhui1610 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: jporterbugs <at> gmail.com, 66700 <at> debbugs.gnu.org
Subject: Re: bug#66700: 30.0.50;
 Some history is missing with multiple eshell buffers
Date: Wed, 15 Nov 2023 17:59:23 +0800

Hi Eli,

Eli Zaretskii <eliz <at> gnu.org> 于2023年11月14日周二 20:32写道：

> > Since Eshell can append new history now, I think it makes sense to
> > change the default behavior from overwriting to appending, i.e., using
> > '(eshell-write-history nil t)' in eshell-exit-hook and
> > eshell-save-some-history. WDYT?
>
> IMO, this is too soon to change the default behavior in incompatible
> ways.  We need to wait for a while, perhaps at least one major
> release, before we do that.
>
> Thanks.

It is not an incompatible change at the user level. It is actually a
bug fix for people using multiple eshell sessions. For people using a
single eshell buffer, the history inside eshell, when reading back
from the file, is always same no matter how history is saved, and the
only difference is the content of history file.

As others have suggested, we could add an option to toggle the
behavior with appending new history as the default. It allows more
testing and users can easily revert back to old behavior. Do you think
it is acceptable?

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Liu Hui <liuhui1610 <at> gmail.com>
Cc: jporterbugs <at> gmail.com, 66700 <at> debbugs.gnu.org
Subject: Re: bug#66700: 30.0.50;
 Some history is missing with multiple eshell buffers
Date: Wed, 15 Nov 2023 14:22:56 +0200

> From: Liu Hui <liuhui1610 <at> gmail.com>
> Date: Wed, 15 Nov 2023 17:59:23 +0800
> Cc: jporterbugs <at> gmail.com, 66700 <at> debbugs.gnu.org
> 
> Eli Zaretskii <eliz <at> gnu.org> 于2023年11月14日周二 20:32写道：
> 
> > IMO, this is too soon to change the default behavior in incompatible
> > ways.  We need to wait for a while, perhaps at least one major
> > release, before we do that.
> >
> > Thanks.
> 
> It is not an incompatible change at the user level. It is actually a
> bug fix for people using multiple eshell sessions. For people using a
> single eshell buffer, the history inside eshell, when reading back
> from the file, is always same no matter how history is saved, and the
> only difference is the content of history file.
> 
> As others have suggested, we could add an option to toggle the
> behavior with appending new history as the default. It allows more
> testing and users can easily revert back to old behavior. Do you think
> it is acceptable?

An option is, of course, acceptable, but its default should not change
the previous behavior.  We could consider changing the default later,
after we have enough experience with this option, and can ask users
whether they like the "append" behavior to be the default.

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

From: Liu Hui <liuhui1610 <at> gmail.com>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 66700 <at> debbugs.gnu.org,
 Sean Whitton <spwhitton <at> spwhitton.name>
Subject: Re: bug#66700: 30.0.50;
 Some history is missing with multiple eshell buffers
Date: Thu, 16 Nov 2023 17:47:43 +0800

[Message part 1 (text/plain, inline)]

Jim Porter <jporterbugs <at> gmail.com> 于2023年11月15日周三 01:15写道：
>
> On 11/14/2023 4:43 AM, Sean Whitton wrote:
> > On Tue 14 Nov 2023 at 02:32pm +02, Eli Zaretskii wrote:
> >>
> >> IMO, this is too soon to change the default behavior in incompatible
> >> ways.  We need to wait for a while, perhaps at least one major
> >> release, before we do that.
> >
> > This is fair enough.  It would be really useful to have the default
> > behaviour now for those of used to it from other shells, though -- Liu,
> > perhaps you would be interested in adding a defcustom?
>
> Yeah, I think a defcustom would be good here. That would also make it
> easier for us to test out the new code and find any bugs.

Thank you all for your suggestions. A defcustom has been added in the
attached patch without changing previous behavior.

[0001-Add-option-eshell-history-append.patch (text/x-patch, attachment)]

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Liu Hui <liuhui1610 <at> gmail.com>
Cc: Jim Porter <jporterbugs <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>,
 66700 <at> debbugs.gnu.org
Subject: Re: bug#66700: 30.0.50; Some history is missing with multiple
 eshell buffers
Date: Wed, 06 Dec 2023 15:33:07 +0000

On Thu 16 Nov 2023 at 05:47pm +08, Liu Hui wrote:

> Jim Porter <jporterbugs <at> gmail.com> 于2023年11月15日周三 01:15写道：
>>
>> On 11/14/2023 4:43 AM, Sean Whitton wrote:
>> > On Tue 14 Nov 2023 at 02:32pm +02, Eli Zaretskii wrote:
>> >>
>> >> IMO, this is too soon to change the default behavior in incompatible
>> >> ways.  We need to wait for a while, perhaps at least one major
>> >> release, before we do that.
>> >
>> > This is fair enough.  It would be really useful to have the default
>> > behaviour now for those of used to it from other shells, though -- Liu,
>> > perhaps you would be interested in adding a defcustom?
>>
>> Yeah, I think a defcustom would be good here. That would also make it
>> easier for us to test out the new code and find any bugs.
>
> Thank you all for your suggestions. A defcustom has been added in the
> attached patch without changing previous behavior.

Thanks.  This looks good, but can you add a NEWS entry, please?

Ideally it would also get documented in the Eshell manual.

> From e2cd3a9d28b35806d4e6a83e98f0dc25550651e2 Mon Sep 17 00:00:00 2001
> From: Liu Hui <liuhui1610 <at> gmail.com>
> Date: Thu, 16 Nov 2023 11:45:28 +0800
> Subject: [PATCH] Add option `eshell-history-append'
>
> * lisp/eshell/em-hist.el (eshell-history-append): New option.
> (eshell--save-history): New function.
> (eshell-hist-initialize):
> (eshell-save-some-history): Replace eshell-write-history with
> eshell--save-history, which respects the new option.
> ---
>  lisp/eshell/em-hist.el | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el
> index cf03f8399a6..79336204847 100644
> --- a/lisp/eshell/em-hist.el
> +++ b/lisp/eshell/em-hist.el
> @@ -116,6 +116,12 @@ eshell-save-history-on-exit
>  		 (const :tag "Ask" ask)
>  		 (const :tag "Always save" t)))
>  
> +(defcustom eshell-history-append nil
> +  "If non-nil, append new entries to the history file when saving history."
> +  :type '(choice (const :tag "Overwrite history file" nil)
> +		 (const :tag "Append new entries to file" t))
> +  :version "30.1")
> +
>  (defcustom eshell-input-filter 'eshell-input-filter-default
>    "Predicate for filtering additions to input history.
>  Takes one argument, the input.  If non-nil, the input may be saved on
> @@ -294,17 +300,21 @@ eshell-hist-initialize
>      (if eshell-history-file-name
>  	(eshell-read-history nil t))
>  
> -    (add-hook 'eshell-exit-hook #'eshell-write-history nil t))
> +    (add-hook 'eshell-exit-hook #'eshell--save-history nil t))
>  
>    (unless eshell-history-ring
>      (setq eshell-history-ring (make-ring eshell-history-size)))
>  
> -  (add-hook 'eshell-exit-hook #'eshell-write-history nil t)
> +  (add-hook 'eshell-exit-hook #'eshell--save-history nil t)
>  
>    (add-hook 'kill-emacs-query-functions #'eshell-save-some-history)
>  
>    (add-hook 'eshell-input-filter-functions #'eshell-add-to-history nil t))
>  
> +(defun eshell--save-history ()
> +  "Save the history for current Eshell buffer."
> +  (eshell-write-history nil eshell-history-append))
> +
>  (defun eshell-save-some-history ()
>    "Save the history for any open Eshell buffers."
>    (dolist (buf (buffer-list))
> @@ -318,7 +328,7 @@ eshell-save-some-history
>  			(format-message
>  			 "Save input history for Eshell buffer `%s'? "
>  			 (buffer-name buf)))))
> -	      (eshell-write-history)))))
> +	      (eshell--save-history)))))
>    t)
>  
>  (defun eshell/history (&rest args)

-- 
Sean Whitton

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

From: Liu Hui <liuhui1610 <at> gmail.com>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: Jim Porter <jporterbugs <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>,
 66700 <at> debbugs.gnu.org
Subject: Re: bug#66700: 30.0.50;
 Some history is missing with multiple eshell buffers
Date: Thu, 7 Dec 2023 18:23:57 +0800

[Message part 1 (text/plain, inline)]

Sean Whitton <spwhitton <at> spwhitton.name> 于2023年12月6日周三 23:33写道：
>
> On Thu 16 Nov 2023 at 05:47pm +08, Liu Hui wrote:
>
> > Jim Porter <jporterbugs <at> gmail.com> 于2023年11月15日周三 01:15写道：
> >>
> >> On 11/14/2023 4:43 AM, Sean Whitton wrote:
> >> > On Tue 14 Nov 2023 at 02:32pm +02, Eli Zaretskii wrote:
> >> >>
> >> >> IMO, this is too soon to change the default behavior in incompatible
> >> >> ways.  We need to wait for a while, perhaps at least one major
> >> >> release, before we do that.
> >> >
> >> > This is fair enough.  It would be really useful to have the default
> >> > behaviour now for those of used to it from other shells, though -- Liu,
> >> > perhaps you would be interested in adding a defcustom?
> >>
> >> Yeah, I think a defcustom would be good here. That would also make it
> >> easier for us to test out the new code and find any bugs.
> >
> > Thank you all for your suggestions. A defcustom has been added in the
> > attached patch without changing previous behavior.
>
> Thanks.  This looks good, but can you add a NEWS entry, please?
>
> Ideally it would also get documented in the Eshell manual.

Thanks for your feedback. I have updated the patch as suggested.

[0001-Add-option-eshell-history-append.patch (text/x-patch, attachment)]

Message #37 received at 66700-done <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Liu Hui <liuhui1610 <at> gmail.com>
Cc: Jim Porter <jporterbugs <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>,
 66700-done <at> debbugs.gnu.org
Subject: Re: bug#66700: 30.0.50; Some history is missing with multiple
 eshell buffers
Date: Thu, 07 Dec 2023 11:33:48 +0000

Hello,

I've installed the change, thank you again.

I copy-edited your manual text a bit while doing so.

-- 
Sean Whitton

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: 66700 <at> debbugs.gnu.org, spwhitton <at> spwhitton.name, liuhui1610 <at> gmail.com
Subject: Re: bug#66700: 30.0.50; Some history is missing with multiple eshell
 buffers
Date: Fri, 8 Dec 2023 14:53:06 -0800

On 12/7/2023 3:33 AM, Sean Whitton wrote:
> I've installed the change, thank you again.
> 
> I copy-edited your manual text a bit while doing so.

Thanks for merging. This fell off my radar.

Previous Next

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