Package: emacs;
Reported by: Drew Adams <drew.adams <at> oracle.com>
Date: Sun, 29 Dec 2019 17:06:01 UTC
Severity: wishlist
Tags: patch
Found in version 26.3
Done: Tino Calancha <tino.calancha <at> gmail.com>
Bug is archived. No further changes may be made.
Message #16 received at 38796 <at> debbugs.gnu.org (full text, mbox):
From: Tino Calancha <tino.calancha <at> gmail.com> To: 38796 <at> debbugs.gnu.org Cc: Eli Zaretskii <eliz <at> gnu.org>, tino.calancha <at> gmail.com, Stefan Monnier <monnier <at> iro.umontreal.ca>, 38796 <at> debbugs.gnu.org, uyennhi.qm <at> gmail.com Subject: Re: bug#38796: 26.3; `view-lossage': Use a variable for the lossage limit Date: Fri, 26 Jun 2020 23:58:01 +0200
Eli Zaretskii <eliz <at> gnu.org> writes: >> From: Drew Adams <drew.adams <at> oracle.com> >> 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. --8<-----------------------------cut here---------------start------------->8--- commit 9723c2645c8f47e651a81a023e80f882d181cc3f Author: Tino Calancha <tino.calancha <at> gmail.com> Date: Sun Jun 7 23:37:59 2020 +0200 Give Lisp control on the lossage size Add an user option to control the maximum number of recorded keystrokes (a.k.a lossage limit) (Bug#38796). * src/keyboard.c (lossage-limit): Add new variable. (MIN_NUM_RECENT_KEYS): Renamed from NUM_RECENT_KEYS. Set it as 100 and use it as the minimum value for lossage-limit. Keep the same default for the vector size as before (300). (update-lossage-limit): New function. (update_recent_keys): Helper function. (command_loop_1) (record_char) (recent-keys) (syms_of_keyboard): Replace NUM_RECENT_KEYS with lossage_limit as the vector size. (clear-this-command-keys): Fix docstring. * lisp/help.el (view-lossage): Mention lossage-limit in the docstring. * lisp/cus-start.el (lossage-limit): Register it as an user option. * lisp/edmacro.el (edit-kbd-macro): Update docstring and commentary header. * etc/NEWS (Changes in Emacs 28.1): Announce the new option. * doc/emacs/help.texi (Misc Help): Document it. * test/src/keyboard-tests.el (keyboard-lossage-limit): Add test. diff --git a/doc/emacs/help.texi b/doc/emacs/help.texi index 167c32c4d2..e2c0dc6802 100644 --- a/doc/emacs/help.texi +++ b/doc/emacs/help.texi @@ -563,10 +563,13 @@ Misc Help @kindex C-h l @findex view-lossage +@vindex lossage-limit 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 option @code{lossage-limit}. +If you see commands that you are not familiar with, you can use @kbd{C-h k} or @kbd{C-h f} to find out what they do. @kindex C-h e diff --git a/etc/NEWS b/etc/NEWS index d58a61be21..da204c5825 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -70,6 +70,10 @@ useful on systems such as FreeBSD which ships only with "etc/termcap". * Changes in Emacs 28.1 ++++ +** The new option 'lossage-limit' controls the maximum number +of keystrokes and commands recorded. + ** Support for '(box . SIZE)' 'cursor-type'. By default, 'box' cursor always has a filled box shape. But if you specify 'cursor-type' to be '(box . SIZE)', the cursor becomes a hollow diff --git a/lisp/cus-start.el b/lisp/cus-start.el index 6632687da4..1ebf554b2b 100644 --- a/lisp/cus-start.el +++ b/lisp/cus-start.el @@ -352,6 +352,8 @@ minibuffer-prompt-properties--setter ;; indent.c (indent-tabs-mode indent boolean) ;; keyboard.c + (lossage-limit keyboard integer "28.1" + :set (lambda (_ val) (update-lossage-limit val))) (meta-prefix-char keyboard character) (auto-save-interval auto-save integer) (auto-save-no-message auto-save boolean "27.1") diff --git a/lisp/edmacro.el b/lisp/edmacro.el index 71474c0289..089fea17ef 100644 --- 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 `lossage-limit' most recent +;; keystrokes and install them as the "current" macro. ;; ;; * any key sequence whose definition is a keyboard macro. ;; @@ -88,7 +88,7 @@ edit-kbd-macro "Edit a keyboard macro. At the prompt, type any key sequence which is bound to a keyboard macro. Or, type `\\[kmacro-end-and-call-macro]' or RET to edit the last -keyboard macro, `\\[view-lossage]' to edit the last 300 +keyboard macro, `\\[view-lossage]' to edit the last `lossage-limit' keystrokes as a keyboard macro, or `\\[execute-extended-command]' to edit a macro by its command name. With a prefix argument, format the macro in a more concise way." diff --git a/lisp/help.el b/lisp/help.el index b7d867eb70..9e3d0922d0 100644 --- a/lisp/help.el +++ b/lisp/help.el @@ -456,9 +456,10 @@ view-external-packages (info "(efaq)Packages that do not come with Emacs")) (defun view-lossage () - "Display last few input keystrokes and the commands run. + "Display last input keystrokes and the commands run. For convenience this uses the same format as `edit-last-kbd-macro'. +See `lossage-limit' to update the number of recorded keystrokes. To record all your input, use `open-dribble-file'." (interactive) diff --git a/src/keyboard.c b/src/keyboard.c index f9b9399d50..a4d27c94fb 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -103,7 +103,7 @@ static KBOARD *all_kboards; /* True in the single-kboard state, false in the any-kboard state. */ static bool single_kboard; -#define NUM_RECENT_KEYS (300) +#define MIN_NUM_RECENT_KEYS (100) /* Index for storing next element into recent_keys. */ static int recent_keys_index; @@ -111,7 +111,7 @@ static int recent_keys_index; /* Total number of elements stored into recent_keys. */ static int total_keys; -/* This vector holds the last NUM_RECENT_KEYS keystrokes. */ +/* This vector holds the last lossage_limit keystrokes. */ static Lisp_Object recent_keys; /* Vector holding the key sequence that invoked the current command. @@ -294,6 +294,7 @@ static union buffered_input_event *kbd_fetch_ptr; /* Pointer to next place to store character in kbd_buffer. */ static union buffered_input_event *kbd_store_ptr; + /* The above pair of variables forms a "queue empty" flag. When we enqueue a non-hook event, we increment kbd_store_ptr. When we dequeue a non-hook event, we increment kbd_fetch_ptr. We say that @@ -1421,10 +1422,10 @@ command_loop_1 (void) /* Execute the command. */ { - total_keys += total_keys < NUM_RECENT_KEYS; + total_keys += total_keys < lossage_limit; ASET (recent_keys, recent_keys_index, Fcons (Qnil, cmd)); - if (++recent_keys_index >= NUM_RECENT_KEYS) + if (++recent_keys_index >= lossage_limit) recent_keys_index = 0; } Vthis_command = cmd; @@ -3248,15 +3249,15 @@ record_char (Lisp_Object c) int ix1, ix2, ix3; if ((ix1 = recent_keys_index - 1) < 0) - ix1 = NUM_RECENT_KEYS - 1; + ix1 = lossage_limit - 1; ev1 = AREF (recent_keys, ix1); if ((ix2 = ix1 - 1) < 0) - ix2 = NUM_RECENT_KEYS - 1; + ix2 = lossage_limit - 1; ev2 = AREF (recent_keys, ix2); if ((ix3 = ix2 - 1) < 0) - ix3 = NUM_RECENT_KEYS - 1; + ix3 = lossage_limit - 1; ev3 = AREF (recent_keys, ix3); if (EQ (XCAR (c), Qhelp_echo)) @@ -3307,12 +3308,12 @@ record_char (Lisp_Object c) { if (!recorded) { - total_keys += total_keys < NUM_RECENT_KEYS; + total_keys += total_keys < lossage_limit; ASET (recent_keys, recent_keys_index, /* Copy the event, in case it gets modified by side-effect by some remapping function (bug#30955). */ CONSP (c) ? Fcopy_sequence (c) : c); - if (++recent_keys_index >= NUM_RECENT_KEYS) + if (++recent_keys_index >= lossage_limit) recent_keys_index = 0; } else if (recorded < 0) @@ -3326,10 +3327,10 @@ record_char (Lisp_Object c) while (recorded++ < 0 && total_keys > 0) { - if (total_keys < NUM_RECENT_KEYS) + if (total_keys < lossage_limit) total_keys--; if (--recent_keys_index < 0) - recent_keys_index = NUM_RECENT_KEYS - 1; + recent_keys_index = lossage_limit - 1; ASET (recent_keys, recent_keys_index, Qnil); } } @@ -10410,6 +10411,55 @@ If CHECK-TIMERS is non-nil, timers that are ready to run will do so. */) ? Qt : Qnil); } +static void +update_recent_keys (int new_size, int kept_keys) +{ + int osize = ASIZE (recent_keys); + eassert (recent_keys_index < osize); + eassert (kept_keys <= min (osize, new_size)); + Lisp_Object v = make_nil_vector (new_size); + int i, idx; + for (i = 0; i < kept_keys; ++i) + { + idx = recent_keys_index - kept_keys + i; + while (idx < 0) + idx += osize; + ASET (v, i, AREF (recent_keys, idx)); + } + recent_keys = v; + total_keys = kept_keys; + recent_keys_index = total_keys % new_size; + lossage_limit = new_size; + +} + +/* Reallocate recent_keys copying the keystrokes in the right order */ +DEFUN ("update-lossage-limit", Fupdate_lossage_limit, + Supdate_lossage_limit, 1, 1, 0, + doc: /* Update the maximum number of saved keystrokes to ARG. +usage: (update-lossage-limit ARG) */) + (Lisp_Object arg) +{ + if (!FIXNATP (arg)) + user_error ("Value must be a positive integer"); + int osize = ASIZE (recent_keys); + eassert (lossage_limit == osize); + int min_size = MIN_NUM_RECENT_KEYS; + int new_size = XFIXNAT (arg); + + if (new_size == osize) + return Qnil; + if (new_size < min_size) + { + AUTO_STRING (fmt, "Value must be >= %d"); + Fsignal (Quser_error, list1 (CALLN (Fformat, fmt, make_fixnum (min_size)))); + } + int kept_keys = new_size > osize ? total_keys : min (new_size, total_keys); + update_recent_keys (new_size, kept_keys); + + return Qnil; +} + DEFUN ("recent-keys", Frecent_keys, Srecent_keys, 0, 1, 0, doc: /* Return vector of last few events, not counting those from keyboard macros. If INCLUDE-CMDS is non-nil, include the commands that were run, @@ -10419,21 +10469,21 @@ represented as pseudo-events of the form (nil . COMMAND). */) bool cmds = !NILP (include_cmds); if (!total_keys - || (cmds && total_keys < NUM_RECENT_KEYS)) + || (cmds && total_keys < lossage_limit)) return Fvector (total_keys, XVECTOR (recent_keys)->contents); else { Lisp_Object es = Qnil; - int i = (total_keys < NUM_RECENT_KEYS + int i = (total_keys < lossage_limit ? 0 : recent_keys_index); - eassert (recent_keys_index < NUM_RECENT_KEYS); + eassert (recent_keys_index < lossage_limit); do { Lisp_Object e = AREF (recent_keys, i); if (cmds || !CONSP (e) || !NILP (XCAR (e))) es = Fcons (e, es); - if (++i >= NUM_RECENT_KEYS) + if (++i >= lossage_limit) i = 0; } while (i != recent_keys_index); es = Fnreverse (es); @@ -10531,8 +10581,8 @@ The value is always a vector. */) DEFUN ("clear-this-command-keys", Fclear_this_command_keys, Sclear_this_command_keys, 0, 1, 0, doc: /* Clear out the vector that `this-command-keys' returns. -Also clear the record of the last 100 events, unless optional arg -KEEP-RECORD is non-nil. */) +Also clear the record of the last `lossage-limit' keystroke events, +unless optional arg KEEP-RECORD is non-nil. */) (Lisp_Object keep_record) { int i; @@ -11686,7 +11736,23 @@ syms_of_keyboard (void) staticpro (&modifier_symbols); } - recent_keys = make_nil_vector (NUM_RECENT_KEYS); + DEFVAR_INT ("lossage-limit", lossage_limit, + doc: /* Maximum number of stored keyboard events and commands run. + +Please, do not set this variable in Lisp with `setq' neither +let-bind it, that will likely crash Emacs. This is because +`setq' only changes the variable, but it doesn't update +the size of the internal vector that holds the keystrokes. + +To update this variable use the customization menu, or +call from Lisp the following expression: + + (update-lossage-limit new-limit) + +That takes care of both, the variable and the internal vector.*/); + lossage_limit = 3 * MIN_NUM_RECENT_KEYS; + + recent_keys = make_nil_vector (lossage_limit); staticpro (&recent_keys); this_command_keys = make_nil_vector (40); @@ -11736,6 +11802,7 @@ syms_of_keyboard (void) defsubr (&Srecursive_edit); defsubr (&Sinternal_track_mouse); defsubr (&Sinput_pending_p); + defsubr (&Supdate_lossage_limit); defsubr (&Srecent_keys); defsubr (&Sthis_command_keys); defsubr (&Sthis_command_keys_vector); diff --git a/test/src/keyboard-tests.el b/test/src/keyboard-tests.el index 1988ba51a7..017d239246 100644 --- a/test/src/keyboard-tests.el +++ b/test/src/keyboard-tests.el @@ -32,5 +32,16 @@ (read-event nil nil 2)) ?\C-b))) +(ert-deftest keyboard-lossage-limit () + "Test `lossage-limit' updates." + (dolist (val (list 100 100 200 500 300 1000 700)) + (update-lossage-limit val) + (should (= val lossage-limit))) + (let ((current-limit lossage-limit)) + (should-error (update-lossage-limit 5)) + (should-error (update-lossage-limit "200")) + (should (= lossage-limit current-limit)))) + + (provide 'keyboard-tests) ;;; keyboard-tests.el ends here commit a60a05994aff16bc27f153ea8f765e15b92f21be Author: Tino Calancha <tino.calancha <at> gmail.com> Date: Mon Jun 22 22:41:34 2020 +0200 Allow disable the record of keystrokes (lossage) Use 1 as the minimum value for lossage-limit; such a value is equivalent to not recording the keystrokes: having just 1 entry, will be overwritten with the view-lossage call itself. * test/src/keyboard-tests.el (keyboard-lossage-limit): Update test. * src/keyboard.c (MIN_NUM_RECENT_KEYS): Delete it. (lossage_limit): Add security note in the doctring. * lisp/cus-start.el (lossage-limit): Let users choose to disable the record of the keystrokes. * doc/emacs/help.texi (Misc Help): Update manual. * etc/NEWS (Changes in Emacs 28.1): Mention that it's possible to disable the record of keystrokes. diff --git a/doc/emacs/help.texi b/doc/emacs/help.texi index e2c0dc6802..69ea96763b 100644 --- a/doc/emacs/help.texi +++ b/doc/emacs/help.texi @@ -569,8 +569,14 @@ Misc Help 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 option @code{lossage-limit}. -If you see commands that you are not familiar with, you can use @kbd{C-h k} or -@kbd{C-h f} to find out what they do. +If you see commands that you are not familiar with, you can use @kbd{C-h k} +or @kbd{C-h f} to find out what they do. +If you don't like that Emacs saves your keystrokes, then you can +set @code{lossage-limit} equal to 1; such a value effectively disables the +record of the keystrokes. Please, do not set this option with @code{setq} +neither let-bind it; that will likely crash Emacs. Use instead the +customization menu, which also updates the internal structure holding +the keystrokes. @kindex C-h e @findex view-echo-area-messages diff --git a/etc/NEWS b/etc/NEWS index da204c5825..17776173e0 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -72,7 +72,8 @@ useful on systems such as FreeBSD which ships only with "etc/termcap". +++ ** The new option 'lossage-limit' controls the maximum number -of keystrokes and commands recorded. +of keystrokes and commands recorded. The value 1 disables +the record of keystrokes. ** Support for '(box . SIZE)' 'cursor-type'. By default, 'box' cursor always has a filled box shape. But if you diff --git a/lisp/cus-start.el b/lisp/cus-start.el index 1ebf554b2b..bd4ff3a74b 100644 --- a/lisp/cus-start.el +++ b/lisp/cus-start.el @@ -352,7 +352,11 @@ minibuffer-prompt-properties--setter ;; indent.c (indent-tabs-mode indent boolean) ;; keyboard.c - (lossage-limit keyboard integer "28.1" + (lossage-limit keyboard + (choice (const :tag "Do not record keystrokes" 1) + integer) + "28.1" + :standard 300 :set (lambda (_ val) (update-lossage-limit val))) (meta-prefix-char keyboard character) (auto-save-interval auto-save integer) diff --git a/src/keyboard.c b/src/keyboard.c index a4d27c94fb..9af1b9ba50 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -103,8 +103,6 @@ static KBOARD *all_kboards; /* True in the single-kboard state, false in the any-kboard state. */ static bool single_kboard; -#define MIN_NUM_RECENT_KEYS (100) - /* Index for storing next element into recent_keys. */ static int recent_keys_index; @@ -10444,7 +10442,7 @@ usage: (update-lossage-limit ARG) */) user_error ("Value must be a positive integer"); int osize = ASIZE (recent_keys); eassert (lossage_limit == osize); - int min_size = MIN_NUM_RECENT_KEYS; + int min_size = 1; int new_size = XFIXNAT (arg); if (new_size == osize) @@ -11749,8 +11747,11 @@ call from Lisp the following expression: (update-lossage-limit new-limit) -That takes care of both, the variable and the internal vector.*/); - lossage_limit = 3 * MIN_NUM_RECENT_KEYS; +That takes care of both, the variable and the internal vector. + +Security note: The value 1 makes impossible to recover a typed string +with `view-lossage'.*/); + lossage_limit = 300; recent_keys = make_nil_vector (lossage_limit); staticpro (&recent_keys); diff --git a/test/src/keyboard-tests.el b/test/src/keyboard-tests.el index 017d239246..4541c389d0 100644 --- a/test/src/keyboard-tests.el +++ b/test/src/keyboard-tests.el @@ -38,7 +38,7 @@ (update-lossage-limit val) (should (= val lossage-limit))) (let ((current-limit lossage-limit)) - (should-error (update-lossage-limit 5)) + (should-error (update-lossage-limit 0)) (should-error (update-lossage-limit "200")) (should (= lossage-limit current-limit)))) --8<-----------------------------cut here---------------end--------------->8--- In GNU Emacs 28.0.50 (build 18, x86_64-pc-linux-gnu, GTK+ Version 3.24.5, cairo version 1.16.0) of 2020-06-26 built on calancha-pc.dy.bbexcite.jp Repository revision: ffb89ed5f07491e33fc79d8b4be49d9deba2ad4a Repository branch: master Windowing system distributor 'The X.Org Foundation', version 11.0.12004000 System Description: Debian GNU/Linux 10 (buster)
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.