From 5f07fc91c23196cac1ff39961d8f60f0dbd57749 Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Mon, 17 Aug 2026 05:37:06 -0400 Subject: [PATCH] refactor(cmdatom): drop RedoBuf #41351 Problem: `RedoBuf` is mostly indirection. It has a mild benefit as an "ownership" signal but it counteracts the general goal of unifying how "redo state" is passed throughout the system, tends to sprout redundant interfaces, and reduces clarity. Solution: Add `CmdSpec.body` to hold the "prefixless" key sequence. Reuse `CmdSpec` to represent a "redo" buf. --- src/nvim/input.c | 116 ++++++++++++++++++---------------- src/nvim/input_cmdatom.c | 7 +- src/nvim/input_cmdatom_defs.h | 2 +- src/nvim/input_defs.h | 22 +++---- src/nvim/insert.c | 6 +- src/nvim/normal_defs.h | 3 +- 6 files changed, 78 insertions(+), 78 deletions(-) diff --git a/src/nvim/input.c b/src/nvim/input.c index 75d698d68f..4b0a071d09 100644 --- a/src/nvim/input.c +++ b/src/nvim/input.c @@ -22,11 +22,7 @@ // - 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` (RedoState): the last change; dot-repeat "." replays it (start_redo()). -// - `redobuff.cur` = the last change. "." replays it IN-PLACE (start_redo()), the multicursor -// cascade replays it PER-CURSOR (mc_cascade()). Same keysequence, either way: a Visual-mode -// change re-executes its captured selection). -// - `redobuff.old` = the previous change; see `redo_new`. +// - `redobuff` (RedoState): the current + previous change. // - `recordbuff`: accumulates the keys of a recording ("q"). // // Buffer bytes are encoded as follows: @@ -123,9 +119,8 @@ static FileDescriptor scriptin[NSCRIPT] = { 0 }; #define MINIMAL_SIZE 20 // minimal size for b_str -#define REDO_INIT { { 0 }, KV_INITIAL_VALUE } -/// The redo state: redo_append_*() captures keys in `redobuff.cur`; "." replays it (start_redo()). -static RedoState redobuff = { REDO_INIT, REDO_INIT }; +/// The current + previous change. redo_append_xx() captures in `redobuff.cur`; "." replays it. +static RedoState redobuff; /// Macro recording. Perf: StringBuilder (not buffheader_T) => fewer allocs/copies. static StringBuilder recordbuff = KV_INITIAL_VALUE; /// First readahead buffer ("stuffbuf"): command translations ("x" => "dl"). Drains before readbuf2. @@ -234,11 +229,11 @@ char *get_recorded(void) } /// 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`). +/// This is "Step 1" of redo-composition ("Step 2" is either `redobuff.cur.body` or `redo_chars`). /// /// @param replay Composing an actual replay (start_redo()): a `"=` register spec appends , /// re-evaluating the last expression. -void redo_prefix(const CmdSpec *spec, StringBuilder *buf, bool replay) +static void redo_prefix(const CmdSpec *spec, StringBuilder *buf, bool replay) FUNC_ATTR_NONNULL_ARG(1) { if (spec->regname != 0) { @@ -257,7 +252,7 @@ void redo_prefix(const CmdSpec *spec, StringBuilder *buf, bool replay) /// 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) +static void redo_chars(const CmdSpec *spec, StringBuilder *buf, bool arg_meta) FUNC_ATTR_NONNULL_ALL { if (spec->op != NUL) { @@ -283,7 +278,7 @@ void redo_chars(const CmdSpec *spec, StringBuilder *buf, bool arg_meta) /// Takes `buf`'s bytes as an allocated, NUL-terminated String, and clears `buf`. /// /// @return String; .data=NULL if `buf` is empty. -String sb_take_string(StringBuilder *buf) +static String sb_take_string(StringBuilder *buf) FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT { if (buf->size == 0) { @@ -298,21 +293,31 @@ String sb_take_string(StringBuilder *buf) return cbuf_as_string(items, len); } -/// Gets the pending change: the `["x][count]` prefix + the captured command body. -/// @return Allocated key sequence. -String redo_keys(void) +/// Gets `spec` (or current pending change, if `spec` is NULL), composed into a "redo" keyseq +/// `["x][count][body]`, where `[body]` is `spec.body`, or composed from the spec if body is empty. +/// +/// @return Allocated key sequence; .data == NULL if the result is empty. +String redo_keys(const CmdSpec *spec) FUNC_ATTR_WARN_UNUSED_RESULT { + spec = spec ? spec : &redobuff.cur; + StringBuilder buf = KV_INITIAL_VALUE; - redo_prefix(&redobuff.cur.spec, &buf, false); - kv_splice(buf, redobuff.cur.keys); + redo_prefix(spec, &buf, false); + if (spec->body.size > 0) { + kv_splice(buf, spec->body); + } else { + redo_chars(spec, &buf, false); + } return sb_take_string(&buf); } -/// Gets the pending change's CmdSpec. +/// Gets the pending change's CmdSpec (without `body`). CmdSpec redo_spec(void) { - return redobuff.cur.spec; + CmdSpec spec = redobuff.cur; + spec.body = (StringBuilder)KV_INITIAL_VALUE; + return spec; } /// Append string after the current block of the given buffer @@ -409,7 +414,7 @@ static size_t key_char_encode(int c, char *buf) return off; } -/// Append character 'c' to buffer "buf". +/// Append character 'c' to `buf`. /// Translates special keys, NUL, K_SPECIAL and multibyte characters. static void add_char_buff(buffheader_T *buf, int c) FUNC_ATTR_NONNULL_ALL @@ -571,23 +576,23 @@ void beep_flush(void) /// @param spec Structured command fields; zeroed if the caller only appends keys. void redo_new(CmdSpec spec) { + assert(spec.body.size == 0); if (block_redo) { return; } - kv_destroy(redobuff.old.keys); + kv_destroy(redobuff.old.body); redobuff.old = redobuff.cur; - redobuff.cur = (RedoBuf)REDO_INIT; - redobuff.cur.spec = spec; + redobuff.cur = spec; } #ifdef EXITFREE /// Frees both redo buffers. void redo_free_all(void) { - kv_destroy(redobuff.cur.keys); - kv_destroy(redobuff.old.keys); - redobuff = (RedoState){ REDO_INIT, REDO_INIT }; + kv_destroy(redobuff.cur.body); + kv_destroy(redobuff.old.body); + redobuff = (RedoState){ 0 }; } #endif @@ -606,7 +611,7 @@ void prep_redo(bool claim, bool arg_meta, CmdSpec spec) if (block_redo) { return; } - redo_chars(&spec, &redobuff.cur.keys, arg_meta); + redo_chars(&spec, &redobuff.cur.body, arg_meta); } /// Prepare for redo of a Visual-mode command: the body opens with `keys` (the captured selection), @@ -623,9 +628,9 @@ void prep_redo_visual(const char *keys, size_t len, CmdSpec spec) if (block_redo) { return; } - kv_concat_len(redobuff.cur.keys, keys, len); - redo_prefix(&spec, &redobuff.cur.keys, false); - redo_chars(&spec, &redobuff.cur.keys, false); + kv_concat_len(redobuff.cur.body, keys, len); + redo_prefix(&spec, &redobuff.cur.body, false); + redo_chars(&spec, &redobuff.cur.body, false); } /// Discard the contents of the redo buffer and restore the previous redo @@ -636,9 +641,9 @@ void redo_cancel(void) return; } - kv_destroy(redobuff.cur.keys); + kv_destroy(redobuff.cur.body); redobuff.cur = redobuff.old; - redobuff.old = (RedoBuf)REDO_INIT; + redobuff.old = (CmdSpec){ 0 }; free_buff(&readbuf1); free_buff(&readbuf2); } @@ -649,12 +654,12 @@ void save_redobuff(RedoState *save_redo) FUNC_ATTR_NONNULL_ALL { *save_redo = redobuff; - redobuff.cur.keys = (StringBuilder)KV_INITIAL_VALUE; - redobuff.old = (RedoBuf)REDO_INIT; + redobuff.cur.body = (StringBuilder)KV_INITIAL_VALUE; + redobuff.old = (CmdSpec){ 0 }; // Make a copy (the fields stayed, copy the body), so that ":normal ." in a // function works. - kv_splice(redobuff.cur.keys, save_redo->cur.keys); + kv_splice(redobuff.cur.body, save_redo->cur.body); } /// Restores the redo state from "save_redo". @@ -662,8 +667,8 @@ void save_redobuff(RedoState *save_redo) void restore_redobuff(RedoState *save_redo) FUNC_ATTR_NONNULL_ALL { - kv_destroy(redobuff.cur.keys); - kv_destroy(redobuff.old.keys); + kv_destroy(redobuff.cur.body); + kv_destroy(redobuff.old.body); redobuff = *save_redo; } @@ -673,7 +678,7 @@ void redo_append_str(const char *s, ptrdiff_t len) { if (!block_redo) { size_t slen = len < 0 ? strlen(s) : (size_t)len; - kv_concat_len(redobuff.cur.keys, s, slen); + kv_concat_len(redobuff.cur.body, s, slen); } } @@ -721,14 +726,14 @@ void sb_add_lit(StringBuilder *buf, const char *str, int len) } } -/// Append to RedoBuf buffer literally; no-op when `block_redo` is set; Insert dot-repeat consumes +/// Append to redo literally; no-op if `block_redo` is set; Insert dot-repeat consumes /// the redo buffer and must not append to it while doing so. void redo_append_lit(const char *str, int len) { if (block_redo) { return; } - sb_add_lit(&redobuff.cur.keys, str, len); + sb_add_lit(&redobuff.cur.body, str, len); } /// Append "s" to the redo buffer, leaving 3-byte special key codes unmodified @@ -742,10 +747,10 @@ void redo_append_spec(const char *s) while (*s != NUL) { if ((uint8_t)(*s) == K_SPECIAL && s[1] != NUL && s[2] != NUL) { // Insert special key literally. - kv_concat_len(redobuff.cur.keys, s, 3); + kv_concat_len(redobuff.cur.body, s, 3); s += 3; } else { - sb_add_char(&redobuff.cur.keys, mb_cptr2char_adv(&s)); + sb_add_char(&redobuff.cur.body, mb_cptr2char_adv(&s)); } } } @@ -755,7 +760,7 @@ void redo_append_spec(const char *s) void redo_append_char(int c) { if (!block_redo) { - sb_add_char(&redobuff.cur.keys, c); + sb_add_char(&redobuff.cur.body, c); } } @@ -763,7 +768,7 @@ void redo_append_char(int c) void redo_append_num(int n) { if (!block_redo) { - kv_printf(redobuff.cur.keys, "%d", n); + kv_printf(redobuff.cur.body, "%d", n); } } @@ -865,13 +870,14 @@ void stuffescaped(const char *arg, bool literally) /// @return FAIL for failure, OK otherwise int start_redo(int count, bool old_redo) { - RedoBuf *rd = old_redo ? &redobuff.old : &redobuff.cur; - if (rd->keys.size == 0 && rd->spec.regname == 0 && rd->spec.count == 0) { + CmdSpec *rd = old_redo ? &redobuff.old : &redobuff.cur; + if (rd->body.size == 0 && rd->regname == 0 && rd->count == 0) { return FAIL; // nothing to redo } - // The replay's divergences from the captured spec, as explicit tweaks of a local copy: - CmdSpec spec = rd->spec; + // Tweak the captured spec for replay: + CmdSpec spec = *rd; + spec.body = (StringBuilder)KV_INITIAL_VALUE; if (spec.regname >= '1' && spec.regname < '9') { spec.regname++; // numbered register: "." steps through the delete history } @@ -887,7 +893,7 @@ int start_redo(int count, bool old_redo) cmd_silent = true; } - add_buff(&readbuf2, rd->keys.items, (ptrdiff_t)rd->keys.size); + add_buff(&readbuf2, rd->body.items, (ptrdiff_t)rd->body.size); return OK; } @@ -897,14 +903,14 @@ int start_redo(int count, bool old_redo) /// @return FAIL for failure, OK otherwise int start_redo_ins(void) { - if (redobuff.cur.keys.size == 0) { + if (redobuff.cur.body.size == 0) { return FAIL; } start_stuff(); // Skip to the insert command; the rest of the keys is the inserted text. - const char *p = redobuff.cur.keys.items; - const char *const end = p + redobuff.cur.keys.size; + const char *p = redobuff.cur.body.items; + const char *const end = p + redobuff.cur.body.size; while (p < end) { if ((uint8_t)(*p) == K_SPECIAL && end - p >= 3) { p += 3; // a special key is never the insert command @@ -3529,7 +3535,7 @@ void paste_store(const uint64_t channel_id, const TriState state, const String s if (state == kFalse && !(State & MODE_INSERT)) { redo_new((CmdSpec){ 0 }); } - sb_add_char(&redobuff.cur.keys, c); + sb_add_char(&redobuff.cur.body, c); } if (need_record) { sb_add_char(&recordbuff, c); @@ -3549,7 +3555,7 @@ void paste_store(const uint64_t channel_id, const TriState state, const String s if (s > start) { if (need_redo) { - kv_concat_len(redobuff.cur.keys, start, (size_t)(s - start)); + kv_concat_len(redobuff.cur.body, start, (size_t)(s - start)); } if (need_record) { kv_concat_len(recordbuff, start, (size_t)(s - start)); @@ -3565,7 +3571,7 @@ void paste_store(const uint64_t channel_id, const TriState state, const String s c = NL; } if (need_redo) { - sb_add_byte(&redobuff.cur.keys, c); + sb_add_byte(&redobuff.cur.body, c); } if (need_record) { sb_add_byte(&recordbuff, c); diff --git a/src/nvim/input_cmdatom.c b/src/nvim/input_cmdatom.c index 44b4d71100..3743dc4002 100644 --- a/src/nvim/input_cmdatom.c +++ b/src/nvim/input_cmdatom.c @@ -165,10 +165,7 @@ CmdSpec atom_cmd_spec(const cmdarg_T *cap) /// @return Allocated key sequence. static char *atom_redo_keys(CmdSpec spec) { - StringBuilder buf = KV_INITIAL_VALUE; - redo_prefix(&spec, &buf, false); - redo_chars(&spec, &buf, false); - char *keys = sb_take_string(&buf).data; + char *keys = redo_keys(&spec).data; assert(keys != NULL); // A spec with no chars/count/reg composes to nothing. return keys; } @@ -176,7 +173,7 @@ static char *atom_redo_keys(CmdSpec spec) /// Gets the pending change as a CmdAtom. Caller owns `keys`. static CmdAtom atom_from_redo(CmdAtomType type) { - String keys = redo_keys(); + String keys = redo_keys(NULL); return (CmdAtom){ .type = type, .spec = redo_spec(), .keys = keys.data }; } diff --git a/src/nvim/input_cmdatom_defs.h b/src/nvim/input_cmdatom_defs.h index b6b58708ff..4302de1d4c 100644 --- a/src/nvim/input_cmdatom_defs.h +++ b/src/nvim/input_cmdatom_defs.h @@ -53,7 +53,7 @@ struct CmdAtom { 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), including `["x][count]` prefix - ///< (unlike `RedoBuf.keys`). + ///< (unlike `CmdSpec.body`, the raw unprefixed form). 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. diff --git a/src/nvim/input_defs.h b/src/nvim/input_defs.h index e56ffe73fb..851e30f6a3 100644 --- a/src/nvim/input_defs.h +++ b/src/nvim/input_defs.h @@ -18,7 +18,7 @@ typedef struct buffblock { /// escaped). Appends (add_buff()) fill the spare space of the last block, allocating a new block /// when full; reads consume from the front (read_readbuf()). /// -/// Note: Append-only key accumulation (RedoBuf, macro recording) uses StringBuilder instead. +/// Note: Append-only key accumulation (redo capture, macro recording) uses StringBuilder instead. typedef struct { buffblock_T bh_first; ///< empty sentinel: bh_first.b_next holds the first content buffblock_T *bh_curr; ///< buffblock for appending @@ -29,7 +29,9 @@ typedef struct { #define BUFFHEADER_INIT { { NULL, 0, { NUL } }, NULL, 0, 0, false } -/// Structured decomposition of a normal-mode command, used two ways: +/// Represents any command. Projection of (`cmdarg_T`, `oparg_T`), which are stack-transient. +/// +/// 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 "non-prepped" command ("u", motions) has no body, so @@ -43,20 +45,16 @@ typedef struct { int cmd; ///< Command/motion char ('J', 'p', 'f', K_LEFT, …; 0 = none) int cmd2; ///< Second char of a two-char command name ("gJ" => 'J'; 0 = none) int arg; ///< Operand ("fx" => 'x', "ma" => 'a'; 0 = none) + StringBuilder body; ///< Captured cmd "body": keys without the `["x][count]` "prefix", updated + ///< as the cmd executes (redo_append_xx); redo_keys() prepends the prefix. + ///< Empty in spec-only contexts (i.e. not a "redo" buf). } CmdSpec; -/// 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; ///< "Metadata", except reg/count provide the "prefix". - StringBuilder keys; ///< Cmd body. Perf: StringBuilder (not buffheader_T) => fewer allocs/copies. -} RedoBuf; - -/// RedoBuf (dot-repeat) state: the pending change atom and the previous one. +/// Dot-repeat state: the pending change and the previous one. /// Also the "save" shape for preserving it across user code (save_redobuff()). typedef struct { - RedoBuf cur; ///< Pending change: "." replays it (start_redo()). - RedoBuf old; ///< Last-but-one change: an insert session's own prep moved the previous change + CmdSpec cur; ///< Pending change: "." replays it (start_redo()). + CmdSpec old; ///< Last-but-one change: an insert session's own prep moved the previous change ///< here (redo_new()); "i_CTRL-O ." replays it. } RedoState; diff --git a/src/nvim/insert.c b/src/nvim/insert.c index 198532b81a..8c843a003b 100644 --- a/src/nvim/insert.c +++ b/src/nvim/insert.c @@ -334,7 +334,7 @@ static void insert_enter(InsertState *s) // Get the current length of the redo buffer, those characters have to be // skipped if we want to get to the inserted characters. - String redo = redo_keys(); + String redo = redo_keys(NULL); Ins.new_insert_skip = (int)redo.size; if (redo.data != NULL) { xfree(redo.data); @@ -2235,7 +2235,7 @@ int stop_arrow(void) } else { // Cursor-move was captured (start_arrow()): the atom mc-cascade will replay it. // Only `last_insert` (the ". register, i_CTRL-A) restarts here, like Vim. - String redo = redo_keys(); + String redo = redo_keys(NULL); Ins.new_insert_skip = (int)redo.size; xfree(redo.data); } @@ -2274,7 +2274,7 @@ static void stop_insert(pos_T *end_insert_pos, int esc, int nomove) // Save the inserted text for later redo with ^@ and CTRL-A. // Don't do it when "restart_edit" was set and nothing was inserted, // otherwise CTRL-O w and then will clear "last_insert". - String redo = redo_keys(); + String redo = redo_keys(NULL); int added = redo.data == NULL ? 0 : (int)redo.size - Ins.new_insert_skip; if (Ins.did_restart_edit == 0 || added > 0) { xfree(last_insert.data); diff --git a/src/nvim/normal_defs.h b/src/nvim/normal_defs.h index dd390ad960..48a55d6772 100644 --- a/src/nvim/normal_defs.h +++ b/src/nvim/normal_defs.h @@ -74,8 +74,7 @@ typedef struct { colnr_T vcol; ///< number of cols or end column (MAXCOL: to end of line) } VisualExtent; -/// Visual/Select mode state, as one global "group" (Visual). Previously these were bare EXTERN -/// symbols in globals.h; grouped here to make subsystem ownership explicit. +/// Visual/Select mode state, as one global "group" (Visual). typedef struct { pos_T start; ///< Start position of the active Visual selection. bool active; ///< Whether Visual mode is active.