refactor(os/input.c): rename os_inchar => input_get #30327

Problem:
The name `os_inchar` (from Vim's old `mch_inchar`) is ambiguous:
"inchar" sounds like it could be reading or enqueuing (setting) input.
Its docstring is also ambiguous.

Solution:
- Rename `os_inchar` to `input_get`.
- Write some mf'ing docstrings.
- Add assert() in TRY_READ().

src/nvim/event/loop.h

@@ -16,15 +16,14 @@ struct loop {
   uv_loop_t uv;
   MultiQueue *events;
   MultiQueue *thread_events;
-  // Immediate events:
+  // Immediate events.
-  //    "Processed after exiting uv_run() (to avoid recursion), but before
+  // - "Processed after exiting `uv_run()` (to avoid recursion), but before returning from
-  //    returning from loop_poll_events()." 502aee690c98
+  //   `loop_poll_events()`." 502aee690c98
-  // Practical consequence (for main_loop): these events are processed by
+  // - Practical consequence (for `main_loop`):
-  //    state_enter()..os_inchar()
+  //    - these are processed by `state_enter()..input_get()` whereas "regular" events
-  // whereas "regular" events (main_loop.events) are processed by
+  //      (`main_loop.events`) are processed by `state_enter()..VimState.execute()`
-  //    state_enter()..VimState.execute()
+  //    - `state_enter()..input_get()` can be "too early" if you want the event to trigger UI
-  // But state_enter()..os_inchar() can be "too early" if you want the event
+  //      updates and other user-activity-related side-effects.
-  // to trigger UI updates and other user-activity-related side-effects.
   MultiQueue *fast_events;
 
   // used by process/job-control subsystem

src/nvim/getchar.c

@@ -1858,7 +1858,7 @@ static void getchar_common(typval_T *argvars, typval_T *rettv)
       if (!char_avail()) {
         // Flush screen updates before blocking.
         ui_flush();
-        os_inchar(NULL, 0, -1, typebuf.tb_change_cnt, main_loop.events);
+        input_get(NULL, 0, -1, typebuf.tb_change_cnt, main_loop.events);
         if (!multiqueue_empty(main_loop.events)) {
           state_handle_k_event();
           continue;
@@ -2981,7 +2981,7 @@ int inchar(uint8_t *buf, int maxlen, long wait_time)
       uint8_t dum[DUM_LEN + 1];
 
       while (true) {
-        len = os_inchar(dum, DUM_LEN, 0, 0, NULL);
+        len = input_get(dum, DUM_LEN, 0, 0, NULL);
         if (len == 0 || (len == 1 && dum[0] == Ctrl_C)) {
           break;
         }
@@ -2997,7 +2997,7 @@ int inchar(uint8_t *buf, int maxlen, long wait_time)
 
     // Fill up to a third of the buffer, because each character may be
     // tripled below.
-    len = os_inchar(buf, maxlen / 3, (int)wait_time, tb_change_cnt, NULL);
+    len = input_get(buf, maxlen / 3, (int)wait_time, tb_change_cnt, NULL);
   }
 
   // If the typebuf was changed further down, it is like nothing was added by

src/nvim/input.c

@@ -37,7 +37,7 @@
 /// @param[in]  str  Prompt: question to ask user. Is always followed by
 ///                  " (y/n)?".
 /// @param[in]  direct  Determines what function to use to get user input. If
-///                     true then os_inchar() will be used, otherwise vgetc().
+///                     true then input_get() will be used, otherwise vgetc().
 ///                     I.e. when direct is true then characters are obtained
 ///                     directly from the user without buffers involved.
 ///
@@ -111,7 +111,7 @@ int get_keystroke(MultiQueue *events)
 
     // First time: blocking wait.  Second time: wait up to 100ms for a
     // terminal code to complete.
-    n = os_inchar(buf + len, maxlen, len == 0 ? -1 : 100, 0, events);
+    n = input_get(buf + len, maxlen, len == 0 ? -1 : 100, 0, events);
     if (n > 0) {
       // Replace zero and K_SPECIAL by a special key code.
       n = fix_input_buffer(buf + len, n);

src/nvim/msgpack_rpc/channel.h

@@ -12,7 +12,7 @@
 
 /// HACK: os/input.c drains this queue immediately before blocking for input.
 ///       Events on this queue are async-safe, but they need the resolved state
-///       of os_inchar(), so they are processed "just-in-time".
+///       of input_get(), so they are processed "just-in-time".
 EXTERN MultiQueue *ch_before_blocking_events INIT( = NULL);
 
 #ifdef INCLUDE_GENERATED_DECLARATIONS

src/nvim/normal.c

@@ -6597,11 +6597,11 @@ static void nv_open(cmdarg_T *cap)
 static void nv_event(cmdarg_T *cap)
 {
   // Garbage collection should have been executed before blocking for events in
-  // the `os_inchar` in `state_enter`, but we also disable it here in case the
+  // the `input_get` in `state_enter`, but we also disable it here in case the
-  // `os_inchar` branch was not executed (!multiqueue_empty(loop.events), which
+  // `input_get` branch was not executed (!multiqueue_empty(loop.events), which
   // could have `may_garbage_collect` set to true in `normal_check`).
   //
-  // That is because here we may run code that calls `os_inchar`
+  // That is because here we may run code that calls `input_get`
   // later(`f_confirm` or `get_keystroke` for example), but in these cases it is
   // not safe to perform garbage collection because there could be unreferenced
   // lists or dicts being used.

src/nvim/os/input.c

@@ -33,12 +33,6 @@
 #define READ_BUFFER_SIZE 0xfff
 #define INPUT_BUFFER_SIZE ((READ_BUFFER_SIZE * 4) + MAX_KEY_CODE_LEN)
 
-typedef enum {
-  kInputNone,
-  kInputAvail,
-  kInputEof,
-} InbufPollResult;
-
 static RStream read_stream = { .s.closed = true };  // Input before UI starts.
 static char input_buffer[INPUT_BUFFER_SIZE];
 static char *input_read_pos = input_buffer;
@@ -84,57 +78,76 @@ static void cursorhold_event(void **argv)
 static void create_cursorhold_event(bool events_enabled)
 {
   // If events are enabled and the queue has any items, this function should not
-  // have been called (inbuf_poll would return kInputAvail).
+  // have been called (`inbuf_poll` would return `kTrue`).
   // TODO(tarruda): Cursorhold should be implemented as a timer set during the
   // `state_check` callback for the states where it can be triggered.
   assert(!events_enabled || multiqueue_empty(main_loop.events));
   multiqueue_put(main_loop.events, cursorhold_event, NULL);
 }
 
-static void restart_cursorhold_wait(int tb_change_cnt)
+static void reset_cursorhold_wait(int tb_change_cnt)
 {
   cursorhold_time = 0;
   cursorhold_tb_change_cnt = tb_change_cnt;
 }
 
-/// Low level input function
+/// Reads OS input into `buf`, and consumes pending events while waiting (if `ms != 0`).
 ///
-/// Wait until either the input buffer is non-empty or, if `events` is not NULL
+/// - Consumes available input received from the OS.
-/// until `events` is non-empty.
+/// - Consumes pending events.
-int os_inchar(uint8_t *buf, int maxlen, int ms, int tb_change_cnt, MultiQueue *events)
+/// - Manages CursorHold events.
+/// - Handles EOF conditions.
+///
+/// Originally based on the Vim `mch_inchar` function.
+///
+/// @param buf Buffer to store read input.
+/// @param maxlen Maximum bytes to read into `buf`, or 0 to skip reading.
+/// @param ms Timeout in milliseconds. -1 for indefinite wait, 0 for no wait.
+/// @param tb_change_cnt Used to detect when typeahead changes.
+/// @param events (optional) Events to process.
+/// @return Bytes read into buf, or 0 if no input was read
+int input_get(uint8_t *buf, int maxlen, int ms, int tb_change_cnt, MultiQueue *events)
 {
   // This check is needed so that feeding typeahead from RPC can prevent CursorHold.
   if (tb_change_cnt != cursorhold_tb_change_cnt) {
-    restart_cursorhold_wait(tb_change_cnt);
+    reset_cursorhold_wait(tb_change_cnt);
   }
 
-  if (maxlen && input_available()) {
+#define TRY_READ() \
-    restart_cursorhold_wait(tb_change_cnt);
+  do { \
-    size_t to_read = MIN((size_t)maxlen, input_available());
+    if (maxlen && input_available()) { \
-    memcpy(buf, input_read_pos, to_read);
+      reset_cursorhold_wait(tb_change_cnt); \
-    input_read_pos += to_read;
+      assert(maxlen >= 0); \
-    return (int)to_read;
+      size_t to_read = MIN((size_t)maxlen, input_available()); \
-  }
+      memcpy(buf, input_read_pos, to_read); \
+      input_read_pos += to_read; \
+      /* This is safe because INPUT_BUFFER_SIZE fits in an int. */ \
+      assert(to_read <= INT_MAX); \
+      return (int)to_read; \
+    } \
+  } while (0)
+
+  TRY_READ();
 
   // No risk of a UI flood, so disable CTRL-C "interrupt" behavior if it's mapped.
   if ((mapped_ctrl_c | curbuf->b_mapped_ctrl_c) & get_real_state()) {
     ctrl_c_interrupts = false;
   }
 
-  InbufPollResult result;
+  TriState result;  ///< inbuf_poll result.
   if (ms >= 0) {
-    if ((result = inbuf_poll(ms, events)) == kInputNone) {
+    if ((result = inbuf_poll(ms, events)) == kFalse) {
       return 0;
     }
   } else {
     uint64_t wait_start = os_hrtime();
     cursorhold_time = MIN(cursorhold_time, (int)p_ut);
-    if ((result = inbuf_poll((int)p_ut - cursorhold_time, events)) == kInputNone) {
+    if ((result = inbuf_poll((int)p_ut - cursorhold_time, events)) == kFalse) {
       if (read_stream.s.closed && silent_mode) {
         // Drained eventloop & initial input; exit silent/batch-mode (-es/-Es).
         read_error_exit();
       }
-      restart_cursorhold_wait(tb_change_cnt);
+      reset_cursorhold_wait(tb_change_cnt);
       if (trigger_cursorhold() && !typebuf_changed(tb_change_cnt)) {
         create_cursorhold_event(events == main_loop.events);
       } else {
@@ -153,32 +166,26 @@ int os_inchar(uint8_t *buf, int maxlen, int ms, int tb_change_cnt, MultiQueue *e
     return 0;
   }
 
-  if (maxlen && input_available()) {
+  TRY_READ();
-    restart_cursorhold_wait(tb_change_cnt);
-    // Safe to convert `to_read` to int, it will never overflow since
-    // INPUT_BUFFER_SIZE fits in an int
-    size_t to_read = MIN((size_t)maxlen, input_available());
-    memcpy(buf, input_read_pos, to_read);
-    input_read_pos += to_read;
-    return (int)to_read;
-  }
 
   // If there are events, return the keys directly
   if (maxlen && pending_events(events)) {
     return push_event_key(buf, maxlen);
   }
 
-  if (result == kInputEof) {
+  if (result == kNone) {
     read_error_exit();
   }
 
   return 0;
+
+#undef TRY_READ
 }
 
 // Check if a character is available for reading
 bool os_char_avail(void)
 {
-  return inbuf_poll(0, NULL) == kInputAvail;
+  return inbuf_poll(0, NULL) == kTrue;
 }
 
 /// Poll for fast events. `got_int` will be set to `true` if CTRL-C was typed.
@@ -463,15 +470,22 @@ bool input_blocking(void)
   return blocking;
 }
 
-// This is a replacement for the old `WaitForChar` function in os_unix.c
+/// Checks for (but does not read) available input, and consumes `main_loop.events` while waiting.
-static InbufPollResult inbuf_poll(int ms, MultiQueue *events)
+///
+/// @param ms Timeout in milliseconds. -1 for indefinite wait, 0 for no wait.
+/// @param events (optional) Queue to check for pending events.
+/// @return TriState:
+///   - kTrue: Input/events available
+///   - kFalse: No input/events
+///   - kNone: EOF reached on the input stream
+static TriState inbuf_poll(int ms, MultiQueue *events)
 {
   if (os_input_ready(events)) {
-    return kInputAvail;
+    return kTrue;
   }
 
   if (do_profiling == PROF_YES && ms) {
-    prof_inchar_enter();
+    prof_input_start();
   }
 
   if ((ms == -1 || ms > 0) && events != main_loop.events && !input_eof) {
@@ -479,20 +493,18 @@ static InbufPollResult inbuf_poll(int ms, MultiQueue *events)
     blocking = true;
     multiqueue_process_events(ch_before_blocking_events);
   }
-  DLOG("blocking... events_enabled=%d events_pending=%d", events != NULL,
+  DLOG("blocking... events=%d pending=%d", !!events, pending_events(events));
-       events && !multiqueue_empty(events));
+  LOOP_PROCESS_EVENTS_UNTIL(&main_loop, NULL, ms, os_input_ready(events) || input_eof);
-  LOOP_PROCESS_EVENTS_UNTIL(&main_loop, NULL, ms,
-                            os_input_ready(events) || input_eof);
   blocking = false;
 
   if (do_profiling == PROF_YES && ms) {
-    prof_inchar_exit();
+    prof_input_end();
   }
 
   if (os_input_ready(events)) {
-    return kInputAvail;
+    return kTrue;
   }
-  return input_eof ? kInputEof : kInputNone;
+  return input_eof ? kNone : kFalse;
 }
 
 static size_t input_read_cb(RStream *stream, const char *buf, size_t c, void *data, bool at_eof)
@@ -533,8 +545,8 @@ static void process_ctrl_c(void)
   }
 }
 
-// Helper function used to push bytes from the 'event' key sequence partially
+/// Pushes bytes from the "event" key sequence (KE_EVENT) partially between calls to input_get when
-// between calls to os_inchar when maxlen < 3
+/// `maxlen < 3`.
 static int push_event_key(uint8_t *buf, int maxlen)
 {
   static const uint8_t key[3] = { K_SPECIAL, KS_EXTRA, KE_EVENT };

src/nvim/profile.c

@@ -383,19 +383,19 @@ void set_context_in_profile_cmd(expand_T *xp, const char *arg)
   xp->xp_context = EXPAND_NOTHING;
 }
 
-static proftime_T inchar_time;
+static proftime_T wait_time;
 
 /// Called when starting to wait for the user to type a character.
-void prof_inchar_enter(void)
+void prof_input_start(void)
 {
-  inchar_time = profile_start();
+  wait_time = profile_start();
 }
 
 /// Called when finished waiting for the user to type a character.
-void prof_inchar_exit(void)
+void prof_input_end(void)
 {
-  inchar_time = profile_end(inchar_time);
+  wait_time = profile_end(wait_time);
-  profile_set_wait(profile_add(profile_get_wait(), inchar_time));
+  profile_set_wait(profile_add(profile_get_wait(), wait_time));
 }
 
 /// @return  true when a function defined in the current script should be

src/nvim/state.c

@@ -74,10 +74,9 @@ getkey:
       }
       // Flush screen updates before blocking.
       ui_flush();
-      // Call `os_inchar` directly to block for events or user input without
+      // Call `input_get` directly to block for events or user input without consuming anything from
-      // consuming anything from `input_buffer`(os/input.c) or calling the
+      // `os/input.c:input_buffer` or calling the mapping engine.
-      // mapping engine.
+      input_get(NULL, 0, -1, typebuf.tb_change_cnt, main_loop.events);
-      os_inchar(NULL, 0, -1, typebuf.tb_change_cnt, main_loop.events);
       // If an event was put into the queue, we send K_EVENT directly.
       if (!input_available() && !multiqueue_empty(main_loop.events)) {
         key = K_EVENT;