Jim Porter <jporterbugs@gmail.com> 于2023年11月4日周六 03:38写道：

> In the meantime though, this is a step in the right direction. Some
> comments on the code:
>
> > +(defvar eshell-hist--new-items 0
> > +  "The number of new history items that have not been written to
> > +file.  This variable is local in each eshell buffer.")
>
> To prevent mistakes, I'd set this to nil here, and then call
> '(setq-local eshell-hist--new-items 0)' in 'eshell-hist-initialize'.
>
> > -(defun eshell-write-history (&optional filename append)
> > +(defun eshell-write-history (&optional filename append new-items)
>
> I don't think this new argument is necessary. I suppose you did this for
> backwards-compatibility, but I'd say that the current appending behavior
> is just a bug, so you don't need to add an explicit way to opt into your
> new behavior; just do it whenever 'append' is non-nil.
>
> Some regression tests would also be nice. There are already a few in
> test/lisp/eshell/em-hist-tests.el that you should be able to use as a basis.

Thanks for your comments! I've updated the patch to address all
concerns.