refactor(cmdatom): atom_redo_keys #41345

Some names/comments are misleading.
Also add some asserts.

src/klib/kvec.h

@@ -114,7 +114,9 @@
     (v).size = (v).size + len; \
   }
 
+/// Appends a string to `v`, without its NUL.
 #define kv_concat(v, str) kv_concat_len(v, str, strlen(str))
+/// Appends all items of `v0` to `v1`.
 #define kv_splice(v1, v0) kv_concat_len(v1, (v0).items, (v0).size)
 
 #define kv_pushp(v) \

src/nvim/input.c

@@ -233,8 +233,8 @@ char *get_recorded(void)
   return p;
 }
 
-/// Appends the composed `["x][count]` keysequence prefix of `spec` to `buf`.
-/// Every prefix emission goes through here.
+/// Composes a `["x][count]` prefix from `spec` and appends it to `buf`.
+/// This is "Step 1" of redo-composition ("Step 2" is either `redobuff.cur.keys` or `redo_chars`).
 ///
 /// @param replay  Composing an actual replay (start_redo()): a `"=` register spec appends <CR>,
 ///                re-evaluating the last expression.
@@ -253,8 +253,8 @@ void redo_prefix(const CmdSpec *spec, StringBuilder *buf, bool replay)
   }
 }
 
-/// Appends the composed command chars of `spec` to `buf`.
-/// Every command-char emission goes through here (see redo_prefix()).
+/// Composes a command keysequence from `spec` and appends it to `buf`.
+/// This is "Step 2" of redo-composition ("Step 1" is `redo_prefix`).
 ///
 /// @param arg_meta  Skip the `arg` byte (see prep_redo()).
 void redo_chars(const CmdSpec *spec, StringBuilder *buf, bool arg_meta)
@@ -280,29 +280,33 @@ void redo_chars(const CmdSpec *spec, StringBuilder *buf, bool arg_meta)
   }
 }
 
-/// Composes a redo's full keysequence: the `["x][v][count]` prefix (from the fields) followed by
-/// the command body.
+/// Takes `buf`'s bytes as an allocated, NUL-terminated String, and clears `buf`.
 ///
-/// @return  allocated String; .data == NULL if the redo is empty.
-static String redo_compose(RedoBuf *r)
+/// @return  String; .data=NULL if `buf` is empty.
+String sb_take_string(StringBuilder *buf)
   FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
 {
-  StringBuilder buf = KV_INITIAL_VALUE;
-  redo_prefix(&r->spec, &buf, false);
-  kv_splice(buf, r->keys);
-  if (buf.size == 0) {
+  if (buf->size == 0) {
+    kv_destroy(*buf);
     return (String)STRING_INIT;
   }
-  assert(buf.items != NULL);  // Coverity false-positive (already checked `size` above).
-  kv_push(buf, NUL);
-  return cbuf_as_string(buf.items, buf.size - 1);
+  size_t len = buf->size;
+  assert(buf->items != NULL);  // Coverity false-positive (already checked `size` above).
+  kv_push(*buf, NUL);
+  char *items = buf->items;  // ownership moves to the caller
+  *buf = (StringBuilder)KV_INITIAL_VALUE;
+  return cbuf_as_string(items, len);
 }
 
-/// Gets the pending change's keysequence (redo_compose()), allocated.
+/// Gets the pending change: the `["x][count]` prefix + the captured command body.
+/// @return Allocated key sequence.
 String redo_keys(void)
   FUNC_ATTR_WARN_UNUSED_RESULT
 {
-  return redo_compose(&redobuff.cur);
+  StringBuilder buf = KV_INITIAL_VALUE;
+  redo_prefix(&redobuff.cur.spec, &buf, false);
+  kv_splice(buf, redobuff.cur.keys);
+  return sb_take_string(&buf);
 }
 
 /// Gets the pending change's CmdSpec.
@@ -841,10 +845,10 @@ void stuffescaped(const char *arg, bool literally)
   }
 }
 
