* bug#38796: 26.3; `view-lossage': Use a variable for the lossage limit @ 2019-12-29 17:04 Drew Adams 2019-12-29 17:30 ` Eli Zaretskii 0 siblings, 1 reply; 18+ messages in thread From: Drew Adams @ 2019-12-29 17:04 UTC (permalink / raw) To: 38796 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. Doing that will give users (and Lisp code) control over the limit. Having a limit that is hard-coded in C is not user-friendly - it's even unemacsy. The latter thread mentioned above ended with RMS's decision to have this taken care of after the release that was then incipient. But this was never done. I'd prefer to see this as a user option, but an ordinary defvar would already be a great improvement. I don't think it's uncommon to have mistakenly typed some keys, gotten in a strange or unfamiliar situation as a result of that, and then tried to find out just what keys led to that. Depending on what else you did in the meantime, before hitting `C-h l' (e.g., to get out of the strange situation), `view-lossage' might not reach far enough backward to show you the mistake you made. It shouldn't be a big deal to move this limit to a Lisp variable. Can this please be done now? --- Not part of this bug report, but `view-lossage' could also be improved by letting you (or Lisp code) filter the recorded "keystroke" history, to remove stuff that you feel is really extraneous (e.g. not keyboard keystrokes). Such filtering could be just hiding that from view. (This is all the more relevant if we can control the lossage limit. The really relevant user input is sometimes a relatively small subset of the full list of recorded events you see now with `C-h l'.) In GNU Emacs 26.3 (build 1, x86_64-w64-mingw32) of 2019-08-29 Repository revision: 96dd0196c28bc36779584e47fffcca433c9309cd Windowing system distributor `Microsoft Corp.', version 10.0.17763 Configured using: `configure --without-dbus --host=x86_64-w64-mingw32 --without-compress-install 'CFLAGS=-O2 -static -g3'' ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#38796: 26.3; `view-lossage': Use a variable for the lossage limit 2019-12-29 17:04 bug#38796: 26.3; `view-lossage': Use a variable for the lossage limit Drew Adams @ 2019-12-29 17:30 ` Eli Zaretskii 2019-12-29 17:34 ` Drew Adams 2020-06-26 21:58 ` Tino Calancha 0 siblings, 2 replies; 18+ messages in thread From: Eli Zaretskii @ 2019-12-29 17:30 UTC (permalink / raw) To: Drew Adams; +Cc: 38796 severity 38796 wishlist thanks > Date: Sun, 29 Dec 2019 09:04:54 -0800 (PST) > From: Drew Adams <drew.adams@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. > > Doing that will give users (and Lisp code) control over the limit. > Having a limit that is hard-coded in C is not user-friendly - it's even > unemacsy. It's a constant size of a vector that records the keys in a cyclical fashion. So 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. Not rocket science, but still. Patches are welcome. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#38796: 26.3; `view-lossage': Use a variable for the lossage limit 2019-12-29 17:30 ` Eli Zaretskii @ 2019-12-29 17:34 ` Drew Adams 2020-06-26 21:58 ` Tino Calancha 1 sibling, 0 replies; 18+ messages in thread From: Drew Adams @ 2019-12-29 17:34 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 38796 > It's a constant size of a vector that records the keys in a cyclical > fashion. So 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. Not rocket science, but still. I see. I hope such an enhancement is made. > Patches are welcome. A C patch will have to come from someone other than me. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#38796: 26.3; `view-lossage': Use a variable for the lossage limit 2019-12-29 17:30 ` Eli Zaretskii 2019-12-29 17:34 ` Drew Adams @ 2020-06-26 21:58 ` Tino Calancha 2020-06-27 8:32 ` Eli Zaretskii 1 sibling, 1 reply; 18+ messages in thread From: Tino Calancha @ 2020-06-26 21:58 UTC (permalink / raw) To: 38796; +Cc: uyennhi.qm, monnier, tino.calancha Eli Zaretskii <eliz@gnu.org> writes: >> From: Drew Adams <drew.adams@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@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". \f * 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@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) ^ permalink raw reply related [flat|nested] 18+ messages in thread
* bug#38796: 26.3; `view-lossage': Use a variable for the lossage limit 2020-06-26 21:58 ` Tino Calancha @ 2020-06-27 8:32 ` Eli Zaretskii 2020-06-28 16:55 ` Tino Calancha 0 siblings, 1 reply; 18+ messages in thread From: Eli Zaretskii @ 2020-06-27 8:32 UTC (permalink / raw) To: Tino Calancha; +Cc: monnier, 38796, uyennhi.qm > From: Tino Calancha <tino.calancha@gmail.com> > Cc: 38796@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>, Stefan Monnier > <monnier@iro.umontreal.ca>,uyennhi.qm@gmail.com, tino.calancha@gmail.com > Date: Fri, 26 Jun 2020 23:58:01 +0200 > > Eli Zaretskii <eliz@gnu.org> writes: > > >> From: Drew Adams <drew.adams@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. 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#38796: 26.3; `view-lossage': Use a variable for the lossage limit 2020-06-27 8:32 ` Eli Zaretskii @ 2020-06-28 16:55 ` Tino Calancha 2020-06-28 18:00 ` Stefan Monnier 0 siblings, 1 reply; 18+ messages in thread From: Tino Calancha @ 2020-06-28 16:55 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Tino Calancha, uyennhi.qm, 38796, monnier On Sat, 27 Jun 2020, Eli Zaretskii wrote: > 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. I totally agree: they are clearly 2 separated features. Thanks for the comments, I will work on them and get back once completed and tested. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#38796: 26.3; `view-lossage': Use a variable for the lossage limit 2020-06-28 16:55 ` Tino Calancha @ 2020-06-28 18:00 ` Stefan Monnier 2020-06-28 20:01 ` Drew Adams 2020-06-28 21:52 ` Tino Calancha 0 siblings, 2 replies; 18+ messages in thread From: Stefan Monnier @ 2020-06-28 18:00 UTC (permalink / raw) To: Tino Calancha; +Cc: uyennhi.qm, 38796 >> 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. > > I totally agree: they are clearly 2 separated features. > > Thanks for the comments, I will work on them and get back once > completed and tested. I agree that disabling should not necessarily be implemented by "abusing" the max-lossage setting. But I don't see any reason to impose a 300 minimum either. I think it's fine to impose a minimum, but it should be dictated by the limits of the code. I'm not saying we should work to push this lower limit down, but just that it should reflect what the code can do safely rather than being an arbitrary number like 300 (I'm pretty sure 100 would be safe as well, since that's what we've used for many years). Stefan ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#38796: 26.3; `view-lossage': Use a variable for the lossage limit 2020-06-28 18:00 ` Stefan Monnier @ 2020-06-28 20:01 ` Drew Adams 2020-06-28 21:52 ` Tino Calancha 1 sibling, 0 replies; 18+ messages in thread From: Drew Adams @ 2020-06-28 20:01 UTC (permalink / raw) To: Stefan Monnier, Tino Calancha; +Cc: uyennhi.qm, 38796 > I agree that disabling should not necessarily be implemented by > "abusing" the max-lossage setting. > > But I don't see any reason to impose a 300 minimum either. I think it's > fine to impose a minimum, but it should be dictated by the limits of the > code. I'm not saying we should work to push this lower limit down, but > just that it should reflect what the code can do safely rather than > being an arbitrary number like 300 (I'm pretty sure 100 would be safe as > well, since that's what we've used for many years). OP here. ++++ +** The new option 'lossage-limit' controls the +maximum number of keystrokes and commands recorded. I don't feel strongly about what's done wrt low values or turning such logging off. As Eli said, the point of the enhancement request is to let users control the max, in particular to be able to make it _larger_ than the default (300). I think I understand why some have argued for a second option for turning logging off. But I don't think it's a great argument (IIUC). I agree with Stefan that disabling should not _necessarily_ be carried out by setting the max limit (e.g. to 0). Not _necessary_, but isn't that logical, from a user point of view? I wouldn't call that "abusing" the max-limit option. 1. In general in Emacs, setting a maximum of zero does (and should) do just what it says: not allow ANY <whatevers>. 2. Having 2 options here goes against Occam, and is likely to lead to some confusion (perhaps even some mistakes, in terms of security?). It'll require the doc of each option to mention the other, in hopes that users will consult both. Iffy if really you see a security problem here. It'll mean that the defcustom for the max one needs to prevent a value as low as zero, in order to avoid misunderstanding. E.g., what would it even mean for a history of length zero that does not, in effect, disable logging? Is the real reason for not letting zero turn off logging a C-implementation reason? What's really wrong, from a user point of view, with doing that? Again, this isn't super important. The request was for users to be able to customize the length, in particular to be able to _increase_ it. But if we allow such length customization, why complicate things by adding another option for getting the natural effect of length zero, i.e., turning logging off? I'm more often in the position of arguing for more, not fewer, options, even when they combine to control something. But this time (until I understand better perhaps), that's not the case. ___ Another possibility is to have, for the same option, a specific value other than zero, to indicate disabling. E.g. `disable' or some other non-number value. But then you have the same confusion, if the numeric value can itself be 0. ___ Although it's not part of this enhancement request, as was mentioned from the outset the main problem with `view-lossage' output (of any length) is the low signal-to-noise ratio. It would really be good to be able to (on the fly) filter out certain kinds of input, in particular kinds of non-keyboard input. Besides on-the-fly filtering, it might be good to have a user variable, to define a preference for (default) filtering. Another nice-to-have would be a way (e.g. filter) to not show the commands long with their keys. For someone who knows the commands associated with keys, they are pretty much noise. And we could perhaps put links on the keys to their commands in the current keymap context. Increasing the logging length is only one possible improvement. In general, we should be looking for ways to help users see what they think is important in a log - ways to show/hide different things. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#38796: 26.3; `view-lossage': Use a variable for the lossage limit 2020-06-28 18:00 ` Stefan Monnier 2020-06-28 20:01 ` Drew Adams @ 2020-06-28 21:52 ` Tino Calancha 2020-06-29 0:05 ` Drew Adams 2020-08-22 21:24 ` Lars Ingebrigtsen 1 sibling, 2 replies; 18+ messages in thread From: Tino Calancha @ 2020-06-28 21:52 UTC (permalink / raw) To: Stefan Monnier; +Cc: uyennhi.qm, 38796 On Sun, 28 Jun 2020, Drew Adams wrote: > OP here. I was waiting for you. Welcome!! :-) Stefan Monnier <monnier@iro.umontreal.ca> writes: > I agree that disabling should not necessarily be implemented by > "abusing" the max-lossage setting. Yeah, it was appealing to me at first look: the code was offering another feature for free! After your comments I rethink about it and become less excited: it always posible that we might be hit back in other parts of the code if they assume a 'sensible large enough' value. > But I don't see any reason to impose a 300 minimum either. I think it's > fine to impose a minimum, but it should be dictated by the limits of the > code. I'm not saying we should work to push this lower limit down, but > just that it should reflect what the code can do safely rather than > being an arbitrary number like 300 (I'm pretty sure 100 would be safe as > well, since that's what we've used for many years). Sure, 100 must be absolutely fine. I have updated the branch 'bug#38796-lossage-limit'. Now, the limit is not exposed to Lisp anymore. The function to update it now is a command 'update-lossage-size' We also have a 'lossage-size' function to retrieve its current value. --8<-----------------------------cut here---------------start------------->8--- commit c86d060b460bdaecdd71d297fa84012fcd397d66 Author: Tino Calancha <tino.calancha@gmail.com> Date: Sun Jun 28 23:30:07 2020 +0200 Do not expose the size of recent_keys to Lisp That prevents from unintentional crashes if the users modify the variable with setq or if they let-bind it. Users can still safely modify the lossage limit with the command `update-lossage-size'. For convenience, add a function `lossage-size' to return the current limit. * src/keyboard.c (lossage_limit): Do not expose it to Lisp; make it a static variable. Keep the current Emacs default (300); accept only values >= 100. (lossage-size): New function; it returns the current size of recent_keys. (update-lossage-size): Rename it from update-lossage-limit (all callers updated); make it a command. * doc/emacs/help.texi (Misc Help) * etc/NEWS * lisp/cus-start.el * lisp/edmacro.el * lisp/help.el: Update all references with the new name. * test/src/keyboard-tests.el (keyboard-lossage-limit): Amend the test. diff --git a/doc/emacs/help.texi b/doc/emacs/help.texi index e2c0dc6802..26cdf161d5 100644 --- a/doc/emacs/help.texi +++ b/doc/emacs/help.texi @@ -563,12 +563,14 @@ Misc Help @kindex C-h l @findex view-lossage -@vindex lossage-limit +@findex update-lossage-size +@findex lossage-size 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 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}. +stores the last 300 events; if you wish, you can change this number with +the command @code{update-lossage-size}. The function @code{lossage-size} +returns the current value for the 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. diff --git a/etc/NEWS b/etc/NEWS index da204c5825..5fc4cb4d87 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -71,8 +71,9 @@ 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. +** The new command 'update-lossage-size' allow users to set the maximum +number of keystrokes and commands recorded. The new function 'lossage-size' +returns the current value of this limit ** 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..6632687da4 100644 --- a/lisp/cus-start.el +++ b/lisp/cus-start.el @@ -352,8 +352,6 @@ 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 089fea17ef..52cbf5b5a6 100644 --- a/lisp/edmacro.el +++ b/lisp/edmacro.el @@ -35,7 +35,7 @@ ;; * `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 `lossage-limit' most recent +;; * `C-h l' (view-lossage), to edit the 300 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 `lossage-limit' +keyboard macro, `\\[view-lossage]' to edit the last 300 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 9e3d0922d0..01ee739a16 100644 --- a/lisp/help.el +++ b/lisp/help.el @@ -459,7 +459,7 @@ view-lossage "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. +See `update-lossage-size' 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 a4d27c94fb..29b157700c 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -103,6 +103,7 @@ static KBOARD *all_kboards; /* True in the single-kboard state, false in the any-kboard state. */ static bool single_kboard; +/* Minimum allowed size of the recent_keys vector */ #define MIN_NUM_RECENT_KEYS (100) /* Index for storing next element into recent_keys. */ @@ -111,6 +112,9 @@ static int recent_keys_index; /* Total number of elements stored into recent_keys. */ static int total_keys; +/* Size of the recent_keys vector */ +static int lossage_limit = 3 * MIN_NUM_RECENT_KEYS; + /* This vector holds the last lossage_limit keystrokes. */ static Lisp_Object recent_keys; @@ -10411,6 +10415,15 @@ If CHECK-TIMERS is non-nil, timers that are ready to run will do so. */) ? Qt : Qnil); } +DEFUN ("lossage-size", Flossage_size, Slossage_size, 0, 0, 0, + doc: /* Return the maximum number of saved keystrokes. +These are the records shown by `view-lossage'. */ ) + (void) +{ + return make_fixnum(lossage_limit); +} + +/* Reallocate recent_keys copying the keystrokes in the right order */ static void update_recent_keys (int new_size, int kept_keys) { @@ -10433,10 +10446,10 @@ update_recent_keys (int new_size, int kept_keys) } -/* Reallocate recent_keys copying the keystrokes in the right order */ -DEFUN ("update-lossage-limit", Fupdate_lossage_limit, - Supdate_lossage_limit, 1, 1, 0, +DEFUN ("update-lossage-size", Fupdate_lossage_size, + Supdate_lossage_size, 1, 1, "nnew-size: ", doc: /* Update the maximum number of saved keystrokes to ARG. +These are the records shown by `view-lossage'. usage: (update-lossage-limit ARG) */) (Lisp_Object arg) { @@ -11736,22 +11749,6 @@ syms_of_keyboard (void) staticpro (&modifier_symbols); } - 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); @@ -11802,7 +11799,8 @@ That takes care of both, the variable and the internal vector.*/); defsubr (&Srecursive_edit); defsubr (&Sinternal_track_mouse); defsubr (&Sinput_pending_p); - defsubr (&Supdate_lossage_limit); + defsubr (&Slossage_size); + defsubr (&Supdate_lossage_size); 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 017d239246..8f2cfe4adc 100644 --- a/test/src/keyboard-tests.el +++ b/test/src/keyboard-tests.el @@ -32,15 +32,15 @@ (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)))) +(ert-deftest keyboard-lossage-size () + "Test `update-lossage-size'." + (dolist (val (list 100 300 400 400 500 1000 700 300)) + (update-lossage-size val) + (should (= val (lossage-size)))) + (let ((current-size (lossage-size))) + (should-error (update-lossage-size 5)) + (should-error (update-lossage-size "200")) + (should (= (lossage-size) current-size)))) (provide 'keyboard-tests) --8<-----------------------------cut here---------------end--------------->8--- In GNU Emacs 28.0.50 (build 25, x86_64-pc-linux-gnu, GTK+ Version 3.24.5, cairo version 1.16.0) of 2020-06-28 built on calancha-pc.dy.bbexcite.jp Repository revision: e13fd1fc373b35f510b70998bfddbb7af5989912 Repository branch: bug#38796-lossage-limit Windowing system distributor 'The X.Org Foundation', version 11.0.12004000 System Description: Debian GNU/Linux 10 (buster) ^ permalink raw reply related [flat|nested] 18+ messages in thread
* bug#38796: 26.3; `view-lossage': Use a variable for the lossage limit 2020-06-28 21:52 ` Tino Calancha @ 2020-06-29 0:05 ` Drew Adams 2020-08-22 21:24 ` Lars Ingebrigtsen 1 sibling, 0 replies; 18+ messages in thread From: Drew Adams @ 2020-06-29 0:05 UTC (permalink / raw) To: Tino Calancha, Stefan Monnier; +Cc: uyennhi.qm, 38796 > > OP here. > I was waiting for you. Welcome!! :-) Sure, but the rest of your reply is a reply to Stefan's message, not mine. And the `>' quoting suggests you're quoting me in the rest of your message too, but you're quoting him. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#38796: 26.3; `view-lossage': Use a variable for the lossage limit 2020-06-28 21:52 ` Tino Calancha 2020-06-29 0:05 ` Drew Adams @ 2020-08-22 21:24 ` Lars Ingebrigtsen 2020-08-22 22:54 ` Drew Adams 2020-08-27 21:28 ` Tino Calancha 1 sibling, 2 replies; 18+ messages in thread From: Lars Ingebrigtsen @ 2020-08-22 21:24 UTC (permalink / raw) To: Tino Calancha; +Cc: Stefan Monnier, 38796, uyennhi.qm Tino Calancha <tino.calancha@gmail.com> writes: >> I agree that disabling should not necessarily be implemented by >> "abusing" the max-lossage setting. > Yeah, it was appealing to me at first look: the code was offering > another feature for free! > After your comments I rethink about it and become less excited: it > always posible that we might be hit back in other parts of the code > if they assume a 'sensible large enough' value. I thought it sounded quite natural to use the same mechanism to switch lossage logging off. `(update-lossage-size 0)' seems like a natural thing to do to have no lossage. > Users can still safely modify the lossage limit with the > command `update-lossage-size'. For convenience, add > a function `lossage-size' to return the current limit. Hm... other functions use the convention of having a zero-parameter function return the data, and having a parameter to set it. So (lossage-size 500) could set it and (lossage-size) could return the current size? Anyway, it doesn't look like you've merged the branch? I think everybody was in favour of adding this feature. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#38796: 26.3; `view-lossage': Use a variable for the lossage limit 2020-08-22 21:24 ` Lars Ingebrigtsen @ 2020-08-22 22:54 ` Drew Adams 2020-08-27 21:28 ` Tino Calancha 1 sibling, 0 replies; 18+ messages in thread From: Drew Adams @ 2020-08-22 22:54 UTC (permalink / raw) To: Lars Ingebrigtsen, Tino Calancha; +Cc: Stefan Monnier, 38796, uyennhi.qm > I thought it sounded quite natural to use the same mechanism to switch > lossage logging off. `(update-lossage-size 0)' seems like a natural > thing to do to have no lossage. FWIW, I said the same thing: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=38796#28 It's not unusual or weird for a zero-valued size parameter to have such an effect. > > Users can still safely modify the lossage limit with the > > command `update-lossage-size'. For convenience, add > > a function `lossage-size' to return the current limit. > > Hm... other functions use the convention of having a zero-parameter > function return the data, and having a parameter to set it. So > (lossage-size 500) could set it and (lossage-size) could return the > current size? FWIW, most `set-*' functions are commands. And a command is what we should shoot for here. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#38796: 26.3; `view-lossage': Use a variable for the lossage limit 2020-08-22 21:24 ` Lars Ingebrigtsen 2020-08-22 22:54 ` Drew Adams @ 2020-08-27 21:28 ` Tino Calancha 2020-08-28 6:04 ` Eli Zaretskii 1 sibling, 1 reply; 18+ messages in thread From: Tino Calancha @ 2020-08-27 21:28 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: uyennhi.qm, stefankangas, 38796, Stefan Monnier Lars Ingebrigtsen <larsi@gnus.org> writes: > Tino Calancha <tino.calancha@gmail.com> writes: > >>> I agree that disabling should not necessarily be implemented by >>> "abusing" the max-lossage setting. >> Yeah, it was appealing to me at first look: the code was offering >> another feature for free! >> After your comments I rethink about it and become less excited: it >> always posible that we might be hit back in other parts of the code >> if they assume a 'sensible large enough' value. > > I thought it sounded quite natural to use the same mechanism to switch > lossage logging off. `(update-lossage-size 0)' seems like a natural > thing to do to have no lossage. > Anyway, it doesn't look like you've merged the branch? I think > everybody was in favour of adding this feature. People agree with that. The friction arises because I kept the current implementation, which can be seen as abusing on it, and/or risky. [ I have neither plan nor time to redo the current implementation. ] >> Users can still safely modify the lossage limit with the >> command `update-lossage-size'. For convenience, add >> a function `lossage-size' to return the current limit. > > Hm... other functions use the convention of having a zero-parameter > function return the data, and having a parameter to set it. So > (lossage-size 500) could set it and (lossage-size) could return the > current size? I have amended the patch to do this. Thank you. 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#38796: 26.3; `view-lossage': Use a variable for the lossage limit 2020-08-27 21:28 ` Tino Calancha @ 2020-08-28 6:04 ` Eli Zaretskii 2020-09-04 9:31 ` Tino Calancha 0 siblings, 1 reply; 18+ messages in thread From: Eli Zaretskii @ 2020-08-28 6:04 UTC (permalink / raw) To: Tino Calancha; +Cc: uyennhi.qm, larsi, stefankangas, 38796, monnier > From: Tino Calancha <tino.calancha@gmail.com> > Cc: Eli Zaretskii <eliz@gnu.org>, Stefan Monnier > <monnier@iro.umontreal.ca>, 38796@debbugs.gnu.org, uyennhi.qm@gmail.com, > stefankangas@gmail.com > Date: Thu, 27 Aug 2020 23:28:03 +0200 > > People agree with that. The friction arises because I kept the current > implementation, which can be seen as abusing on it, and/or risky. > [ I have neither plan nor time to redo the current implementation. ] > >> Users can still safely modify the lossage limit with the > >> command `update-lossage-size'. For convenience, add > >> a function `lossage-size' to return the current limit. > > > > Hm... other functions use the convention of having a zero-parameter > > function return the data, and having a parameter to set it. So > > (lossage-size 500) could set it and (lossage-size) could return the > > current size? > I have amended the patch to do this. Thank you. > > 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? ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#38796: 26.3; `view-lossage': Use a variable for the lossage limit 2020-08-28 6:04 ` Eli Zaretskii @ 2020-09-04 9:31 ` Tino Calancha 2020-09-04 12:07 ` Eli Zaretskii 0 siblings, 1 reply; 18+ messages in thread From: Tino Calancha @ 2020-09-04 9:31 UTC (permalink / raw) To: Eli Zaretskii; +Cc: larsi, monnier, uyennhi.qm, 38796, stefankangas Eli Zaretskii <eliz@gnu.org> 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): --8<-----------------------------cut here---------------start------------->8--- diff --git a/doc/emacs/help.texi b/doc/emacs/help.texi index 06ad5a583d..34a354891b 100644 --- a/doc/emacs/help.texi +++ b/doc/emacs/help.texi @@ -573,10 +573,13 @@ Misc Help @kindex C-h l @findex view-lossage +@findex lossage-size 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 command @code{lossage-size}. +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 964b626d7b..8bb3527d85 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -85,6 +85,10 @@ useful on systems such as FreeBSD which ships only with "etc/termcap". \f * Changes in Emacs 28.1 ++++ +** 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 +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/comint.el b/lisp/comint.el index be0e32b9e0..afc8395b36 100644 --- a/lisp/comint.el +++ b/lisp/comint.el @@ -2393,7 +2393,7 @@ comint-send-invisible Then send it to the process running in the current buffer. The string is sent using `comint-input-sender'. Security bug: your string can still be temporarily recovered with -\\[view-lossage]; `clear-this-command-keys' can fix that." +\\[view-lossage]; `clear-this-command-keys' and `lossage-size' can fix that." (interactive "P") ; Defeat snooping via C-x ESC ESC (let ((proc (get-buffer-process (current-buffer))) (prefix diff --git a/lisp/edmacro.el b/lisp/edmacro.el index 71474c0289..52cbf5b5a6 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 300 most recent +;; keystrokes and install them as the "current" macro. ;; ;; * any key sequence whose definition is a keyboard macro. ;; diff --git a/lisp/help.el b/lisp/help.el index 1b0149616f..d3b0e02f08 100644 --- a/lisp/help.el +++ b/lisp/help.el @@ -455,9 +455,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-size' to update the number of recorded keystrokes. To record all your input, use `open-dribble-file'." (interactive) diff --git a/lisp/term.el b/lisp/term.el index e77c2c1331..e39d03e8cd 100644 --- a/lisp/term.el +++ b/lisp/term.el @@ -2236,7 +2236,8 @@ term-read-noecho Note that the keystrokes comprising the text can still be recovered \(temporarily) with \\[view-lossage]. This may be a security bug for some -applications." +applications. See `clear-this-command-keys' and `lossage-size' for ways +to fix that." (declare (obsolete read-passwd "27.1")) (let ((ans "") (c 0) diff --git a/src/keyboard.c b/src/keyboard.c index 5fa58abce1..bf8d75b602 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -103,15 +103,16 @@ Copyright (C) 1985-1989, 1993-1997, 1999-2020 Free Software Foundation, /* True in the single-kboard state, false in the any-kboard state. */ static bool single_kboard; -#define NUM_RECENT_KEYS (300) - /* Index for storing next element into recent_keys. */ 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. */ +/* Size of the recent_keys vector */ +static int lossage_limit = 300; + +/* 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 +295,7 @@ #define GROW_RAW_KEYBUF \ /* 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 +1423,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 +3250,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 +3309,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 +3328,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 +10412,62 @@ DEFUN ("input-pending-p", Finput_pending_p, Sinput_pending_p, 0, 1, 0, ? Qt : Qnil); } +/* Reallocate recent_keys copying the keystrokes in the right order */ +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; + +} + +DEFUN ("lossage-size", Flossage_size, Slossage_size, 0, 1, + "(list (read-number \"new-size: \" (lossage-size)))", + doc: /* Return the maximum number of saved keystrokes. +Called with ARG, then set this number to ARG. + +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. +usage: (lossage-size &optional ARG) */) + (Lisp_Object arg) +{ + if (NILP(arg)) + return make_fixnum(lossage_limit == 1 ? 0 : lossage_limit); + + if (!FIXNATP (arg)) + user_error ("Value must be a non-negative integer"); + int osize = ASIZE (recent_keys); + eassert (lossage_limit == osize); + int new_size = XFIXNAT (arg); + + if (new_size == osize) + return Qnil; + /* 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; + + 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 +10477,21 @@ DEFUN ("recent-keys", Frecent_keys, Srecent_keys, 0, 1, 0, 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,7 +10589,7 @@ DEFUN ("this-single-command-raw-keys", Fthis_single_command_raw_keys, 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 300 input events, unless optional arg +Also clear the record of the last input events, unless optional arg KEEP-RECORD is non-nil. */) (Lisp_Object keep_record) { @@ -11686,7 +11744,7 @@ syms_of_keyboard (void) staticpro (&modifier_symbols); } - recent_keys = make_nil_vector (NUM_RECENT_KEYS); + recent_keys = make_nil_vector (lossage_limit); staticpro (&recent_keys); this_command_keys = make_nil_vector (40); @@ -11736,6 +11794,7 @@ syms_of_keyboard (void) defsubr (&Srecursive_edit); defsubr (&Sinternal_track_mouse); defsubr (&Sinput_pending_p); + defsubr (&Slossage_size); 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..d4ac4a518d 100644 --- a/test/src/keyboard-tests.el +++ b/test/src/keyboard-tests.el @@ -32,5 +32,23 @@ keyboard-unread-command-events (read-event nil nil 2)) ?\C-b))) +(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)))) + (let ((current-size (lossage-size))) + ;; Set lossage size to 1 and 0 are equivalent + (lossage-size 0) + (should (zerop (lossage-size))) + (lossage-size 1) + (should (zerop (lossage-size))) + (lossage-size current-size) + ;; Argument must be a natural number + (should-error (lossage-size -5)) + (should-error (lossage-size "200")) + (should (= (lossage-size) current-size)))) + + (provide 'keyboard-tests) ;;; keyboard-tests.el ends here --8<-----------------------------cut here---------------end--------------->8--- ^ permalink raw reply related [flat|nested] 18+ messages in thread
* bug#38796: 26.3; `view-lossage': Use a variable for the lossage limit 2020-09-04 9:31 ` Tino Calancha @ 2020-09-04 12:07 ` Eli Zaretskii 2020-09-12 12:29 ` Tino Calancha 0 siblings, 1 reply; 18+ messages in thread From: Eli Zaretskii @ 2020-09-04 12:07 UTC (permalink / raw) To: Tino Calancha; +Cc: larsi, monnier, uyennhi.qm, 38796, stefankangas > From: Tino Calancha <tino.calancha@gmail.com> > 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 <eliz@gnu.org> 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#38796: 26.3; `view-lossage': Use a variable for the lossage limit 2020-09-04 12:07 ` Eli Zaretskii @ 2020-09-12 12:29 ` Tino Calancha 2020-09-17 14:35 ` Tino Calancha 0 siblings, 1 reply; 18+ messages in thread From: Tino Calancha @ 2020-09-12 12:29 UTC (permalink / raw) To: Eli Zaretskii; +Cc: larsi, stefankangas, monnier, 38796, uyennhi.qm Eli Zaretskii <eliz@gnu.org> writes: Thank you for the review Eli. I have fixed all your comments. Regarding the feature to disable the recording of keystrokes, I am not happy with my hackish implementation either: I have dropped it. I prefer just to keep my patch within the scope of the original proposal: i.e., be able to set the lossage limit with Lisp (within well tested limits). I am glad to push the following patch next week if there are no complaints. --8<-----------------------------cut here---------------start------------->8--- commit bee193a5d6cd7889183cf811eb001e0071052724 Author: Tino Calancha <ccalancha@suse.com> Date: Sat Sep 12 14:14:45 2020 +0200 Give Lisp control on the lossage size Add a command 'lossage-size' to set the maximum number or recorded keystrokes (Bug#38796). * src/keyboard.c (lossage_limit): Static variable with the current lossage size limit. (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). (lossage-size): New command. (update_recent_keys): Helper function. (command_loop_1) (record_char) (recent-keys) (syms_of_keyboard): Use lossage_limit as the vector size. * lisp/help.el (view-lossage): Mention the new command in the docstring. * etc/NEWS (Changes in Emacs 28.1): Announce this change. * doc/emacs/help.texi (Misc Help): Update manual. * test/src/keyboard-tests.el (keyboard-lossage-size): Add test. diff --git a/doc/emacs/help.texi b/doc/emacs/help.texi index 06ad5a583d..232b611f41 100644 --- a/doc/emacs/help.texi +++ b/doc/emacs/help.texi @@ -573,10 +573,13 @@ Misc Help @kindex C-h l @findex view-lossage +@findex lossage-size 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 keystrokes; if you wish, you can change this number with +the command @code{lossage-size}. +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 8ff62b6dc0..6fd23422cf 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -85,6 +85,10 @@ useful on systems such as FreeBSD which ships only with "etc/termcap". \f * Changes in Emacs 28.1 ++++ +** The new command 'lossage-size' allow users to set the maximum +number of keystrokes and commands recorded. + +++ *** Emacs now defaults to UTF-8 instead of ISO-8859-1. This is only for the default, where the user has set no LANG (or diff --git a/lisp/help.el b/lisp/help.el index 729684af6b..edef78d207 100644 --- a/lisp/help.el +++ b/lisp/help.el @@ -458,6 +458,7 @@ view-lossage "Display last few input keystrokes and the commands run. For convenience this uses the same format as `edit-last-kbd-macro'. +See `lossage-size' 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 590d183c4c..c0a41e6c4c 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -103,7 +103,8 @@ Copyright (C) 1985-1989, 1993-1997, 1999-2020 Free Software Foundation, /* True in the single-kboard state, false in the any-kboard state. */ static bool single_kboard; -#define NUM_RECENT_KEYS (300) +/* Minimum allowed size of the recent_keys vector. */ +#define MIN_NUM_RECENT_KEYS (100) /* Index for storing next element into recent_keys. */ static int recent_keys_index; @@ -111,7 +112,10 @@ #define NUM_RECENT_KEYS (300) /* Total number of elements stored into recent_keys. */ static int total_keys; -/* This vector holds the last NUM_RECENT_KEYS keystrokes. */ +/* Size of the recent_keys vector. */ +static int lossage_limit = 3 * MIN_NUM_RECENT_KEYS; + +/* This vector holds the last lossage_limit keystrokes. */ static Lisp_Object recent_keys; /* Vector holding the key sequence that invoked the current command. @@ -1421,10 +1425,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 +3252,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 +3311,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 +3330,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 +10414,62 @@ DEFUN ("input-pending-p", Finput_pending_p, Sinput_pending_p, 0, 1, 0, ? Qt : Qnil); } +/* Reallocate recent_keys copying the keystrokes in the right order */ +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; + +} + +DEFUN ("lossage-size", Flossage_size, Slossage_size, 0, 1, + "(list (read-number \"new-size: \" (lossage-size)))", + doc: /* Return the maximum number of saved keystrokes. +Called with ARG, then set this limit to ARG and return it. + +The saved keystrokes are the records shown by `view-lossage'. */) + (Lisp_Object arg) +{ + if (NILP(arg)) + return make_fixnum (lossage_limit); + + 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 make_fixnum (lossage_limit); + + 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 make_fixnum (lossage_limit); +} + 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 +10479,21 @@ DEFUN ("recent-keys", Frecent_keys, Srecent_keys, 0, 1, 0, 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); @@ -11686,7 +11746,7 @@ syms_of_keyboard (void) staticpro (&modifier_symbols); } - recent_keys = make_nil_vector (NUM_RECENT_KEYS); + recent_keys = make_nil_vector (lossage_limit); staticpro (&recent_keys); this_command_keys = make_nil_vector (40); @@ -11736,6 +11796,7 @@ syms_of_keyboard (void) defsubr (&Srecursive_edit); defsubr (&Sinternal_track_mouse); defsubr (&Sinput_pending_p); + defsubr (&Slossage_size); 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..970a53555f 100644 --- a/test/src/keyboard-tests.el +++ b/test/src/keyboard-tests.el @@ -32,5 +32,20 @@ keyboard-unread-command-events (read-event nil nil 2)) ?\C-b))) +(ert-deftest keyboard-lossage-size () + "Test `lossage-size'." + (let ((min-value 100) + (lossage-orig (lossage-size))) + (dolist (factor (list 1 3 4 5 10 7 3)) + (let ((new-lossage (* factor min-value))) + (should (= new-lossage (lossage-size new-lossage))))) + ;; Wrong type + (should-error (lossage-size -5)) + (should-error (lossage-size "200")) + ;; Less that minimum value + (should-error (lossage-size (1- min-value))) + (should (= lossage-orig (lossage-size lossage-orig))))) + + (provide 'keyboard-tests) ;;; keyboard-tests.el ends here --8<-----------------------------cut here---------------end--------------->8--- In GNU Emacs 28.0.50 (build 6, x86_64-pc-linux-gnu, GTK+ Version 3.24.22, cairo version 1.16.0) of 2020-09-12 built on localhost.example.com Repository revision: 4c3f3bf25623c936ca07249203147ae0332d64ed Repository branch: master ^ permalink raw reply related [flat|nested] 18+ messages in thread
* bug#38796: 26.3; `view-lossage': Use a variable for the lossage limit 2020-09-12 12:29 ` Tino Calancha @ 2020-09-17 14:35 ` Tino Calancha 0 siblings, 0 replies; 18+ messages in thread From: Tino Calancha @ 2020-09-17 14:35 UTC (permalink / raw) To: 38796-done; +Cc: uyennhi.qm, larsi, stefankangas, 38796, monnier Tino Calancha <tino.calancha@gmail.com> writes: > I am glad to push the following patch next week if there are no > complaints. Pushed into master branch as commit 'Give Lisp control on the lossage size' (23a3333b3ef0768f48f64f382ee899050b6103be) ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2020-09-17 14:35 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-12-29 17:04 bug#38796: 26.3; `view-lossage': Use a variable for the lossage limit Drew Adams 2019-12-29 17:30 ` Eli Zaretskii 2019-12-29 17:34 ` Drew Adams 2020-06-26 21:58 ` Tino Calancha 2020-06-27 8:32 ` Eli Zaretskii 2020-06-28 16:55 ` Tino Calancha 2020-06-28 18:00 ` Stefan Monnier 2020-06-28 20:01 ` Drew Adams 2020-06-28 21:52 ` Tino Calancha 2020-06-29 0:05 ` Drew Adams 2020-08-22 21:24 ` Lars Ingebrigtsen 2020-08-22 22:54 ` Drew Adams 2020-08-27 21:28 ` Tino Calancha 2020-08-28 6:04 ` Eli Zaretskii 2020-09-04 9:31 ` Tino Calancha 2020-09-04 12:07 ` Eli Zaretskii 2020-09-12 12:29 ` Tino Calancha 2020-09-17 14:35 ` Tino Calancha
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).