refactor(input): ungetchars() usage #40685

Repurpose `ins_char_typebuf` to make its usage more obvious.
Rename it and update docs.

src/nvim/getchar.c

@@ -1,5 +1,36 @@
-// getchar.c: Code related to getting a character from the user or a script
-// file, manipulations with redo buffer and stuff buffer.
+// getchar.c: the input engine.
+// - Get a character from the user, a script file, or the keybufs described below.
+// - Apply mappings and abbreviations to typed keys (the :map tables live in mapping.c; the
+//   matching loop is handle_mapping()).
+// - Assemble K_SPECIAL/multibyte byte sequences into keys (vgetc()).
+// - Record macros (recordbuff) and the redo keys (redobuff: "." capture and replay).
+//
+// Concepts:
+// - "Stuffing" = when some internal logic pushes keys to execute next. This is how a cmd
+//   "translates" into another cmd: "x" stuffs "dl" (nv_optrans()), "." stuffs the redo buf.
+// - TYPEAHEAD vs READ-AHEAD:
+//   - typeahead = external input that arrived faster than can be executed (user typed fast).
+//   - readahead = Nvim's own self-input.
+//     - vgetorpeek() drains it BEFORE typeahead.
+//     - Its keys are never mapped (mappings apply only to the
+//       typebuf; abbreviations are disabled too), though they count as typed for most purposes
+//       (KeyStuffed).
+//
+// These buffers are used:
+// - stuff buffers (`readbuf1`, `readbuf2`) are readahead buffers.
+//   - TWO stuff buffers, because stuffing nests: a cmd executed FROM redo keys (readbuf2) may
+//     itself stuff a translation (readbuf1), which must be consumed before the remaining redo.
+// - `typebuf`: typeahead (see below).
+// - `redobuff`: the keys of the last change ("." stuffs them, start_redo());
+//   `old_redobuff` is the previous change, for "CTRL-O ." in Insert mode.
+// - `recordbuff`: accumulates the keys of a recording ("q").
+//
+// Buffer bytes are encoded as follows:
+// - K_SPECIAL introduces a special key (two more bytes follow).
+//   A literal K_SPECIAL is stored as K_SPECIAL KS_SPECIAL KE_FILLER.
+// - These translations are also done on multi-byte characters!
+// - Escaping K_SPECIAL is done by inchar().
+// - Un-escaping is done by vgetc().
 
 #include <assert.h>
 #include <limits.h>
@@ -86,19 +117,6 @@ static int curscript = -1;
 /// Streams to read script from
 static FileDescriptor scriptin[NSCRIPT] = { 0 };
 
-// These buffers are used for storing:
-// - stuffed characters: A command that is translated into another command.
-// - redo characters: will redo the last change.
-// - recorded characters: for the "q" command.
-//
-// The bytes are stored like in the typeahead buffer:
-// - K_SPECIAL introduces a special key (two more bytes follow).  A literal
-//   K_SPECIAL is stored as K_SPECIAL KS_SPECIAL KE_FILLER.
-// These translations are also done on multi-byte characters!
-//
-// Escaping K_SPECIAL is done by inchar().
-// Un-escaping is done by vgetc().
-
 #define MINIMAL_SIZE 20                 // minimal size for b_str
 
 static buffheader_T redobuff = { { NULL, 0, { NUL } }, NULL, 0, 0, false };
@@ -1046,25 +1064,21 @@ int ins_typebuf(char *str, int noremap, int offset, bool nottyped, bool silent)
   return OK;
 }
 
-/// Put character "c" back into the typeahead buffer.
-/// Can be used for a character obtained by vgetc() that needs to be put back.
-/// Uses cmd_silent, KeyTyped and KeyNoremap to restore the flags belonging to
-/// the char.
+/// Re-queues key `c`: puts a key obtained by vgetc() back into typeahead, to be reprocessed: mapped
+/// (cf. vungetc(), which bypasses mapping), recorded and reported. Calls ungetchars() to drop the
+/// effects of consuming it the first time.
 ///
