From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!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: Fri, 04 Sep 2020 15:07:50 +0300 Message-ID: <83o8mlu4mh.fsf@gnu.org> References: <43aac56d-ecf1-44ed-9be1-ffb8e5f8a7ce@default> <83zhfbm448.fsf@gnu.org> <87zh8pij6e.fsf@calancha-pc.dy.bbexcite.jp> <83v9jc3o3q.fsf@gnu.org> <87wo3qj1s6.fsf@calancha-pc.dy.bbexcite.jp> <87eenygyrx.fsf@gnus.org> <87d03beq3g.fsf@gmail.com> <83zh6fz4q1.fsf@gnu.org> <87363xj3be.fsf@gmail.com> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="7875"; mail-complaints-to="usenet@ciao.gmane.io" Cc: larsi@gnus.org, monnier@iro.umontreal.ca, uyennhi.qm@gmail.com, 38796@debbugs.gnu.org, stefankangas@gmail.com To: Tino Calancha Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Sep 04 14:09: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 1kEAWl-0001xm-CR for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 04 Sep 2020 14:09:11 +0200 Original-Received: from localhost ([::1]:58016 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kEAWk-00056n-BL for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 04 Sep 2020 08:09:10 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:49128) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kEAWc-00056T-3I for bug-gnu-emacs@gnu.org; Fri, 04 Sep 2020 08:09:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:54395) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kEAWb-0000eX-PR for bug-gnu-emacs@gnu.org; Fri, 04 Sep 2020 08:09:01 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1kEAWb-0007T6-J9 for bug-gnu-emacs@gnu.org; Fri, 04 Sep 2020 08:09: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: Fri, 04 Sep 2020 12:09:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 38796 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 38796-submit@debbugs.gnu.org id=B38796.159922129228650 (code B ref 38796); Fri, 04 Sep 2020 12:09:01 +0000 Original-Received: (at 38796) by debbugs.gnu.org; 4 Sep 2020 12:08:12 +0000 Original-Received: from localhost ([127.0.0.1]:37708 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kEAVn-0007S2-R1 for submit@debbugs.gnu.org; Fri, 04 Sep 2020 08:08:12 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:38882) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kEAVk-0007Ro-A0 for 38796@debbugs.gnu.org; Fri, 04 Sep 2020 08:08:10 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:40281) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kEAVe-0000ZW-KV; Fri, 04 Sep 2020 08:08:02 -0400 Original-Received: from [176.228.60.248] (port=1830 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1kEAVd-00032I-K8; Fri, 04 Sep 2020 08:08:02 -0400 In-Reply-To: <87363xj3be.fsf@gmail.com> (message from Tino Calancha on Fri, 04 Sep 2020 11:31:33 +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:187112 Archived-At: > From: Tino Calancha > Cc: larsi@gnus.org, stefankangas@gmail.com, monnier@iro.umontreal.ca, > 38796@debbugs.gnu.org, uyennhi.qm@gmail.com > Date: Fri, 04 Sep 2020 11:31:33 +0200 > > Eli Zaretskii writes: > > >> FWIW I have been running Emacs almost 3 months using this branch > >> with no issues. > >> If Eli is OK with it, I can merge it to master next week. > > > > Sorry, OK with what? where's the patch which I should agree with? > > Branch origin/bug#38796-lossage-limit > > The following is the difference with master branch when I updated > the branch (last August 27th): Not sure what that means. Is this the patch you suggest installing, or does it need more work to adapt it to the current master? > If something surprising happens, and you are not sure what you typed, > use @kbd{C-h l} (@code{view-lossage}). @kbd{C-h l} displays your last > -300 input keystrokes and the commands they invoked. If you see > -commands that you are not familiar with, you can use @kbd{C-h k} or > +input keystrokes and the commands they invoked. By default, Emacs > +stores the last 300 events; if you wish, you can change this number with ^^^^^^^^^^^^^^^ The first sentence talks about keystrokes, but the last sentence talks about "events". The reader might become confused whether these two terms refer to the same entity. > ++++ > +** The new command 'lossage-limit' controls the maximum number > +of keystrokes and commands recorded. > + > +++ > ** New variables that hold default buffer names for shell output. > The new constants 'shell-command-buffer-name' and > @@ -92,6 +96,9 @@ The new constants 'shell-command-buffer-name' and > for the output of, respectively, synchronous and async shell > commands. > > +** The new command lossage-size' allow users to set the maximum Missing opening quote for lossage-size. > +number of keystrokes and commands recorded. NEWS mentions 2 separate commands, but I see only one in the implementation. > --- a/lisp/edmacro.el > +++ b/lisp/edmacro.el > @@ -35,8 +35,8 @@ > ;; * `M-x' followed by a command name, to edit a named command > ;; whose definition is a keyboard macro. > ;; > -;; * `C-h l' (view-lossage), to edit the 300 most recent keystrokes > -;; and install them as the "current" macro. > +;; * `C-h l' (view-lossage), to edit the 300 most recent > +;; keystrokes and install them as the "current" macro. This change is a no-op; why is it needed? > (defun view-lossage () > - "Display last few input keystrokes and the commands run. > + "Display last input keystrokes and the commands run. Why this change? > -/* This vector holds the last NUM_RECENT_KEYS keystrokes. */ > +/* Size of the recent_keys vector */ ^^^ A comment should end in a period and 2 spaces before "*/". > /* Pointer to next place to store character in kbd_buffer. */ > static union buffered_input_event *kbd_store_ptr; > > + Do we really need to add an empty line here? > +DEFUN ("lossage-size", Flossage_size, Slossage_size, 0, 1, > + "(list (read-number \"new-size: \" (lossage-size)))", > + doc: /* Return the maximum number of saved keystrokes. The first line describes only one of the two functionalities of this command; it should describe both. > +Called with ARG, then set this number to ARG. "ARG non-nil means set the maximum number of keystrokes to that number." > +The saved keystrokes are the records shown by `view-lossage'. > +If you want to disable the lossage records, then set this maximum to a > +small number, e.g. 0. The "small number, e.g." part is inaccurate: it _must_ be zero, right? > +usage: (lossage-size &optional ARG) */) Is this "usage" needed? what happens if you don't use it? > + if (NILP(arg)) > + return make_fixnum(lossage_limit == 1 ? 0 : lossage_limit); ^ Space here. So if the user sets the limit to 1, the next call to lossage-size will return zero? Isn't that confusing? > + /* Internally, the minimum lossage_limit is 1; users will likely use > + 0 to disable the lossage, thus here we change 0 -> 1. */ > + if (new_size == 0) > + new_size = 1; I still don't like this. I think it will cause confusion and errors. > + return Qnil; Why return nil when setting the limit? why not the previous limit? > +(ert-deftest keyboard-lossage-size () > + "Test `lossage-size'." > + (dolist (val (list 100 300 400 400 500 1000 700 300)) > + (lossage-size val) > + (should (= val (lossage-size)))) This doesn't test the actual recording of VAL events. Thanks.