GNU bug report logs - #66768
30.0.50; [PATCH] Improve read/append behavior of eshell history command

Previous Next

Full log

View this message in rfc822 format

From: Liu Hui <liuhui1610 <at> gmail.com>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 66768 <at> debbugs.gnu.org
Subject: bug#66768: 30.0.50; [PATCH] Improve read/append behavior of eshell history command
Date: Tue, 7 Nov 2023 18:14:30 +0800

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

Jim Porter <jporterbugs <at> 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.

[0001-Improve-read-append-behavior-of-eshell-history-comma.patch (application/x-patch, attachment)]

Previous Next

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