Re: Please add a lossage-limit option for `view-lossage'

[Stefan Monnier]

Hi,

> I took this task as a goal in my first attempt to write code to Emacs C core.

Very good choice!

> In the Lisp reference documentation, I was unable to find a way to do
> that.  I thought that if that mechanism existed it’d be important enough
> to be documented.  I couldn’t find it in a quick look around the code
> neither.  How is the interception of change of variables from the Lisp
> world canonically handled by Emacs C source code?

Indeed, there's no such canonical thing.

One way this kind of problem is solved sometimes is by not exporting the
variable to Lisp, and instead only providing access to the variable via
functions to read and/or set it.

But here's how I'd recommend you solve this specific case:

1- consolidate the (apprently 3) places in the current code that add an
   element to the vector into its own function.  That's a good change in
   itself, will make the code shorter (by reducing duplication) and
   more readable.

2- Replace the NUM_RECENT_KEYS constant with a num_recent_keys variable
   (tho maybe we can just replace the var with `ASIZE (recent_keys)`
   instead!).

3- Add a new Lisp variable which we could call `Vnum_recent_keys`, tho
   maybe I'd rather make it use the `recent-key` prefix, so it could be
   `Vrecent_key_size`?
   [ Notice that, by convention, Lisp variables in the C world are
     prefixed with a `V`.  ]

4- Synchronize this new var with `num_recent_keys` at just one place:
   inside the function introduced at step 1.  This means that calling
   `recent-keys` just after setting the var will not reflect the new
   value of the variable, but nobody cares about that, I think.


        Stefan


> Footnotes: 
>
> [1]  This is the requested user option.
>
> [2] Actually creating another vector of the right size and copying the
>     data over as needed.
>
>
> From f87911cf8f6d12046f25e2964bbbdbcd3ca0581d Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Bruno=20F=C3=A9lix=20Rezende=20Ribeiro?= <address@hidden>
> Date: Mon, 6 Apr 2020 08:30:16 -0300
> Subject: [PATCH] =?UTF-8?q?Transform=20C=20constant=20=E2=80=98NUM=5FRECEN?=
>  =?UTF-8?q?T=5FKEYS=E2=80=99=20into=20=E2=80=98num-recent-keys=E2=80=99=20?=
>  =?UTF-8?q?Lisp=20variable?=
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> * src/keyboard.c: remove NUM_RECENT_KEYS constant.
>   (ensure_recent_keys_size): new function.
>   (command_loop_1, record_char, recent-keys): add calls to
>   ‘ensure_recent_key_size’.
>   (syms_of_keyboard): add ‘num-recent-keys’ Lisp integer variable.
> * lisp/cus-start.el: add ‘num-recent-keys’.
> ---
>  lisp/cus-start.el |  1 +
>  src/keyboard.c    | 62 
> ++++++++++++++++++++++++++++++++++++++-----------------
>  2 files changed, 44 insertions(+), 19 deletions(-)
>
> diff --git a/lisp/cus-start.el b/lisp/cus-start.el
> index dff4d9a..001f79b 100644
> --- a/lisp/cus-start.el
> +++ b/lisp/cus-start.el
> @@ -353,6 +353,7 @@ minibuffer-prompt-properties--setter
>            (indent-tabs-mode indent boolean)
>            ;; keyboard.c
>            (meta-prefix-char keyboard character)
> +             (num-recent-keys keyboard integer)
>            (auto-save-interval auto-save integer)
>               (auto-save-no-message auto-save boolean "27.1")
>            (auto-save-timeout auto-save (choice (const :tag "off" nil)
> diff --git a/src/keyboard.c b/src/keyboard.c
> index 9ce168c..cbbb572 100644
> --- a/src/keyboard.c
> +++ b/src/keyboard.c
> @@ -103,15 +103,13 @@ 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.  */
> +/* This vector holds the last num_recent_keys keystrokes.  */
>  static Lisp_Object recent_keys;
>  
>  /* Vector holding the key sequence that invoked the current command.
> @@ -371,6 +369,7 @@ #define READABLE_EVENTS_IGNORE_SQUEEZABLES        (1 << 2)
>  static void deliver_user_signal (int);
>  static char *find_user_signal_name (int);
>  static void store_user_signal_events (void);
> +static void ensure_recent_keys_size (void);
>  
>  /* Advance or retreat a buffered input event pointer.  */
>  
> @@ -448,6 +447,20 @@ kset_system_key_syms (struct kboard *kb, Lisp_Object val)
>  }
>  
>
> +void
> +ensure_recent_keys_size (void)
> +{
> +  if (ASIZE (recent_keys) != num_recent_keys)
> +    {
> +      Lisp_Object new_recent_keys = make_nil_vector (num_recent_keys);
> +      memcpy (XVECTOR (new_recent_keys)->contents, XVECTOR 
> (recent_keys)->contents,
> +              min (ASIZE (recent_keys), num_recent_keys)
> +              * sizeof *XVECTOR (recent_keys)->contents);
> +      recent_keys = new_recent_keys;
> +    }
> +}
> +
> +
>  static bool
>  echo_keystrokes_p (void)
>  {
> @@ -1421,10 +1434,11 @@ command_loop_1 (void)
>        /* Execute the command.  */
>  
>        {
> -     total_keys += total_keys < NUM_RECENT_KEYS;
> +        ensure_recent_keys_size ();
> +     total_keys += total_keys < num_recent_keys;
>       ASET (recent_keys, recent_keys_index,
>             Fcons (Qnil, cmd));
> -     if (++recent_keys_index >= NUM_RECENT_KEYS)
> +     if (++recent_keys_index >= num_recent_keys)
>         recent_keys_index = 0;
>        }
>        Vthis_command = cmd;
> @@ -3249,16 +3263,18 @@ record_char (Lisp_Object c)
>        Lisp_Object ev1, ev2, ev3;
>        int ix1, ix2, ix3;
>  
> +      ensure_recent_keys_size ();
> +
>        if ((ix1 = recent_keys_index - 1) < 0)
> -     ix1 = NUM_RECENT_KEYS - 1;
> +     ix1 = num_recent_keys - 1;
>        ev1 = AREF (recent_keys, ix1);
>  
>        if ((ix2 = ix1 - 1) < 0)
> -     ix2 = NUM_RECENT_KEYS - 1;
> +     ix2 = num_recent_keys - 1;
>        ev2 = AREF (recent_keys, ix2);
>  
>        if ((ix3 = ix2 - 1) < 0)
> -     ix3 = NUM_RECENT_KEYS - 1;
> +     ix3 = num_recent_keys - 1;
>        ev3 = AREF (recent_keys, ix3);
>  
>        if (EQ (XCAR (c), Qhelp_echo))
> @@ -3309,12 +3325,13 @@ record_char (Lisp_Object c)
>      {
>        if (!recorded)
>       {
> -       total_keys += total_keys < NUM_RECENT_KEYS;
> +          ensure_recent_keys_size ();
> +       total_keys += total_keys < num_recent_keys;
>         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 >= num_recent_keys)
>           recent_keys_index = 0;
>       }
>        else if (recorded < 0)
> @@ -3328,10 +3345,11 @@ record_char (Lisp_Object c)
>  
>         while (recorded++ < 0 && total_keys > 0)
>           {
> -           if (total_keys < NUM_RECENT_KEYS)
> +              ensure_recent_keys_size ();
> +           if (total_keys < num_recent_keys)
>               total_keys--;
>             if (--recent_keys_index < 0)
> -             recent_keys_index = NUM_RECENT_KEYS - 1;
> +             recent_keys_index = num_recent_keys - 1;
>             ASET (recent_keys, recent_keys_index, Qnil);
>           }
>       }
> @@ -10420,22 +10438,24 @@ DEFUN ("recent-keys", Frecent_keys, Srecent_keys, 
> 0, 1, 0,
>  {
>    bool cmds = !NILP (include_cmds);
>  
> +  ensure_recent_keys_size ();
> +
>    if (!total_keys
> -      || (cmds && total_keys < NUM_RECENT_KEYS))
> +      || (cmds && total_keys < num_recent_keys))
>      return Fvector (total_keys,
>                   XVECTOR (recent_keys)->contents);
>    else
>      {
>        Lisp_Object es = Qnil;
> -      int i = (total_keys < NUM_RECENT_KEYS
> +      int i = (total_keys < num_recent_keys
>              ? 0 : recent_keys_index);
> -      eassert (recent_keys_index < NUM_RECENT_KEYS);
> +      eassert (recent_keys_index < num_recent_keys);
>        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 >= num_recent_keys)
>           i = 0;
>       } while (i != recent_keys_index);
>        es = Fnreverse (es);
> @@ -11690,9 +11710,6 @@ syms_of_keyboard (void)
>      staticpro (&modifier_symbols);
>    }
>  
> -  recent_keys = make_nil_vector (NUM_RECENT_KEYS);
> -  staticpro (&recent_keys);
> -
>    this_command_keys = make_nil_vector (40);
>    staticpro (&this_command_keys);
>  
> @@ -11858,6 +11875,13 @@ syms_of_keyboard (void)
>  result of looking up the original command in the active keymaps.  */);
>    Vthis_original_command = Qnil;
>  
> +  DEFVAR_INT ("num-recent-keys", num_recent_keys,
> +           doc: /* Number of input events to consider. */);
> +  num_recent_keys = 300;
> +
> +  recent_keys = make_nil_vector (num_recent_keys);
> +  staticpro (&recent_keys);
> +
>    DEFVAR_INT ("auto-save-interval", auto_save_interval,
>             doc: /* Number of input events between auto-saves.
>  Zero means disable autosaving due to number of characters typed.  */);
> -- 
> 2.7.4