-/// @param on_key_ignore don't store these bytes for vim.on_key()
+/// Uses cmd_silent, KeyTyped and KeyNoremap to restore the flags belonging to the char.
 ///
-/// @return the length of what was inserted
-int ins_char_typebuf(int c, int modifiers, bool on_key_ignore)
+/// @param recorded  Pass false if the caller suppressed recording on the initial read.
+void requeue_key(int c, int modifiers, bool recorded)
 {
   char buf[MB_MAXBYTES * 3 + 4];
   unsigned len = special_to_buf(c, modifiers, true, buf);
   assert(len < sizeof(buf));
   buf[len] = NUL;
   ins_typebuf(buf, KeyNoremap, 0, !KeyTyped, cmd_silent);
-  if (KeyTyped && on_key_ignore) {
-    on_key_ignore_len += len;
-  }
-  return (int)len;
+  ungetchars((int)len, recorded);
 }
 
 /// Return true if the typeahead buffer was changed (while waiting for a
@@ -1270,18 +1284,24 @@ void gotchars_ignore(void)
   gotchars(nop_buf, 3);
 }
 
-/// Undo the last gotchars() for "len" bytes.  To be used when putting a typed
-/// character back into the typeahead buffer, thus gotchars() will be called
-/// again.
-/// Only affects recorded characters.
-void ungetchars(int len)
+/// Undo the last gotchars() for `len` bytes: a key that was read is being re-queued into typeahead,
+/// so it will be read (+ gotchars()) again. Un-reports the bytes, and (if `recorded`) removes them
+/// from the recording.
+///
+/// @param recorded  Pass false if recording was suppressed on the first read: the trailing
+///        `recordbuff` bytes are unrelated and deleting them would corrupt it.
+static void ungetchars(int len, bool recorded)
 {
-  if (reg_recording == 0) {
-    return;
+  if (!KeyTyped) {
+    return;  // gotchars() only sees typed keys; nothing to undo.
   }
-
-  delete_buff_tail(&recordbuff, len);
-  last_recorded_len -= (size_t)len;
+  if (recorded && reg_recording != 0) {
+    delete_buff_tail(&recordbuff, len);
+    last_recorded_len -= (size_t)len;
+  }
+  size_t trim = MIN((size_t)len, on_key_buf.size);
+  on_key_buf.size -= trim;
+  on_key_ignore_len += (size_t)len - trim;
 }
 
 /// Sync undo.  Called when typed characters are obtained from the typeahead
@@ -1709,13 +1729,15 @@ int vgetc(void)
       if (!no_mapping && KeyTyped && mod_mask == MOD_MASK_ALT
           && !(State & MODE_TERMINAL) && !is_mouse_key(c)) {
         mod_mask = 0;
-        int len = ins_char_typebuf(c, 0, false);
-        ins_char_typebuf(ESC, 0, false);
-        int old_len = len + 3;  // K_SPECIAL KS_MODIFIER MOD_MASK_ALT takes 3 more bytes
-        ungetchars(old_len);
-        if (on_key_buf.size >= (size_t)old_len) {
-          on_key_buf.size -= (size_t)old_len;
-        }
+        char kbuf[MB_MAXBYTES * 3 + 4];
+        unsigned klen = special_to_buf(c, 0, true, kbuf);
+        assert(klen < sizeof(kbuf));
+        kbuf[klen] = NUL;
+        // Un-record/un-report the consumed <M-x> (its K_SPECIAL KS_MODIFIER MOD_MASK_ALT prefix
+        // took 3 more bytes); the rewritten <Esc>x is consumed (recorded, reported) in its place.
+        ungetchars((int)klen + 3, true);
+        ins_typebuf(kbuf, KeyNoremap, 0, !KeyTyped, cmd_silent);
+        ins_typebuf(ESC_STR, KeyNoremap, 0, !KeyTyped, cmd_silent);
         continue;
       }
 

src/nvim/message.c

@@ -1554,7 +1554,9 @@ void wait_return(int redraw)
       } else if (vim_strchr("\r\n ", c) == NULL && c != Ctrl_C && c != 'q') {
         // Put the character back in the typeahead buffer.  Don't use the
         // stuff buffer, because lmaps wouldn't work.
-        ins_char_typebuf(vgetc_char, vgetc_mod_mask, true);
+        requeue_key(vgetc_char, vgetc_mod_mask,
+                    // Recording was suppressed around safe_vgetc() above.
+                    false);
         do_redraw = true;  // need a redraw even though there is typeahead
       }
     } else {
@@ -3782,7 +3784,7 @@ int do_dialog(int type, const char *title, const char *message, const char *butt
       }
       if (c == ':' && ex_cmd) {
         retval = dfltbutton;
-        ins_char_typebuf(':', 0, false);
+        ins_typebuf(":", REMAP_YES, 0, false, false);
         break;
       }
 

src/nvim/normal.c

@@ -1100,13 +1100,7 @@ static int normal_execute(VimState *state, int key)
     // When "restart_edit" is set fake a "d"elete command, Insert mode will restart automatically.
     // Insert the typed character in the typeahead buffer, so that it can
     // be mapped in Insert mode.  Required for ":lmap" to work.
-    int len = ins_char_typebuf(vgetc_char, vgetc_mod_mask, true);
-
-    // When recording and gotchars() was called the character will be
-    // recorded again, remove the previous recording.
-    if (KeyTyped) {
-      ungetchars(len);
-    }
+    requeue_key(vgetc_char, vgetc_mod_mask, true);
 
     if (restart_edit != 0) {
       s->c = 'd';

src/nvim/terminal.c

@@ -2307,10 +2307,7 @@ end:
     return false;
   }
 
-  int len = ins_char_typebuf(vgetc_char, vgetc_mod_mask, true);
-  if (KeyTyped) {
-    ungetchars(len);
-  }
+  requeue_key(vgetc_char, vgetc_mod_mask, true);
   return true;
 }
 

