From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.ciao.gmane.io!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#38796: 26.3; `view-lossage': Use a variable for the lossage limit Date: Sat, 27 Jun 2020 11:32:57 +0300 Message-ID: <83v9jc3o3q.fsf@gnu.org> References: <43aac56d-ecf1-44ed-9be1-ffb8e5f8a7ce@default> <83zhfbm448.fsf@gnu.org> <87zh8pij6e.fsf@calancha-pc.dy.bbexcite.jp> Injection-Info: ciao.gmane.io; posting-host="ciao.gmane.io:159.69.161.202"; logging-data="19594"; mail-complaints-to="usenet@ciao.gmane.io" Cc: monnier@iro.umontreal.ca, 38796@debbugs.gnu.org, uyennhi.qm@gmail.com To: Tino Calancha Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Jun 27 10:34:12 2020 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 1jp6Hs-0004vx-Fw for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 27 Jun 2020 10:34:12 +0200 Original-Received: from localhost ([::1]:49298 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jp6Hq-0005kd-Aw for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 27 Jun 2020 04:34:10 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:37238) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jp6Hj-0005kU-NL for bug-gnu-emacs@gnu.org; Sat, 27 Jun 2020 04:34:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:60139) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jp6Hi-0008Uh-EN for bug-gnu-emacs@gnu.org; Sat, 27 Jun 2020 04:34:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1jp6Hh-0007jM-Jj for bug-gnu-emacs@gnu.org; Sat, 27 Jun 2020 04:34:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 27 Jun 2020 08:34:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 38796 X-GNU-PR-Package: emacs Original-Received: via spool by 38796-submit@debbugs.gnu.org id=B38796.159324680329665 (code B ref 38796); Sat, 27 Jun 2020 08:34:01 +0000 Original-Received: (at 38796) by debbugs.gnu.org; 27 Jun 2020 08:33:23 +0000 Original-Received: from localhost ([127.0.0.1]:43452 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jp6H5-0007iO-9b for submit@debbugs.gnu.org; Sat, 27 Jun 2020 04:33:23 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:50886) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jp6H1-0007iB-VL for 38796@debbugs.gnu.org; Sat, 27 Jun 2020 04:33:21 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:54653) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jp6Gw-0008S0-AW; Sat, 27 Jun 2020 04:33:14 -0400 Original-Received: from [176.228.60.248] (port=2841 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1jp6Gv-0002oM-Lp; Sat, 27 Jun 2020 04:33:14 -0400 In-Reply-To: <87zh8pij6e.fsf@calancha-pc.dy.bbexcite.jp> (message from Tino Calancha on Fri, 26 Jun 2020 23:58:01 +0200) 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" Xref: news.gmane.io gmane.emacs.bugs:182431 Archived-At: > From: Tino Calancha > Cc: 38796@debbugs.gnu.org, Eli Zaretskii , Stefan Monnier > ,uyennhi.qm@gmail.com, tino.calancha@gmail.com > Date: Fri, 26 Jun 2020 23:58:01 +0200 > > Eli Zaretskii writes: > > >> From: Drew Adams > >> This report is a follow-up to this emacs-devel thread: > >> https://lists.gnu.org/archive/html/emacs-devel/2019-12/msg00678.html > >> and to the older thread that it references: > >> https://lists.gnu.org/archive/html/emacs-devel/2006-03/msg01012.html > >> Please use a variable for the limit of how many characters to use for > >> recording lossage. > > > it isn't enough to expose the value to Lisp; one must > > also write code that reallocates the vector when the value changes, > > and copies the keystrokes from the old to the new vector in the > > correct order. > > > > Patches are welcome. > > I have uploaded the branch feature/bug#38796-lossage-limit > [I've been using it during last 10 days with no issues] > > Initially I decided to hardcode a sensible minimum of 100 (keeping > the current default of 300). That is the first commit. > Actually, the code works for N > 0, but as Drew suggested, a value < 100 > (which gives only ~ 50 lines of past inputs) is too small to be useful > for the use case of checking typing mistakes. > > After some time I thought that, for some people (anyone in this thread), > a very small value in fact can be seen as a security feature; > see for instance the docstrings of `term-read-noecho' and > `comint-send-invisible'. That motivates the second commit. Thanks. Some initial comments below: . The original request was for allowing to keep _more_ than just 300 keystrokes, there was no request for recording less than that number. I'm not sure we should allow reducing the number; if someone thinks it could be a security issue, then we should have a separate feature that disabled recording the keys, not as a side effect of reducing the vector size. . The size of the vector shouldn't be exposed to Lisp, there's no reason for doing that. You say in the doc string of the variable not to set nor bind it, but documenting the restriction is not enough, as someone, perhaps even some malicious code, could just do what you ask not to. Instead, there should be a function to get the value and a function to set the value (which should also resize the vector as a side effect). . Using 1 as the value that disables recording is unnatural; it might be convenient from the implementation POV, but this implementation convenience cannot justify asking the users to remember such a strange convention. The usual way of disabling a feature is either by setting the value to zero or using a suitable symbol, like nil. (But I'm not sure we should provide this feature as part of this changeset, see below.) . Last, but not least: if we are going to allow to disable recording of keys, we should carefully audit all the places that call recent-keys, and make sure they will not break due to such a radical change. My personal view is that we should allow only growing the size and resetting it to the original size of 300. Disabling the key record should be a separate feature, most probably implemented by means other than shrinking the recent_keys vector. Thanks.