Commit: patch 9.2.0970: buffered listener cannot obtain the text of a change
Christian Brabandt <[email protected]>
| Newsgroups | gmane.editors.vim.devel |
|---|---|
| Message-ID | <[email protected]> |
patch 9.2.0970: buffered listener cannot obtain the text of a change Commit: https://github.com/vim/vim/commit/6362cc82bbf01b498da37036ec76ffa987b85e4e Author: Hirohito Higashi <[email protected]> Date: Tue Aug 18 19:34:57 2026 +0000 patch 9.2.0970: buffered listener cannot obtain the text of a change Problem: A callback added with listener_add() is given the line numbers of a change but not the text. When changes are buffered, two changes to the same line do not change the line count and are reported together, so getbufline() can only return the result of the second one. Solution: Let listener_add() take a Dictionary of options and add a "text" option, which stores the resulting text in each item of the list of changes. The text is copied when the change is recorded, so it is not affected by later changes. Invoke the callback early when a lot of text has been recorded, to bound the memory used. related: #19621 closes: #21071 Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Signed-off-by: Hirohito Higashi <[email protected]> Signed-off-by: Christian Brabandt <[email protected]> diff --git a/runtime/doc/builtin.txt b/runtime/doc/builtin.txt index d88d5deac..11934bcfc 100644 --- a/runtime/doc/builtin.txt +++ b/runtime/doc/builtin.txt @@ -1,4 +1,4 @@ -*builtin.txt* For Vim version 9.2. Last change: 2026 Aug 11 +*builtin.txt* For Vim version 9.2. Last change: 2026 Aug 18 VIM REFERENCE MANUAL by Bram Moolenaar @@ -390,7 +390,7 @@ lispindent({lnum}) Number Lisp indent for line {lnum} list2blob({list}) Blob turn {list} of numbers into a Blob list2str({list} [, {utf8}]) String turn {list} of numbers into a String list2tuple({list}) Tuple turn {list} of items into a tuple -listener_add({callback} [, {buf} [, {unbuffered}]]) +listener_add({callback} [, {buf} [, {options}]]) Number add a callback to listen to changes listener_flush([{buf}]) none invoke listener callbacks listener_remove({id}) Number remove a listener callback @@ -6811,7 +6811,7 @@ list2tuple({list}) *list2tuple()* Return type: tuple<{type}> (depending on the given |List|) -listener_add({callback} [, {buf} [, {unbuffered}]]) *listener_add()* +listener_add({callback} [, {buf} [, {options}]]) *listener_add()* Add a callback function that will be invoked when changes have been made to buffer {buf}. {buf} refers to a buffer name or number. For the accepted @@ -6848,6 +6848,8 @@ listener_add({callback} [, {buf} [, {unbuffered}]]) *listener_add()* the change; one if unknown or the whole line was affected; this is a byte index, first character has a value of one. + text only present when the "text" option is set + for at least one listener, see below When lines are inserted (not when a line is split, e.g. by typing CR in Insert mode) the values are: lnum line above which the new line is added @@ -6866,7 +6868,15 @@ listener_add({callback} [, {buf} [, {unbuffered}]]) *listener_add()* added 0 col first column with a change or 1 - When {unbuffered} is |FALSE| or not provided the {callback} is + {options} is a Dictionary with these optional entries: + unbuffered invoke the {callback} for every single change + instead of for a batch of changes + text include the resulting text as a |List| of + Strings in each item of "changes" + For backwards compatibility {options} may also be a |Boolean|, + which is then used for the "unbuffered" entry. + + When "unbuffered" is |FALSE| or not provided the {callback} is invoked: 1. Just before the screen is updated. @@ -6874,17 +6884,23 @@ listener_add({callback} [, {buf} [, {unbuffered}]]) *listener_add()* 3. When a change is being made that changes the line count in a way that causes a line number in the list of changes to become invalid. + 4. When "text" is set and a lot of text has been recorded. The entries are in the order the changes were made, thus the most recent change is at the end. + Reasons three and four make the {callback} run while another + command is executing, carrying the changes made before it. Do + not assume that what a {callback} receives was caused by the + command that is running. + Because of the third reason for triggering a callback listed above, the line numbers passed to the callback are not guaranteed to be valid. In particular, the end value can be - greater than line('$') + 1. If this is a problem then make - {unbuffered} |TRUE|. + greater than line('$') + 1. If this is a problem then set + "unbuffered". - When {unbuffered} is |TRUE| the {callback} is invoked for every + When "unbuffered" is |TRUE| the {callback} is invoked for every single change. The changes list only holds a single dictionary and the "start", "end" and "added" values in the dictionary are the same as the corresponding callback @@ -6892,6 +6908,35 @@ listener_add({callback} [, {buf} [, {unbuffered}]]) *listener_add()* invoked, but later changes may make them invalid, thus keeping a copy for later might not work. + *listener-text* + When "text" is set each item in "changes" gets a "text" entry, + a List with the lines that occupy the changed region right + after that change was made: the lines "lnum" up to but not + including "end" plus "added". This List is empty when the + change only deleted lines. Together with "lnum", "end" and + "added" the item then fully describes the change: replace the + lines "lnum" up to but not including "end" with "text". + + The text is copied at the moment the change is recorded, so it + is not affected by later changes. Without it a buffered + {callback} cannot reliably obtain the text of a change: two + changes to the same line do not change the line count and are + reported together, and by then |getbufline()| can only return + the result of the second one. Setting "unbuffered" avoids + that as well, but then the {callback} is invoked for every + single change, which is expensive for a command that changes + many lines. + + Setting "text" costs a copy of the changed lines for every + change. To keep the memory used by a long sequence of changes + bounded, the {callback} is also invoked when a lot of text has + been recorded. + + The recorded changes are kept per buffer and shared by all + listeners for that buffer. Therefore, when at least one + listener for the buffer sets "text", every {callback} for that + buffer gets the "text" entry. + The {callback} is invoked with the text locked, see |textlock|. If you do need to make changes to the buffer, use a timer to do this later |timer_start()|. diff --git a/runtime/doc/tags b/runtime/doc/tags index 41e7a1072..a8ff635fe 100644 --- a/runtime/doc/tags +++ b/runtime/doc/tags @@ -8942,6 +8942,7 @@ list-repeat windows.txt /*list-repeat* list2blob() builtin.txt /*list2blob()* list2str() builtin.txt /*list2str()* list2tuple() builtin.txt /*list2tuple()* +listener-text builtin.txt /*listener-text* listener_add() builtin.txt /*listener_add()* listener_flush() builtin.txt /*listener_flush()* listener_remove() builtin.txt /*listener_remove()* diff --git a/src/change.c b/src/change.c index c9e27c194..7edbd9b8d 100644 --- a/src/change.c +++ b/src/change.c @@ -207,6 +207,7 @@ clean_listener_list(buf_T *buf, listener_T **list, bool all) { list_unref(buf->b_recorded_changes); buf->b_recorded_changes = NULL; + buf->b_recorded_text_size = 0; } } } @@ -267,6 +268,68 @@ check_recorded_changes( } } +// Amount of recorded text after which the listeners are invoked, to bound the +// memory used by a long sequence of changes. +# define LISTENER_TEXT_MAX (4 * 1024 * 1024) + +/* + * Return true when any listener in "list" asked for the resulting text. + */ + static bool +listeners_want_text(listener_T *list) +{ + listener_T *lnr; + + for (lnr = list; lnr != NULL; lnr = lnr->lr_next) + if (lnr->lr_text) + return true; + return false; +} + +/* + * Store in "dict" the text that occupies the changed region right after the + * change: the lines from "lnum" up to but not including "lnume" + "xtra". + * Returns the number of bytes stored. + */ + static size_t +add_change_text( + buf_T *buf, + dict_T *dict, + linenr_T lnum, + linenr_T lnume, + long xtra) +{ + list_T *l = list_alloc(); + linenr_T below = lnume + xtra; // line below the changed region + size_t size = 0; + + if (l == NULL) + return 0; + + if (below > buf->b_ml.ml_line_count + 1) + below = buf->b_ml.ml_line_count + 1; + for (linenr_T lp = lnum < 1 ? 1 : lnum; lp < below; ++lp) + { + char_u *line = ml_get_buf(buf, lp, FALSE); + colnr_T len = ml_get_buf_len(buf, lp); + + // Rather than storing a part of the text, store none of it. + if (list_append_string(l, line, len) == FAIL) + { + list_free(l); + return 0; + } + size += (size_t)len + 1; + } + + if (dict_add_list(dict, "text", l) == FAIL) + { + list_free(l); + return 0; + } + return size; +} + /* * Record a change for listeners added with listener_add(). * Always for the current buffer. @@ -306,8 +369,15 @@ may_record_change( dict_add_number(dict, "end", (varnumber_T)lnume); dict_add_number(dict, "added", (varnumber_T)xtra); dict_add_number(dict, "col", (varnumber_T)col + 1); + if (listeners_want_text(curbuf->b_listener)) + curbuf->b_recorded_text_size += + add_change_text(curbuf, dict, lnum, lnume, xtra); list_append_dict(curbuf->b_recorded_changes, dict); + + // Invoking the listeners resets the size, do not try while they are busy. + if (!recursive && curbuf->b_recorded_text_size > LISTENER_TEXT_MAX) + invoke_listeners(curbuf); } /* @@ -319,7 +389,8 @@ f_listener_add(typval_T *argvars, typval_T *rettv) callback_T callback; listener_T *lnr; buf_T *buf = curbuf; - int unbuffered = 0; + bool unbuffered = false; + bool want_text = false; if (check_secure()) return; @@ -332,7 +403,7 @@ f_listener_add(typval_T *argvars, typval_T *rettv) if (in_vim9script() && ( check_for_opt_buffer_arg(argvars, 1) == FAIL - || check_for_opt_bool_arg(argvars, 2) == FAIL)) + || check_for_opt_bool_or_dict_arg(argvars, 2) == FAIL)) return; callback = get_callback(&argvars[0]); @@ -347,8 +418,15 @@ f_listener_add(typval_T *argvars, typval_T *rettv) free_callback(&callback); return; } - if (argvars[2].v_type != VAR_UNKNOWN) - unbuffered = (int)tv_get_bool(&argvars[2]); + if (argvars[2].v_type == VAR_DICT) + { + dict_T *d = argvars[2].vval.v_dict; + + unbuffered = dict_get_bool(d, "unbuffered", false); + want_text = dict_get_bool(d, "text", false); + } + else if (argvars[2].v_type != VAR_UNKNOWN) + unbuffered = tv_get_bool(&argvars[2]); } lnr = ALLOC_CLEAR_ONE(listener_T); @@ -377,6 +455,7 @@ f_listener_add(typval_T *argvars, typval_T *rettv) set_callback(&lnr->lr_callback, &callback); + lnr->lr_text = want_text; lnr->lr_id = ++next_listener_id; rettv->vval.v_number = lnr->lr_id; } @@ -573,6 +652,8 @@ invoke_sync_listeners( dict_add_number(dict, "end", (varnumber_T)end); dict_add_number(dict, "added", (varnumber_T)added); dict_add_number(dict, "col", (varnumber_T)col + 1); + if (listeners_want_text(buf->b_sync_listener)) + (void)add_change_text(buf, dict, start, end, added); list_append_dict(recorded_changes, dict); invoke_listener_set( @@ -616,6 +697,7 @@ invoke_listeners(buf_T *buf) list_unref(buf->b_recorded_changes); buf->b_recorded_changes = NULL; + buf->b_recorded_text_size = 0; } /* diff --git a/src/evalfunc.c b/src/evalfunc.c index 92a316354..6820cd8c9 100644 --- a/src/evalfunc.c +++ b/src/evalfunc.c @@ -412,6 +412,20 @@ arg_bool_or_nr(type_T *type, type_T *decl_type UNUSED, argcontext_T *context) return FAIL; } +/* + * Check "type" is a bool or a dict of 'any'. + */ + static int +arg_bool_or_dict_any( + type_T *type, + type_T *decl_type UNUSED, + argcontext_T *context) +{ + if (type->tt_type == VAR_DICT || type_any_or_unknown(type)) + return OK; + return check_arg_type(&t_bool, type, context); +} + /* * Check "type" is a list of 'any' or a blob. */ @@ -1301,7 +1315,8 @@ static argcheck_T arg2_string_or_list_number[] = {arg_string_or_list_any, arg_nu static argcheck_T arg2_string_string_or_number[] = {arg_string, arg_string_or_nr}; static argcheck_T arg2_blob_dict[] = {arg_blob, arg_dict_any}; static argcheck_T arg2_list_or_tuple_string[] = {arg_list_or_tuple, arg_string}; -static argcheck_T arg3_any_buffer_bool[] = {arg_any, arg_buffer, arg_bool}; +static argcheck_T arg3_any_buffer_bool_or_dict[] = { + arg_any, arg_buffer, arg_bool_or_dict_any}; static argcheck_T arg3_any_list_dict[] = {arg_any, arg_list_any, arg_dict_any}; static argcheck_T arg3_buffer_lnum_lnum[] = {arg_buffer, arg_lnum, arg_lnum}; static argcheck_T arg3_buffer_number_number[] = {arg_buffer, arg_number, arg_number}; @@ -2515,7 +2530,7 @@ static const funcentry_T global_functions[] = ret_string, f_list2str}, {"list2tuple", 1, 1, FEARG_1, arg1_list_any, ret_tuple_any, f_list2tuple}, - {"listener_add", 1, 3, FEARG_2, arg3_any_buffer_bool, + {"listener_add", 1, 3, FEARG_2, arg3_any_buffer_bool_or_dict, ret_number, f_listener_add}, {"listener_flush", 0, 1, FEARG_1, arg1_buffer, ret_void, f_listener_flush}, diff --git a/src/proto/typval.pro b/src/proto/typval.pro index fcd9cd1c4..1b5eb2905 100644 --- a/src/proto/typval.pro +++ b/src/proto/typval.pro @@ -20,6 +20,7 @@ int check_for_float_or_nr_arg(typval_T *args, int idx); int check_for_bool_arg(typval_T *args, int idx); int check_for_opt_bool_arg(typval_T *args, int idx); int check_for_opt_bool_or_number_arg(typval_T *args, int idx); +int check_for_opt_bool_or_dict_arg(typval_T *args, int idx); int check_for_blob_arg(typval_T *args, int idx); int check_for_list_arg(typval_T *args, int idx); int check_for_nonnull_list_arg(typval_T *args, int idx); diff --git a/src/structs.h b/src/structs.h index d41cd182b..984acc1d9 100644 --- a/src/structs.h +++ b/src/structs.h @@ -2973,6 +2973,7 @@ struct listener_S { listener_T *lr_next; int lr_id; + bool lr_text; // include the resulting text in each change callback_T lr_callback; }; @@ -3633,6 +3634,7 @@ struct file_buffer listener_T *b_listener; // Listeners accepting buffered reports. listener_T *b_sync_listener; // Listeners requiring unbuffered reports. list_T *b_recorded_changes; + size_t b_recorded_text_size; // bytes of text held by the above #endif #ifdef FEAT_PROP_POPUP bool b_has_textprop; // true when text props were added diff --git a/src/testdir/test_listener.vim b/src/testdir/test_listener.vim index 6b91e8d04..b981fd72d 100644 --- a/src/testdir/test_listener.vim +++ b/src/testdir/test_listener.vim @@ -801,6 +801,185 @@ func Test_listener_blockwise_paste() bwipe! endfunc +func s:StoreChanges(l) + call add(s:changes, deepcopy(a:l)) +endfunc + +" The "text" option makes each change self-contained. +func Test_listener_text() + new + call setline(1, ['one', 'two', 'three']) + let s:changes = [] + let id = listener_add({b, s, e, a, l -> s:StoreChanges(l)}, + \ bufnr(), #{text: v:true}) + + " Two changes to the same line do not change the line count, so they are + " reported together. Each one still carries the text it resulted in. + call setline(1, 'first') + call setline(1, 'second') + call listener_flush() + call assert_equal([[ + \ {'lnum': 1, 'end': 2, 'col': 1, 'added': 0, 'text': ['first']}, + \ {'lnum': 1, 'end': 2, 'col': 1, 'added': 0, 'text': ['second']}]], + \ s:changes) + + " Inserted lines are reported as the text that was inserted. + let s:changes = [] + call append(1, ['a', 'b']) + call listener_flush() + call assert_equal([[ + \ {'lnum': 2, 'end': 2, 'col': 1, 'added': 2, 'text': ['a', 'b']}]], + \ s:changes) + + " Deleting lines leaves an empty text. + let s:changes = [] + 2,3del + call listener_flush() + call assert_equal([[ + \ {'lnum': 2, 'end': 4, 'col': 1, 'added': -2, 'text': []}]], + \ s:changes) + + " Deleting and inserting at the same spot keeps both entries apart. + let s:changes = [] + call setline(1, ['one', 'two', 'three']) + call listener_flush() + let s:changes = [] + 1del + call append(0, 'zero') + call listener_flush() + call assert_equal([ + \ [{'lnum': 1, 'end': 2, 'col': 1, 'added': -1, 'text': []}], + \ [{'lnum': 1, 'end': 1, 'col': 1, 'added': 1, 'text': ['zero']}]], + \ s:changes) + + call listener_remove(id) + bwipe! +endfunc + +" Deleting every line and undoing it are each reported as one change. +func Test_listener_text_whole_buffer() + new + call setline(1, range(1, 3000)->map({_, v -> 'line ' .. v})) + let s:changes = [] + let id = listener_add({b, s, e, a, l -> s:StoreChanges(l)}, + \ bufnr(), #{text: v:true}) + let &undolevels = &undolevels + + " Nothing is left, so no text is copied. + %delete _ + call listener_flush() + call assert_equal([[ + \ {'lnum': 1, 'end': 3001, 'col': 1, 'added': -3000, 'text': []}]], + \ s:changes) + + " The undo restores all the lines, so they are all copied. + let s:changes = [] + undo + call listener_flush() + call assert_equal(1, len(s:changes)) + call assert_equal(1, len(s:changes[0])) + let change = s:changes[0][0] + call assert_equal(1, change.lnum) + call assert_equal(2, change.end) + call assert_equal(2999, change.added) + call assert_equal(3000, len(change.text)) + call assert_equal('line 1', change.text[0]) + call assert_equal('line 3000', change.text[-1]) + + call listener_remove(id) + bwipe! +endfunc + +" A lot of recorded text invokes the callback without waiting for a flush. +func Test_listener_text_size_limit() + new + call setline(1, 'one') + let s:changes = [] + let id = listener_add({b, s, e, a, l -> s:StoreChanges(l)}, + \ bufnr(), #{text: v:true}) + + " Below the limit the change is only reported when flushed. + call setline(1, repeat('x', 1024 * 1024)) + call assert_equal([], s:changes) + call listener_flush() + call assert_equal(1, len(s:changes)) + + " Above the limit the callback is invoked right away. + let s:changes = [] + call setline(1, repeat('y', 5 * 1024 * 1024)) + call assert_equal(1, len(s:changes)) + call assert_equal(1, len(s:changes[0])) + call assert_equal(5 * 1024 * 1024, len(s:changes[0][0].text[0])) + + " The size is reset, so the next change waits for a flush again. + let s:changes = [] + call setline(1, 'small') + call assert_equal([], s:changes) + call listener_flush() + call assert_equal(1, len(s:changes)) + + call listener_remove(id) + bwipe! +endfunc + +" The "text" option also applies to an unbuffered listener. +func Test_listener_text_unbuffered() + new + call setline(1, ['one', 'two', 'three']) + let s:changes = [] + let id = listener_add({b, s, e, a, l -> s:StoreChanges(l)}, + \ bufnr(), #{unbuffered: v:true, text: v:true}) + + call setline(2, 'two two') + call assert_equal([[ + \ {'lnum': 2, 'end': 3, 'col': 1, 'added': 0, 'text': ['two two']}]], + \ s:changes) + + call listener_remove(id) + bwipe! +endfunc + +" Without the "text" option there is no "text" entry. +func Test_listener_no_text_by_default() + new + call setline(1, ['one', 'two']) + let s:changes = [] + let id = listener_add({b, s, e, a, l -> s:StoreChanges(l)}, + \ bufnr(), #{unbuffered: v:true}) + + call setline(1, 'one one') + call assert_equal([[ + \ {'lnum': 1, 'end': 2, 'col': 1, 'added': 0}]], s:changes) + + call listener_remove(id) + bwipe! +endfunc + +" The third argument keeps accepting a Boolean. +func Test_listener_options_argument() + new + call setline(1, ['one', 'two']) + let s:changes = [] + let id = listener_add({b, s, e, a, l -> s:StoreChanges(l)}, bufnr(), v:true) + call setline(1, 'one one') + call assert_equal([[ + \ {'lnum': 1, 'end': 2, 'col': 1, 'added': 0}]], s:changes) + call listener_remove(id) + + " An empty Dictionary is the same as not passing the argument. + let s:changes = [] + let id = listener_add({b, s, e, a, l -> s:StoreChanges(l)}, bufnr(), {}) + call setline(1, 'one two') + call assert_equal([], s:changes) + call listener_flush() + call assert_equal([[ + \ {'lnum': 1, 'end': 2, 'col': 1, 'added': 0}]], s:changes) + call listener_remove(id) + + call assert_fails('call listener_add("Foo", bufnr(), [])', 'E745:') + bwipe! +endfunc + func Test_listener_add_in_sandbox() call assert_fails( \ 'sandbox call redraw_listener_add({"on_start": function("tr")})', diff --git a/src/testdir/test_vim9_builtin.vim b/src/testdir/test_vim9_builtin.vim index 350a790fb..49ab862d1 100644 --- a/src/testdir/test_vim9_builtin.vim +++ b/src/testdir/test_vim9_builtin.vim @@ -2657,6 +2657,7 @@ enddef def Test_listener_add() v9.CheckSourceDefAndScriptFailure(['listener_add("1", true)'], ['E1013: Argument 2: type mismatch, expected string but got bool', 'E1220: String or Number required for argument 2']) + v9.CheckSourceDefAndScriptFailure(['listener_add("1", 1, [1])'], ['E1013: Argument 3: type mismatch, expected bool but got list<number>', 'E1212: Bool required for argument 3']) enddef def Test_listener_flush() diff --git a/src/typval.c b/src/typval.c index f704f039c..6a4947d8a 100644 --- a/src/typval.c +++ b/src/typval.c @@ -577,6 +577,18 @@ check_for_opt_bool_or_number_arg(typval_T *args, int idx) return check_for_bool_or_number_arg(args, idx); } +/* + * Check for an optional bool or dict argument at 'idx'. + * Return FAIL if the type is wrong. + */ + int +check_for_opt_bool_or_dict_arg(typval_T *args, int idx) +{ + if (args[idx].v_type == VAR_UNKNOWN || args[idx].v_type == VAR_DICT) + return OK; + return check_for_bool_arg(args, idx); +} + /* * Give an error and return FAIL unless "args[idx]" is a blob. */ diff --git a/src/version.c b/src/version.c index 7f82d187b..921e57820 100644 --- a/src/version.c +++ b/src/version.c @@ -763,6 +763,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ +/**/ + 970, /**/ 969, /**/ -- -- You received this message from the "vim_dev" maillist. Do not top-post! Type your reply below the text you are replying to. For more information, visit http://www.vim.org/maillist.php --- You received this message because you are subscribed to the Google Groups "vim_dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion visit https://groups.google.com/d/msgid/vim_dev/E1wwPk8-00AeOn-U5%40256bit.org.