test/functional/editor/macro_spec.lua

@@ -1,4 +1,5 @@
 local t = require('test.testutil')
+local Screen = require('test.functional.ui.screen')
 local n = require('test.functional.testnvim')()
 
 local eq = t.eq
@@ -11,6 +12,42 @@ local fn = n.fn
 local api = n.api
 local insert = n.insert
 
+describe('macro recording with requeued key', function()
+  before_each(function()
+    clear({ args_rm = { '--cmd' } })
+  end)
+
+  it('mapped key does not corrupt the recording', function()
+    -- Typing over a Select-mode selection puts the key back for Insert mode (requeue_key()).
+    -- A key produced by a mapping is not "typed", so ungetchars() must not touch the recording.
+    command('snoremap Z Y')
+    insert('abc')
+    feed('qq0ghZ<Esc>q')
+    -- "Y" replaced the selected "a".
+    expect('Ybc')
+    -- The recording holds the typed keys: the "Y" mapping must not have eaten the recorded "Z".
+    eq('0ghZ\27', eval('@q'))
+  end)
+
+  it('at hit-enter prompt records the key ONCE', function()
+    -- A non-prompt key typed at the hit-enter prompt is put back to execute
+    -- as a normal command: it is consumed twice, but must be recorded once.
+    local screen = Screen.new(40, 6)
+    insert('abc')
+    feed('qq0')
+    feed(':echo "one\\ntwo"<CR>')
+    -- The prompt must actually engage (needs an attached UI).
+    screen:expect({ any = 'Press ENTER' })
+    feed('x')
+    feed('q')
+    expect('bc')
+    eq('0:echo "one\\ntwo"\rx', eval('@q'))
+    -- Replaying executes "x" once, not twice.
+    feed('@q')
+    expect('c')
+  end)
+end)
+
 describe('macros with default mappings', function()
   before_each(function()
     clear({ args_rm = { '--cmd' } })