bug#66768: 30.0.50; [PATCH] Improve read/append behavior of eshell history command

[Liu Hui <liuhui1610@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1147 bytes --]

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.

[-- Attachment #2: 0001-Improve-read-append-behavior-of-eshell-history-comma.patch --]
[-- Type: application/x-patch, Size: 7814 bytes --]