-/// Dot-repeat "." command: repeats the last change by composing the redo (fields + body, see
-/// RedoBuf) into readbuf2. "3." replaces count; a numbered-register redo increments regname
-/// ('"1p' then "." pastes '"2'), so "." steps through the delete history. A Visual-mode change
-/// re-executes its captured selection keys (embedded in the body, see prep_redo()).
+/// Dot-repeat "." command: repeats the last change by composing the redo into readbuf2. "3."
+/// replaces count; a numbered-register redo increments regname ('"1p' then "." pastes '"2'), so "."
+/// steps through the delete history. A Visual-mode change re-executes its captured selection keys
+/// (embedded in the body, see prep_redo()).
 ///
 /// @param old_redo  repeat the last-but-one change (i_CTRL-O ".": the insert
 ///                  session's own prep moved the last change to redobuff.old)

src/nvim/input_cmdatom.c

@@ -160,56 +160,55 @@ CmdSpec atom_cmd_spec(const cmdarg_T *cap)
   };
 }
 
-/// Composes "redo keys" (allocated) from `spec`, as prep_redo() + "." would: for commands
-/// that never prep (motions, "u", "zz"). NULL during a cascade.
-static char *atom_compose_keys(CmdSpec spec)
+/// Composes a CmdSpec into `redo_keys` format.
+/// @return Allocated key sequence.
+static char *atom_redo_keys(CmdSpec spec)
 {
-  StringBuilder sb = KV_INITIAL_VALUE;
-  redo_prefix(&spec, &sb, false);
-  redo_chars(&spec, &sb, false);
-  if (sb.size == 0) {
-    return NULL;
-  }
-  assert(sb.items != NULL);  // Coverity false-positive (already checked `size` above).
-  kv_push(sb, NUL);
-  return sb.items;
+  StringBuilder buf = KV_INITIAL_VALUE;
+  redo_prefix(&spec, &buf, false);
+  redo_chars(&spec, &buf, false);
+  char *keys = sb_take_string(&buf).data;
+  assert(keys != NULL);  // A spec with no chars/count/reg composes to nothing.
+  return keys;
 }
 
-/// The pending change as a CmdAtom: the composed keysequence plus the structured fields.
-/// Caller owns `keys`.
+/// Gets the pending change as a CmdAtom. Caller owns `keys`.
 static CmdAtom atom_from_redo(CmdAtomType type)
 {
   String keys = redo_keys();
   return (CmdAtom){ .type = type, .spec = redo_spec(), .keys = keys.data };
 }
 
-/// Builds a CmdAtom whose `keys` (atom_compose_keys()) and fields both come from `spec`.
+/// Gets a CmdAtom from a CmdSpec.
 static CmdAtom atom_from_spec(CmdAtomType type, CmdSpec spec)
 {
-  return (CmdAtom){ .type = type, .spec = spec, .keys = atom_compose_keys(spec) };
+  return (CmdAtom){ .type = type, .spec = spec, .keys = atom_redo_keys(spec) };
 }
 
-/// Builds the atom of a typed cmdline:
+/// Gets a typed cmdline as a CmdAtom.
 ///   ":cnext<CR>" => CmdAtom{ kAEx, keys=":cnext<NL>", text="cnext" }
-static CmdAtom atom_from_cmdline(CmdAtomType type, cmdarg_T *ca, const char *line)
+static CmdAtom atom_from_cmdline(CmdAtomType type, cmdarg_T *ca, const char *cmdline)
 {
   StringBuilder sb = KV_INITIAL_VALUE;
   if (type != kAEx && ca->count0 != 0) {
     kv_printf(sb, "%d", ca->count0);
   }
   sb_add_char(&sb, ca->cmdchar);
-  sb_add_lit(&sb, line, -1);
+  sb_add_lit(&sb, cmdline, -1);
   sb_add_char(&sb, NL);
   kv_push(sb, NUL);
   return (CmdAtom){
     .type = type,
     .spec = { .count = ca->count0, .cmd = ca->cmdchar },
     .keys = sb.items,
-    .text = xstrdup(line),
+    .text = xstrdup(cmdline),
   };
 }
 
