From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Jim Porter Newsgroups: gmane.emacs.bugs Subject: bug#66768: 30.0.50; [PATCH] Improve read/append behavior of eshell history command Date: Fri, 3 Nov 2023 12:38:35 -0700 Message-ID: <3d7b6c5e-ee22-91a2-93bd-1fd99d3eb301@gmail.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="7870"; mail-complaints-to="usenet@ciao.gmane.io" To: Liu Hui , 66768@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Nov 03 20:39:52 2023 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1qz01H-0001wb-NR for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 03 Nov 2023 20:39:51 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qz00y-0002pf-Bx; Fri, 03 Nov 2023 15:39:32 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qz00w-0002pV-96 for bug-gnu-emacs@gnu.org; Fri, 03 Nov 2023 15:39:30 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qz00s-0000JN-V5 for bug-gnu-emacs@gnu.org; Fri, 03 Nov 2023 15:39:28 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1qz01S-00078x-BP for bug-gnu-emacs@gnu.org; Fri, 03 Nov 2023 15:40:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Jim Porter Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 03 Nov 2023 19:40:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 66768 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 66768-submit@debbugs.gnu.org id=B66768.169904036227388 (code B ref 66768); Fri, 03 Nov 2023 19:40:02 +0000 Original-Received: (at 66768) by debbugs.gnu.org; 3 Nov 2023 19:39:22 +0000 Original-Received: from localhost ([127.0.0.1]:59864 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qz00n-00077g-Vg for submit@debbugs.gnu.org; Fri, 03 Nov 2023 15:39:22 -0400 Original-Received: from mail-pf1-x430.google.com ([2607:f8b0:4864:20::430]:60543) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qz00l-00077O-Bi for 66768@debbugs.gnu.org; Fri, 03 Nov 2023 15:39:20 -0400 Original-Received: by mail-pf1-x430.google.com with SMTP id d2e1a72fcca58-6c3077984e8so2355860b3a.0 for <66768@debbugs.gnu.org>; Fri, 03 Nov 2023 12:38:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1699040318; x=1699645118; darn=debbugs.gnu.org; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:mime-version:date:message-id:from:to:cc :subject:date:message-id:reply-to; bh=BcUXs2hTx4u3mPntPy448L+vGAxIkvMQFd/DZjlDyw8=; b=MUarj2SeBs3O20NeVa6D+lcV2BF1lukdUSB0WlK0lQVbw+Av/TDFVX3rhkKrQSMy1e vGAhEeD33V3jly0tKRYkC/Bd82dp6QdRZ0/jjqVJwOObpfNYQ/VN6wDY3wj/7TZBr7Tm dO4OqrpMM2HPLttYZ2N02SNLOEmKB38kOrGZfBOKdFcJfOGsv4kmCHradd9G/pTRMvgn Jwf4lFlRpyO0PWXr7o/CiinclfAs8cAPGKknOrrjy7kmpRLWeFSNbPE3+UCK8n9tlD2+ gsV744BWLhNnqUYoqAG7icP2wZ5DdAwboiI9K6IZ1/XADLjGS09oWEmVxrUzgEkmvAEa //NA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699040318; x=1699645118; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=BcUXs2hTx4u3mPntPy448L+vGAxIkvMQFd/DZjlDyw8=; b=sMH4SO0MRmxR3djSW2v5aBnDxUHi/SmrQrI5IEoYi+zFlD9Y4ytGf9zFHVYDSNsfOb HcIn8YyW2iSSEN3FJdmnRfCulnT6w3SuDOFmEjAiMUSfNxgKZdpXMVxA/Asp3eVsMk7E 0/cCYciZn5sXZhl1nkJlFA9qB+7VmDv5w80CvRxXvtkIBHRf70+3NQkXizpgG+X3u+Kj HAafgbALQ0MC9Qr131OENgFtN2epq+AHOCUlyhDpLfytzeEzu0no8Gge6Em/94icU2Jr 878DFdeFFEMQ3QUOotyZzZWBqBzfORLIMQRTExzYLkvC61+DMoUysUaq871dqiTxbF07 dZkQ== X-Gm-Message-State: AOJu0YyF0VnUM0URY8s35HH0b5zxnYeH0taDgwWSU7kTxH94MHHiehwC hwJEtCITga0M4PEyx5agQnY= X-Google-Smtp-Source: AGHT+IF9XR215YEx3sS7zX09eib3q63vhbWff7a3fXFGfLN8fPs8NUR3wsmP3CNO1EfbIzEILMmKgQ== X-Received: by 2002:a05:6a00:cc8:b0:68f:dd50:aef8 with SMTP id b8-20020a056a000cc800b0068fdd50aef8mr24663787pfv.4.1699040317642; Fri, 03 Nov 2023 12:38:37 -0700 (PDT) Original-Received: from [192.168.1.2] (cpe-76-168-148-233.socal.res.rr.com. [76.168.148.233]) by smtp.googlemail.com with ESMTPSA id t8-20020a63dd08000000b005b628aa2a8dsm1589926pgg.69.2023.11.03.12.38.36 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 03 Nov 2023 12:38:37 -0700 (PDT) Content-Language: en-US In-Reply-To: X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:273724 Archived-At: On 10/26/2023 9:15 PM, Liu Hui wrote: > This patch fixes some corner cases of the eshell history command about > reading (-r) and appending (-a). Thanks for the patch. I think this makes sense overall, though I do want to go through em-hist.el at some point and rework how shared history functions. (I use shared history from multiple Zsh shells pretty extensively, so I'm hoping to at least provide the necessary hooks to make Eshell work like my setup.) 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.