-/// Concatenates the keys of multiple atoms into one (allocated) string.
+/// Joins the `keys` of a list of (composite) subatoms. This is a plain concat (the `keys` field of
+/// each subatom is assumed to be in `redo_keys` format).
+///
+/// @return Allocated keysequence, "" if `atoms` is empty (never NULL).
 static String atoms_concat_keys(CmdAtomVec atoms)
 {
   StringBuilder keys = KV_INITIAL_VALUE;
@@ -755,11 +754,7 @@ static void atom_capture_visual(cmdarg_T *ca, const CmdBaseline *old)
   // Omit `regname`, it would prefix '"x' to every command captured after a register spec.
   CmdSpec spec = { .count = ca->count0, .cmd = ca->cmdchar,
                    .cmd2 = operand ? NUL : ca->nchar, .arg = operand ? ca->nchar : NUL };
-  char *keys = atom_compose_keys(spec);
-  if (keys == NULL) {
-    return;
-  }
-  kv_push(vatom.atoms, ((CmdAtom){ .type = kAMotion, .spec = spec, .keys = keys }));
+  kv_push(vatom.atoms, ((CmdAtom){ .type = kAMotion, .spec = spec, .keys = atom_redo_keys(spec) }));
 }
 
 /// Ends the pending visual atom, appends `suffix`, and stages it. Or discards it if selection is
@@ -830,7 +825,7 @@ static bool atom_visual_end_suffix(char *suffix, const CmdSpec *spec, bool redoa
 /// @return  True if the redo was prepped.
 bool atom_visual_end(CmdSpec spec, bool redoable)
 {
-  return atom_visual_end_suffix(atom_compose_keys(spec), &spec, redoable);
+  return atom_visual_end_suffix(atom_redo_keys(spec), &spec, redoable);
 }
 
 /// Captures a pending operator's atom before it executes. Prep-exempt commands (yank without cpo-y,

src/nvim/input_cmdatom_defs.h

@@ -52,7 +52,8 @@ struct CmdAtom {
   CmdSpec spec;   ///< Structured fields.
   CmdAtomVec atoms;  ///< Composite (multi-command mapping, Visual sequence): its subatoms,
                      ///< in order; their keys concatenate to `keys`. Empty for non-composite.
-  char *keys;     ///< Resolved keysequence (typeahead encoding).
+  char *keys;     ///< Resolved keysequence (typeahead encoding), including `["x][count]` prefix
+                  ///< (unlike `RedoBuf.keys`).
   char *text;     ///< Payload: insert-session text, or Ex or search cmdline.
   char *lhs;      ///< Mapping LHS or macro register ("gj", "@q") that produced this atom, or NULL.
                   ///< Label/hint, not replayed.

src/nvim/input_defs.h

@@ -32,8 +32,8 @@ typedef struct {
 /// Structured decomposition of a normal-mode command, used two ways:
 /// - Capture (prep_redo()): the command appends its own bytes to the redo body; only `regname`
 ///   and `count` are functional (the `["x][count]` prefix), the rest is CmdAtom metadata.
-/// - Reconstruction (atom_from_spec()): a command that never preps ("u", motions) has no body, so
-///   atom_compose_keys() composes the whole keysequence from the spec.
+/// - Reconstruction (atom_from_spec()): a "non-prepped" command ("u", motions) has no body, so
+///   atom_redo_keys() composes the keysequence fully from the spec.
 typedef struct {
   long count;        ///< Effective count (0 = none)
   int regname;       ///< Register (`"x` prefix; 0 = none)
@@ -45,10 +45,10 @@ typedef struct {
   int arg;           ///< Operand ("fx" => 'x', "ma" => 'a'; 0 = none)
 } CmdSpec;
 
-/// The last change: structured fields plus the command body, filled as the command executes
-/// (redo_append_xx()). "." (start_redo()) composes the `["x][count]` prefix around the body.
+/// The last change. Updated as the command executes (redo_append_xx). redo_keys() treats `keys` as
+/// the command "body", but gets the "prefix" `["x][count]` from `spec`.
 typedef struct {
-  CmdSpec spec;        ///< Structured command fields.
+  CmdSpec spec;        ///< "Metadata", except reg/count provide the "prefix".
   StringBuilder keys;  ///< Cmd body. Perf: StringBuilder (not buffheader_T) => fewer allocs/copies.
 } RedoBuf;