From 9be19b770d7802e62ab1a084996ac2192b71fd70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Linse?= Date: Fri, 7 Feb 2020 19:54:29 +0100 Subject: [PATCH 1/5] api: set_text: first stab at nvim_buf_set_text --- src/nvim/api/buffer.c | 188 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 171 insertions(+), 17 deletions(-) diff --git a/src/nvim/api/buffer.c b/src/nvim/api/buffer.c index f1151d196a..bc84e2144b 100644 --- a/src/nvim/api/buffer.c +++ b/src/nvim/api/buffer.c @@ -362,22 +362,9 @@ void nvim_buf_set_lines(uint64_t channel_id, return; } - for (size_t i = 0; i < replacement.size; i++) { - if (replacement.items[i].type != kObjectTypeString) { - api_set_error(err, - kErrorTypeValidation, - "All items in the replacement array must be strings"); - return; - } - // Disallow newlines in the middle of the line. - if (channel_id != VIML_INTERNAL_CALL) { - const String l = replacement.items[i].data.string; - if (memchr(l.data, NL, l.size)) { - api_set_error(err, kErrorTypeValidation, - "String cannot contain newlines"); - return; - } - } + bool disallow_nl = (channel_id != VIML_INTERNAL_CALL); + if (!check_string_array(replacement, disallow_nl, err)) { + return; } size_t new_len = replacement.size; @@ -466,7 +453,7 @@ void nvim_buf_set_lines(uint64_t channel_id, // Adjust marks. Invalidate any which lie in the // changed range, and move any in the remainder of the buffer. - // Only adjust marks if we managed to switch to a window that holds + // Only adjust mapks if we managed to switch to a window that holds // the buffer, otherwise line numbers will be invalid. mark_adjust((linenr_T)start, (linenr_T)(end - 1), @@ -487,6 +474,173 @@ end: try_end(err); } +void nvim_buf_set_text(uint64_t channel_id, + Buffer buffer, + Integer start_row, + Integer start_col, + Integer end_row, + Integer end_col, + ArrayOf(String) replacement, + Error *err) + FUNC_API_SINCE(7) +{ + // TODO: treat [] as [''] for convenience + assert(replacement.size > 0); + buf_T *buf = find_buffer_by_handle(buffer, err); + if (!buf) { + return; + } + size_t new_len = replacement.size; + + // TODO: do the nl-for-NUL dance as well? + bool disallow_nl = (channel_id != VIML_INTERNAL_CALL); + if (!check_string_array(replacement, disallow_nl, err)) { + return; + } + + // TODO: check range is ordered and everything! + // start_row, end_row within buffer len (except add text past the end?) + char *str_at_start = (char *)ml_get_buf(buf, start_row+1, false); + if (start_col < 0 || (size_t)start_col > strlen(str_at_start)) { + api_set_error(err, kErrorTypeValidation, "start_col out of bounds"); + return; + } + + char *str_at_end = (char *)ml_get_buf(buf, end_row+1, false); + size_t len_at_end = strlen(str_at_end); + if (end_col < 0 || (size_t)end_col > len_at_end) { + api_set_error(err, kErrorTypeValidation, "end_col out of bounds"); + return; + } + + String first_item = replacement.items[0].data.string; + String last_item = replacement.items[replacement.size-1].data.string; + size_t firstlen = (size_t)start_col+first_item.size; + size_t last_part_len = strlen(str_at_end) - (size_t)end_col; + if (replacement.size == 1) { + firstlen += last_part_len; + } + char *first = xmallocz(firstlen), *last = NULL; + memcpy(first, str_at_start, (size_t)start_col); + memcpy(first+start_col, first_item.data, first_item.size); + if (replacement.size == 1) { + memcpy(first+start_col+first_item.size, str_at_end+end_col, last_part_len); + } else { + last = xmallocz(last_item.size+last_part_len); + memcpy(last, last_item.data, last_item.size); + memcpy(last+last_item.size, str_at_end+end_col, last_part_len); + } + + char **lines = (new_len != 0) ? xcalloc(new_len, sizeof(char *)) : NULL; + lines[0] = first; + for (size_t i = 1; i < new_len-1; i++) { + lines[i] = replacement.items[i].data.string.data; + } + if (replacement.size > 1) { + lines[replacement.size-1] = last; + } + + try_start(); + aco_save_T aco; + aucmd_prepbuf(&aco, (buf_T *)buf); + + if (!MODIFIABLE(buf)) { + api_set_error(err, kErrorTypeException, "Buffer is not 'modifiable'"); + goto end; + } + + // TODO: dubbel KOLLA KOLLA indexen här + if (u_save((linenr_T)start_row, (linenr_T)end_row+2) == FAIL) { + api_set_error(err, kErrorTypeException, "Failed to save undo information"); + goto end; + } + + + ptrdiff_t extra = 0; // lines added to text, can be negative + size_t old_len = (size_t)(end_row-start_row+1); + + // If the size of the range is reducing (ie, new_len < old_len) we + // need to delete some old_len. We do this at the start, by + // repeatedly deleting line "start". + size_t to_delete = (new_len < old_len) ? (size_t)(old_len - new_len) : 0; + for (size_t i = 0; i < to_delete; i++) { + if (ml_delete((linenr_T)start_row+1, false) == FAIL) { + api_set_error(err, kErrorTypeException, "Failed to delete line"); + goto end; + } + } + + if (to_delete > 0) { + extra -= (ptrdiff_t)to_delete; + } + + // For as long as possible, replace the existing old_len with the + // new old_len. This is a more efficient operation, as it requires + // less memory allocation and freeing. + size_t to_replace = old_len < new_len ? old_len : new_len; + for (size_t i = 0; i < to_replace; i++) { + int64_t lnum = start_row + 1 + (int64_t)i; + + if (lnum >= MAXLNUM) { + api_set_error(err, kErrorTypeValidation, "Index value is too high"); + goto end; + } + + if (ml_replace((linenr_T)lnum, (char_u *)lines[i], false) == FAIL) { + api_set_error(err, kErrorTypeException, "Failed to replace line"); + goto end; + } + // Mark lines that haven't been passed to the buffer as they need + // to be freed later + lines[i] = NULL; + } + + // Now we may need to insert the remaining new old_len + for (size_t i = to_replace; i < new_len; i++) { + int64_t lnum = start_row + (int64_t)i; + + if (lnum >= MAXLNUM) { + api_set_error(err, kErrorTypeValidation, "Index value is too high"); + goto end; + } + + if (ml_append((linenr_T)lnum, (char_u *)lines[i], 0, false) == FAIL) { + api_set_error(err, kErrorTypeException, "Failed to insert line"); + goto end; + } + + // Same as with replacing, but we also need to free lines + xfree(lines[i]); + lines[i] = NULL; + extra++; + } + + // Adjust marks. Invalidate any which lie in the + // changed range, and move any in the remainder of the buffer. + // Only adjust mapks if we managed to switch to a window that holds + // the buffer, otherwise line numbers will be invalid. + mark_adjust((linenr_T)start_row, + (linenr_T)end_row, + MAXLNUM, + (long)extra, + kExtmarkNOOP); + + colnr_T col_extent = (colnr_T)(end_col + - ((end_row > start_col) ? start_col : 0)); + extmark_splice(buf, (int)start_row, (colnr_T)start_col, + (int)(end_row-start_row), col_extent, + (int)new_len-1, (colnr_T)last_item.size, kExtmarkUndo); + + changed_lines((linenr_T)start_row+1, 0, (linenr_T)end_row+1, (long)extra, true); + + // TODO: adjust cursor like an extmark ( i e it was inside last_part_len) + fix_cursor((linenr_T)start_row+1, (linenr_T)end_row+1, (linenr_T)extra); + +end: + aucmd_restbuf(&aco); + try_end(err); +} + /// Returns the byte offset of a line (0-indexed). |api-indexing| /// /// Line 1 (index=0) has offset 0. UTF-8 bytes are counted. EOL is one byte. From 29ad2ebc1688176a1c7acaa81103dac289de0ad1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bla=C5=BE=20Hrastnik?= Date: Mon, 4 May 2020 10:30:59 +0900 Subject: [PATCH 2/5] api: set_text: fix validation and some issues fix double free because intermediary lines weren't xmemdup'd. NL-for-NUL dance. Normalize row indices and perform more validation. Adjust the cursor position if it's on the right side of the replacement. Tests and documentation. --- src/nvim/api/buffer.c | 93 ++++++++++++++++++++++------- test/functional/api/buffer_spec.lua | 83 +++++++++++++++++++++++++ 2 files changed, 154 insertions(+), 22 deletions(-) diff --git a/src/nvim/api/buffer.c b/src/nvim/api/buffer.c index bc84e2144b..7fa30e7a7d 100644 --- a/src/nvim/api/buffer.c +++ b/src/nvim/api/buffer.c @@ -453,7 +453,7 @@ void nvim_buf_set_lines(uint64_t channel_id, // Adjust marks. Invalidate any which lie in the // changed range, and move any in the remainder of the buffer. - // Only adjust mapks if we managed to switch to a window that holds + // Only adjust marks if we managed to switch to a window that holds // the buffer, otherwise line numbers will be invalid. mark_adjust((linenr_T)start, (linenr_T)(end - 1), @@ -474,6 +474,23 @@ end: try_end(err); } +/// Sets (replaces) a range in the buffer. +/// +/// Indexing is zero-based, end-exclusive. +/// +/// To insert text at a given index, set `start` and `end` ranges to the same +/// index. To delete a range, set `replacement` to an empty array. +/// +/// Prefer nvim_buf_set_lines when modifying entire lines. +/// +/// @param channel_id +/// @param buffer Buffer handle, or 0 for current buffer +/// @param start_row First line index +/// @param start_column Last column +/// @param end_row Last line index (exclusive) +/// @param end_column Last column +/// @param replacement Array of lines to use as replacement +/// @param[out] err Error details, if any void nvim_buf_set_text(uint64_t channel_id, Buffer buffer, Integer start_row, @@ -490,29 +507,48 @@ void nvim_buf_set_text(uint64_t channel_id, if (!buf) { return; } - size_t new_len = replacement.size; - // TODO: do the nl-for-NUL dance as well? - bool disallow_nl = (channel_id != VIML_INTERNAL_CALL); - if (!check_string_array(replacement, disallow_nl, err)) { + bool oob = false; + + // check range is ordered and everything! + // start_row, end_row within buffer len (except add text past the end?) + start_row = normalize_index(buf, start_row, &oob); + if (oob) { + api_set_error(err, kErrorTypeValidation, "start_row out of bounds"); return; } - // TODO: check range is ordered and everything! - // start_row, end_row within buffer len (except add text past the end?) - char *str_at_start = (char *)ml_get_buf(buf, start_row+1, false); + end_row = normalize_index(buf, end_row, &oob); + /* if (oob) { */ + /* api_set_error(err, kErrorTypeValidation, "end_row out of bounds"); */ + /* return; */ + /* } */ + + char *str_at_start = (char *)ml_get_buf(buf, start_row, false); if (start_col < 0 || (size_t)start_col > strlen(str_at_start)) { api_set_error(err, kErrorTypeValidation, "start_col out of bounds"); return; } - char *str_at_end = (char *)ml_get_buf(buf, end_row+1, false); + char *str_at_end = (char *)ml_get_buf(buf, end_row, false); size_t len_at_end = strlen(str_at_end); if (end_col < 0 || (size_t)end_col > len_at_end) { api_set_error(err, kErrorTypeValidation, "end_col out of bounds"); return; } + if (start_row > end_row || (end_row == start_row && start_col > end_col)) { + api_set_error(err, kErrorTypeValidation, "start is higher than end"); + return; + } + + bool disallow_nl = (channel_id != VIML_INTERNAL_CALL); + if (!check_string_array(replacement, disallow_nl, err)) { + return; + } + + size_t new_len = replacement.size; + String first_item = replacement.items[0].data.string; String last_item = replacement.items[replacement.size-1].data.string; size_t firstlen = (size_t)start_col+first_item.size; @@ -523,18 +559,26 @@ void nvim_buf_set_text(uint64_t channel_id, char *first = xmallocz(firstlen), *last = NULL; memcpy(first, str_at_start, (size_t)start_col); memcpy(first+start_col, first_item.data, first_item.size); + memchrsub(first+start_col, NUL, NL, first_item.size); if (replacement.size == 1) { memcpy(first+start_col+first_item.size, str_at_end+end_col, last_part_len); } else { last = xmallocz(last_item.size+last_part_len); memcpy(last, last_item.data, last_item.size); + memchrsub(last, NUL, NL, last_item.size); memcpy(last+last_item.size, str_at_end+end_col, last_part_len); } char **lines = (new_len != 0) ? xcalloc(new_len, sizeof(char *)) : NULL; lines[0] = first; for (size_t i = 1; i < new_len-1; i++) { - lines[i] = replacement.items[i].data.string.data; + const String l = replacement.items[i].data.string; + + // Fill lines[i] with l's contents. Convert NULs to newlines as required by + // NL-used-for-NUL. + lines[i] = xmemdupz(l.data, l.size); + memchrsub(lines[i], NUL, NL, l.size); + /* lines[i] = replacement.items[i].data.string.data; */ } if (replacement.size > 1) { lines[replacement.size-1] = last; @@ -549,13 +593,11 @@ void nvim_buf_set_text(uint64_t channel_id, goto end; } - // TODO: dubbel KOLLA KOLLA indexen här - if (u_save((linenr_T)start_row, (linenr_T)end_row+2) == FAIL) { + if (u_save((linenr_T)start_row - 1, (linenr_T)end_row) == FAIL) { api_set_error(err, kErrorTypeException, "Failed to save undo information"); goto end; } - ptrdiff_t extra = 0; // lines added to text, can be negative size_t old_len = (size_t)(end_row-start_row+1); @@ -564,7 +606,7 @@ void nvim_buf_set_text(uint64_t channel_id, // repeatedly deleting line "start". size_t to_delete = (new_len < old_len) ? (size_t)(old_len - new_len) : 0; for (size_t i = 0; i < to_delete; i++) { - if (ml_delete((linenr_T)start_row+1, false) == FAIL) { + if (ml_delete((linenr_T)start_row, false) == FAIL) { api_set_error(err, kErrorTypeException, "Failed to delete line"); goto end; } @@ -579,7 +621,7 @@ void nvim_buf_set_text(uint64_t channel_id, // less memory allocation and freeing. size_t to_replace = old_len < new_len ? old_len : new_len; for (size_t i = 0; i < to_replace; i++) { - int64_t lnum = start_row + 1 + (int64_t)i; + int64_t lnum = start_row + (int64_t)i; if (lnum >= MAXLNUM) { api_set_error(err, kErrorTypeValidation, "Index value is too high"); @@ -597,7 +639,7 @@ void nvim_buf_set_text(uint64_t channel_id, // Now we may need to insert the remaining new old_len for (size_t i = to_replace; i < new_len; i++) { - int64_t lnum = start_row + (int64_t)i; + int64_t lnum = start_row + (int64_t)i - 1; if (lnum >= MAXLNUM) { api_set_error(err, kErrorTypeValidation, "Index value is too high"); @@ -617,7 +659,7 @@ void nvim_buf_set_text(uint64_t channel_id, // Adjust marks. Invalidate any which lie in the // changed range, and move any in the remainder of the buffer. - // Only adjust mapks if we managed to switch to a window that holds + // Only adjust marks if we managed to switch to a window that holds // the buffer, otherwise line numbers will be invalid. mark_adjust((linenr_T)start_row, (linenr_T)end_row, @@ -626,17 +668,24 @@ void nvim_buf_set_text(uint64_t channel_id, kExtmarkNOOP); colnr_T col_extent = (colnr_T)(end_col - - ((end_row > start_col) ? start_col : 0)); - extmark_splice(buf, (int)start_row, (colnr_T)start_col, + - ((end_col > start_col) ? start_col : 0)); + extmark_splice(buf, (int)start_row-1, (colnr_T)start_col, (int)(end_row-start_row), col_extent, (int)new_len-1, (colnr_T)last_item.size, kExtmarkUndo); - changed_lines((linenr_T)start_row+1, 0, (linenr_T)end_row+1, (long)extra, true); + changed_lines((linenr_T)start_row, 0, (linenr_T)end_row, (long)extra, true); - // TODO: adjust cursor like an extmark ( i e it was inside last_part_len) - fix_cursor((linenr_T)start_row+1, (linenr_T)end_row+1, (linenr_T)extra); + // adjust cursor like an extmark ( i e it was inside last_part_len) + if (curwin->w_cursor.lnum == end_row && curwin->w_cursor.col > end_col) { + curwin->w_cursor.col -= col_extent - (colnr_T)last_item.size; + } + fix_cursor((linenr_T)start_row, (linenr_T)end_row, (linenr_T)extra); end: + for (size_t i = 0; i < new_len; i++) { + xfree(lines[i]); + } + xfree(lines); aucmd_restbuf(&aco); try_end(err); } diff --git a/test/functional/api/buffer_spec.lua b/test/functional/api/buffer_spec.lua index 8ed642b43e..60ff246436 100644 --- a/test/functional/api/buffer_spec.lua +++ b/test/functional/api/buffer_spec.lua @@ -392,6 +392,89 @@ describe('api/buf', function() end) end) + describe('nvim_buf_get_lines, nvim_buf_set_text', function() + local get_lines, set_text = curbufmeths.get_lines, curbufmeths.set_text + local line_count = curbufmeths.line_count + + it('works', function() + insert([[ + hello foo! + text + ]]) + + eq({'hello foo!'}, get_lines(0, 1, true)) + + + -- can replace a single word + set_text(0, 6, 0, 9, {'world'}) + eq({'hello world!', 'text'}, get_lines(0, 2, true)) + + -- can replace with multiple lines + local err = set_text(0, 6, 0, 11, {'foo', 'wo', 'more'}) + eq({'hello foo', 'wo', 'more!', 'text'}, get_lines(0, 4, true)) + + -- will join multiple lines if needed + set_text(0, 6, 3, 4, {'bar'}) + eq({'hello bar'}, get_lines(0, 1, true)) + end) + + it('updates the cursor position', function() + insert([[ + hello world! + ]]) + + -- position the cursor on `!` + curwin('set_cursor', {1, 11}) + -- replace 'world' with 'foo' + set_text(0, 6, 0, 11, {'foo'}) + eq('hello foo!', curbuf_depr('get_line', 0)) + -- cursor should be moved left by two columns (replacement is shorter by 2 chars) + eq({1, 9}, curwin('get_cursor')) + end) + + it('can handle NULs', function() + set_text(0, 0, 0, 0, {'ab\0cd'}) + eq('ab\0cd', curbuf_depr('get_line', 0)) + end) + + it('adjusts extmarks', function() + local ns = request('nvim_create_namespace', "my-fancy-plugin") + insert([[ + foo bar + baz + ]]) + local id1 = curbufmeths.set_extmark(ns, 0, 0, 1, {}) + local id2 = curbufmeths.set_extmark(ns, 0, 0, 7, {}) + local id3 = curbufmeths.set_extmark(ns, 0, 1, 1, {}) + set_text(0, 4, 0, 7, {"q"}) + + -- TODO: if we set text at 0,3, what happens to the mark at 0,3 + + eq({'foo q', 'baz'}, get_lines(0, 2, true)) + -- mark before replacement point is unaffected + rv = curbufmeths.get_extmark_by_id(ns, id1) + eq({0, 1}, rv) + -- mark gets shifted back because the replacement was shorter + rv = curbufmeths.get_extmark_by_id(ns, id2) + eq({0, 5}, rv) + -- mark on the next line is unaffected + rv = curbufmeths.get_extmark_by_id(ns, id3) + eq({1, 1}, rv) + + -- replacing the text spanning two lines will adjust the mark on the next line + set_text(0, 3, 1, 3, {"qux"}) + rv = curbufmeths.get_extmark_by_id(ns, id3) + eq({'fooqux', ''}, get_lines(0, 2, true)) + eq({0, 6}, rv) + -- but mark before replacement point is still unaffected + rv = curbufmeths.get_extmark_by_id(ns, id1) + eq({0, 1}, rv) + -- and the mark in the middle was shifted to the end of the insertion + rv = curbufmeths.get_extmark_by_id(ns, id2) + eq({0, 6}, rv) + end) + end) + describe('nvim_buf_get_offset', function() local get_offset = curbufmeths.get_offset it('works', function() From 45b14f88db1b332938f4a73be28966ae60563a50 Mon Sep 17 00:00:00 2001 From: Tony Chen Date: Mon, 21 Dec 2020 17:51:52 -0800 Subject: [PATCH 3/5] api: set_text: rebase, update to new api, and add more tests --- src/nvim/api/buffer.c | 66 +++++++++++++++++++++++++---- src/nvim/extmark.c | 17 ++++++++ test/functional/api/buffer_spec.lua | 63 +++++++++++++++++++++++---- 3 files changed, 129 insertions(+), 17 deletions(-) diff --git a/src/nvim/api/buffer.c b/src/nvim/api/buffer.c index 7fa30e7a7d..21a7e897a9 100644 --- a/src/nvim/api/buffer.c +++ b/src/nvim/api/buffer.c @@ -309,6 +309,27 @@ end: return rv; } +static bool check_string_array(Array arr, bool disallow_nl, Error *err) +{ + for (size_t i = 0; i < arr.size; i++) { + if (arr.items[i].type != kObjectTypeString) { + api_set_error(err, + kErrorTypeValidation, + "All items in the replacement array must be strings"); + return false; + } + // Disallow newlines in the middle of the line. + if (disallow_nl) { + const String l = arr.items[i].data.string; + if (memchr(l.data, NL, l.size)) { + api_set_error(err, kErrorTypeValidation, + "String cannot contain newlines"); + return false; + } + } + } + return true; +} /// Sets (replaces) a line-range in the buffer. /// @@ -519,10 +540,10 @@ void nvim_buf_set_text(uint64_t channel_id, } end_row = normalize_index(buf, end_row, &oob); - /* if (oob) { */ - /* api_set_error(err, kErrorTypeValidation, "end_row out of bounds"); */ - /* return; */ - /* } */ + if (oob) { + api_set_error(err, kErrorTypeValidation, "end_row out of bounds"); + return; + } char *str_at_start = (char *)ml_get_buf(buf, start_row, false); if (start_col < 0 || (size_t)start_col > strlen(str_at_start)) { @@ -547,10 +568,36 @@ void nvim_buf_set_text(uint64_t channel_id, return; } + bcount_t new_byte = 0; + bcount_t old_byte = 0; + + // calculate byte size of old region before it gets modified/deleted + if (start_row == end_row) { + old_byte = (bcount_t)end_col - start_col; + } else { + char_u *line; + old_byte += (bcount_t)strlen(str_at_start) - start_col; + for (size_t i = 0; i < (bcount_t)end_row - start_row; i++){ + int64_t lnum = start_row + (int64_t)i; + + if (lnum >= MAXLNUM) { + api_set_error(err, kErrorTypeValidation, "Index value is too high"); + goto end; + } + + line = ml_get_buf(buf, lnum, false); + old_byte += (bcount_t)(strlen((char *)line)); + } + old_byte += end_col; + } + size_t new_len = replacement.size; String first_item = replacement.items[0].data.string; String last_item = replacement.items[replacement.size-1].data.string; + + new_byte += (bcount_t)(first_item.size); + size_t firstlen = (size_t)start_col+first_item.size; size_t last_part_len = strlen(str_at_end) - (size_t)end_col; if (replacement.size == 1) { @@ -578,10 +625,11 @@ void nvim_buf_set_text(uint64_t channel_id, // NL-used-for-NUL. lines[i] = xmemdupz(l.data, l.size); memchrsub(lines[i], NUL, NL, l.size); - /* lines[i] = replacement.items[i].data.string.data; */ + new_byte += (bcount_t)(l.size); } if (replacement.size > 1) { lines[replacement.size-1] = last; + new_byte += (bcount_t)(last_item.size); } try_start(); @@ -593,7 +641,7 @@ void nvim_buf_set_text(uint64_t channel_id, goto end; } - if (u_save((linenr_T)start_row - 1, (linenr_T)end_row) == FAIL) { + if (u_save((linenr_T)start_row - 1, (linenr_T)end_row + 1) == FAIL) { api_set_error(err, kErrorTypeException, "Failed to save undo information"); goto end; } @@ -670,8 +718,10 @@ void nvim_buf_set_text(uint64_t channel_id, colnr_T col_extent = (colnr_T)(end_col - ((end_col > start_col) ? start_col : 0)); extmark_splice(buf, (int)start_row-1, (colnr_T)start_col, - (int)(end_row-start_row), col_extent, - (int)new_len-1, (colnr_T)last_item.size, kExtmarkUndo); + (int)(end_row-start_row), col_extent, (bcount_t)(old_byte), + (int)new_len-1, (colnr_T)last_item.size, (bcount_t)new_byte, + kExtmarkUndo); + changed_lines((linenr_T)start_row, 0, (linenr_T)end_row, (long)extra, true); diff --git a/src/nvim/extmark.c b/src/nvim/extmark.c index ba685b158e..b2d8532cd7 100644 --- a/src/nvim/extmark.c +++ b/src/nvim/extmark.c @@ -560,6 +560,23 @@ void extmark_adjust(buf_T *buf, new_row, 0, new_byte, undo); } +// Adjust extmarks following a text edit. +// +// @param buf +// @param start_row Start row of the region to be changed +// @param start_col Start col of the region to be changed +// @param old_row End row of the region to be changed. +// Encoded as an offset to start_row. +// @param old_col End col of the region to be changed. Encodes +// an offset from start_col if old_row = 0; otherwise, +// encodes the end column of the old region. +// @param old_byte Byte extent of the region to be changed. +// @param new_row Row offset of the new region. +// @param new_col Col offset of the new region. Encodes an offset from +// start_col if new_row = 0; otherwise, encodes +// the end column of the new region. +// @param new_byte Byte extent of the new region. +// @param undo void extmark_splice(buf_T *buf, int start_row, colnr_T start_col, int old_row, colnr_T old_col, bcount_t old_byte, diff --git a/test/functional/api/buffer_spec.lua b/test/functional/api/buffer_spec.lua index 60ff246436..dbe585a064 100644 --- a/test/functional/api/buffer_spec.lua +++ b/test/functional/api/buffer_spec.lua @@ -409,6 +409,14 @@ describe('api/buf', function() set_text(0, 6, 0, 9, {'world'}) eq({'hello world!', 'text'}, get_lines(0, 2, true)) + -- can insert text + set_text(0, 0, 0, 0, {'well '}) + eq({'well hello world!', 'text'}, get_lines(0, 2, true)) + + -- can delete text + set_text(0, 0, 0, 5, {''}) + eq({'hello world!', 'text'}, get_lines(0, 2, true)) + -- can replace with multiple lines local err = set_text(0, 6, 0, 11, {'foo', 'wo', 'more'}) eq({'hello foo', 'wo', 'more!', 'text'}, get_lines(0, 4, true)) @@ -418,6 +426,43 @@ describe('api/buf', function() eq({'hello bar'}, get_lines(0, 1, true)) end) + pending('can handle multibyte characters', function() + insert([[ + hellØ world! + ]]) + + eq({'hellØ world!'}, get_lines(0, 1, true)) + + -- inserting multibyte + set_text(0, 11, 0, 11, {'Ø'}) + eq({'hellØ worldØ!'}, get_lines(0, 1, true)) + + -- deleting multibyte + set_text(0, 0, 0, 6, {''}) + eq({'worldØ!'}, get_lines(0, 1, true)) + end) + + it('works with undo', function() + insert([[ + hello world! + ]]) + + -- setting text + set_text(0, 0, 0, 0, {'well '}) + feed('u') + eq({'hello world!'}, get_lines(0, 1, true)) + + -- deleting text + set_text(0, 0, 0, 6, {''}) + feed('u') + eq({'hello world!'}, get_lines(0, 1, true)) + + -- inserting newlines + set_text(0, 0, 0, 0, {'hello', 'mr '}) + feed('u') + eq({'hello world!'}, get_lines(0, 1, true)) + end) + it('updates the cursor position', function() insert([[ hello world! @@ -443,34 +488,34 @@ describe('api/buf', function() foo bar baz ]]) - local id1 = curbufmeths.set_extmark(ns, 0, 0, 1, {}) - local id2 = curbufmeths.set_extmark(ns, 0, 0, 7, {}) - local id3 = curbufmeths.set_extmark(ns, 0, 1, 1, {}) + local id1 = curbufmeths.set_extmark(ns, 0, 1, {}) + local id2 = curbufmeths.set_extmark(ns, 0, 7, {}) + local id3 = curbufmeths.set_extmark(ns, 1, 1, {}) set_text(0, 4, 0, 7, {"q"}) -- TODO: if we set text at 0,3, what happens to the mark at 0,3 eq({'foo q', 'baz'}, get_lines(0, 2, true)) -- mark before replacement point is unaffected - rv = curbufmeths.get_extmark_by_id(ns, id1) + rv = curbufmeths.get_extmark_by_id(ns, id1, {}) eq({0, 1}, rv) -- mark gets shifted back because the replacement was shorter - rv = curbufmeths.get_extmark_by_id(ns, id2) + rv = curbufmeths.get_extmark_by_id(ns, id2, {}) eq({0, 5}, rv) -- mark on the next line is unaffected - rv = curbufmeths.get_extmark_by_id(ns, id3) + rv = curbufmeths.get_extmark_by_id(ns, id3, {}) eq({1, 1}, rv) -- replacing the text spanning two lines will adjust the mark on the next line set_text(0, 3, 1, 3, {"qux"}) - rv = curbufmeths.get_extmark_by_id(ns, id3) + rv = curbufmeths.get_extmark_by_id(ns, id3, {}) eq({'fooqux', ''}, get_lines(0, 2, true)) eq({0, 6}, rv) -- but mark before replacement point is still unaffected - rv = curbufmeths.get_extmark_by_id(ns, id1) + rv = curbufmeths.get_extmark_by_id(ns, id1, {}) eq({0, 1}, rv) -- and the mark in the middle was shifted to the end of the insertion - rv = curbufmeths.get_extmark_by_id(ns, id2) + rv = curbufmeths.get_extmark_by_id(ns, id2, {}) eq({0, 6}, rv) end) end) From f7d01a65d50a4783527acd2de3998b65e7b78331 Mon Sep 17 00:00:00 2001 From: chentau Date: Mon, 21 Dec 2020 18:24:15 -0800 Subject: [PATCH 4/5] api: set_text: more tests, and fixing lint removing pending virtcol tests Allow passing in empty array as a shorthand for array with empty string; add more documentation add check for start_row as well --- src/nvim/api/buffer.c | 52 ++++++++++++++----------- test/functional/api/buffer_spec.lua | 59 +++++++++++++---------------- 2 files changed, 57 insertions(+), 54 deletions(-) diff --git a/src/nvim/api/buffer.c b/src/nvim/api/buffer.c index 21a7e897a9..1916d720a7 100644 --- a/src/nvim/api/buffer.c +++ b/src/nvim/api/buffer.c @@ -495,21 +495,24 @@ end: try_end(err); } -/// Sets (replaces) a range in the buffer. +/// Sets (replaces) a range in the buffer, retaining any extmarks +/// that may lie in that range. /// -/// Indexing is zero-based, end-exclusive. +/// Indexing is zero-based; end_row is inclusive, while end_col is +/// exclusive. /// /// To insert text at a given index, set `start` and `end` ranges to the same -/// index. To delete a range, set `replacement` to an empty array. +/// index. To delete a range, set `replacement` to an array containing +/// an empty string, or simply an empty array. /// -/// Prefer nvim_buf_set_lines when modifying entire lines. +/// Prefer nvim_buf_set_lines when adding or deleting entire lines. /// /// @param channel_id /// @param buffer Buffer handle, or 0 for current buffer /// @param start_row First line index /// @param start_column Last column -/// @param end_row Last line index (exclusive) -/// @param end_column Last column +/// @param end_row Last line index (inclusive) +/// @param end_column Last column (exclusive) /// @param replacement Array of lines to use as replacement /// @param[out] err Error details, if any void nvim_buf_set_text(uint64_t channel_id, @@ -522,8 +525,12 @@ void nvim_buf_set_text(uint64_t channel_id, Error *err) FUNC_API_SINCE(7) { - // TODO: treat [] as [''] for convenience - assert(replacement.size > 0); + if (replacement.size == 0) { + String s = { .data = "", .size = 0 }; + ADD(replacement, STRING_OBJ(s)); + replacement.size = 1; + } + buf_T *buf = find_buffer_by_handle(buffer, err); if (!buf) { return; @@ -534,13 +541,13 @@ void nvim_buf_set_text(uint64_t channel_id, // check range is ordered and everything! // start_row, end_row within buffer len (except add text past the end?) start_row = normalize_index(buf, start_row, &oob); - if (oob) { + if (oob || start_row == buf->b_ml.ml_line_count + 1) { api_set_error(err, kErrorTypeValidation, "start_row out of bounds"); return; } end_row = normalize_index(buf, end_row, &oob); - if (oob) { + if (oob || end_row == buf->b_ml.ml_line_count + 1) { api_set_error(err, kErrorTypeValidation, "end_row out of bounds"); return; } @@ -568,6 +575,8 @@ void nvim_buf_set_text(uint64_t channel_id, return; } + size_t new_len = replacement.size; + bcount_t new_byte = 0; bcount_t old_byte = 0; @@ -575,29 +584,25 @@ void nvim_buf_set_text(uint64_t channel_id, if (start_row == end_row) { old_byte = (bcount_t)end_col - start_col; } else { - char_u *line; + const char *bufline; old_byte += (bcount_t)strlen(str_at_start) - start_col; - for (size_t i = 0; i < (bcount_t)end_row - start_row; i++){ - int64_t lnum = start_row + (int64_t)i; + for (int64_t i = 0; i < end_row - start_row; i++) { + int64_t lnum = start_row + i; if (lnum >= MAXLNUM) { api_set_error(err, kErrorTypeValidation, "Index value is too high"); goto end; } - line = ml_get_buf(buf, lnum, false); - old_byte += (bcount_t)(strlen((char *)line)); + bufline = (char *)ml_get_buf(buf, lnum, false); + old_byte += (bcount_t)(strlen(bufline)); } - old_byte += end_col; + old_byte += (bcount_t)end_col; } - size_t new_len = replacement.size; - String first_item = replacement.items[0].data.string; String last_item = replacement.items[replacement.size-1].data.string; - new_byte += (bcount_t)(first_item.size); - size_t firstlen = (size_t)start_col+first_item.size; size_t last_part_len = strlen(str_at_end) - (size_t)end_col; if (replacement.size == 1) { @@ -618,6 +623,7 @@ void nvim_buf_set_text(uint64_t channel_id, char **lines = (new_len != 0) ? xcalloc(new_len, sizeof(char *)) : NULL; lines[0] = first; + new_byte += (bcount_t)(first_item.size); for (size_t i = 1; i < new_len-1; i++) { const String l = replacement.items[i].data.string; @@ -641,6 +647,8 @@ void nvim_buf_set_text(uint64_t channel_id, goto end; } + // Small note about undo states: unlike set_lines, we want to save the + // undo state of one past the end_row, since end_row is inclusive. if (u_save((linenr_T)start_row - 1, (linenr_T)end_row + 1) == FAIL) { api_set_error(err, kErrorTypeException, "Failed to save undo information"); goto end; @@ -718,8 +726,8 @@ void nvim_buf_set_text(uint64_t channel_id, colnr_T col_extent = (colnr_T)(end_col - ((end_col > start_col) ? start_col : 0)); extmark_splice(buf, (int)start_row-1, (colnr_T)start_col, - (int)(end_row-start_row), col_extent, (bcount_t)(old_byte), - (int)new_len-1, (colnr_T)last_item.size, (bcount_t)new_byte, + (int)(end_row-start_row), col_extent, old_byte, + (int)new_len-1, (colnr_T)last_item.size, new_byte, kExtmarkUndo); diff --git a/test/functional/api/buffer_spec.lua b/test/functional/api/buffer_spec.lua index dbe585a064..fb8ed6a9d7 100644 --- a/test/functional/api/buffer_spec.lua +++ b/test/functional/api/buffer_spec.lua @@ -394,7 +394,6 @@ describe('api/buf', function() describe('nvim_buf_get_lines, nvim_buf_set_text', function() local get_lines, set_text = curbufmeths.get_lines, curbufmeths.set_text - local line_count = curbufmeths.line_count it('works', function() insert([[ @@ -418,7 +417,7 @@ describe('api/buf', function() eq({'hello world!', 'text'}, get_lines(0, 2, true)) -- can replace with multiple lines - local err = set_text(0, 6, 0, 11, {'foo', 'wo', 'more'}) + set_text(0, 6, 0, 11, {'foo', 'wo', 'more'}) eq({'hello foo', 'wo', 'more!', 'text'}, get_lines(0, 4, true)) -- will join multiple lines if needed @@ -426,25 +425,10 @@ describe('api/buf', function() eq({'hello bar'}, get_lines(0, 1, true)) end) - pending('can handle multibyte characters', function() - insert([[ - hellØ world! - ]]) - - eq({'hellØ world!'}, get_lines(0, 1, true)) - - -- inserting multibyte - set_text(0, 11, 0, 11, {'Ø'}) - eq({'hellØ worldØ!'}, get_lines(0, 1, true)) - - -- deleting multibyte - set_text(0, 0, 0, 6, {''}) - eq({'worldØ!'}, get_lines(0, 1, true)) - end) - it('works with undo', function() insert([[ hello world! + foo bar ]]) -- setting text @@ -461,6 +445,11 @@ describe('api/buf', function() set_text(0, 0, 0, 0, {'hello', 'mr '}) feed('u') eq({'hello world!'}, get_lines(0, 1, true)) + + -- deleting newlines + set_text(0, 0, 1, 4, {'hello'}) + feed('u') + eq({'hello world!'}, get_lines(0, 1, true)) end) it('updates the cursor position', function() @@ -493,30 +482,36 @@ describe('api/buf', function() local id3 = curbufmeths.set_extmark(ns, 1, 1, {}) set_text(0, 4, 0, 7, {"q"}) - -- TODO: if we set text at 0,3, what happens to the mark at 0,3 - eq({'foo q', 'baz'}, get_lines(0, 2, true)) -- mark before replacement point is unaffected - rv = curbufmeths.get_extmark_by_id(ns, id1, {}) - eq({0, 1}, rv) + eq({0, 1}, curbufmeths.get_extmark_by_id(ns, id1, {})) -- mark gets shifted back because the replacement was shorter - rv = curbufmeths.get_extmark_by_id(ns, id2, {}) - eq({0, 5}, rv) + eq({0, 5}, curbufmeths.get_extmark_by_id(ns, id2, {})) -- mark on the next line is unaffected - rv = curbufmeths.get_extmark_by_id(ns, id3, {}) - eq({1, 1}, rv) + eq({1, 1}, curbufmeths.get_extmark_by_id(ns, id3, {})) -- replacing the text spanning two lines will adjust the mark on the next line set_text(0, 3, 1, 3, {"qux"}) - rv = curbufmeths.get_extmark_by_id(ns, id3, {}) eq({'fooqux', ''}, get_lines(0, 2, true)) - eq({0, 6}, rv) + eq({0, 6}, curbufmeths.get_extmark_by_id(ns, id3, {})) -- but mark before replacement point is still unaffected - rv = curbufmeths.get_extmark_by_id(ns, id1, {}) - eq({0, 1}, rv) + eq({0, 1}, curbufmeths.get_extmark_by_id(ns, id1, {})) -- and the mark in the middle was shifted to the end of the insertion - rv = curbufmeths.get_extmark_by_id(ns, id2, {}) - eq({0, 6}, rv) + eq({0, 6}, curbufmeths.get_extmark_by_id(ns, id2, {})) + + -- marks should be put back into the same place after undoing + set_text(0, 0, 0, 2, {''}) + feed('u') + eq({0, 1}, curbufmeths.get_extmark_by_id(ns, id1, {})) + eq({0, 6}, curbufmeths.get_extmark_by_id(ns, id2, {})) + eq({0, 6}, curbufmeths.get_extmark_by_id(ns, id3, {})) + + -- marks should be shifted over by the correct number of bytes for multibyte + -- chars + set_text(0, 0, 0, 0, {'Ø'}) + eq({0, 3}, curbufmeths.get_extmark_by_id(ns, id1, {})) + eq({0, 8}, curbufmeths.get_extmark_by_id(ns, id2, {})) + eq({0, 8}, curbufmeths.get_extmark_by_id(ns, id3, {})) end) end) From 39d098f9f9dc244a84958202e221ed0bdc6ee88a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Linse?= Date: Wed, 30 Dec 2020 11:31:17 +0100 Subject: [PATCH 5/5] api: set_text: fix some byte count issues add byte count tests update documentation --- src/nvim/api/buffer.c | 53 +++++++--------- test/functional/lua/buffer_updates_spec.lua | 67 ++++++++++++++++++++- 2 files changed, 88 insertions(+), 32 deletions(-) diff --git a/src/nvim/api/buffer.c b/src/nvim/api/buffer.c index 1916d720a7..8d82d22040 100644 --- a/src/nvim/api/buffer.c +++ b/src/nvim/api/buffer.c @@ -495,40 +495,38 @@ end: try_end(err); } -/// Sets (replaces) a range in the buffer, retaining any extmarks -/// that may lie in that range. +/// Sets (replaces) a range in the buffer /// -/// Indexing is zero-based; end_row is inclusive, while end_col is -/// exclusive. +/// This is recommended over nvim_buf_set_lines when only modifying parts of a +/// line, as extmarks will be preserved on non-modified parts of the touched +/// lines. +/// +/// Indexing is zero-based and end-exclusive. /// /// To insert text at a given index, set `start` and `end` ranges to the same /// index. To delete a range, set `replacement` to an array containing /// an empty string, or simply an empty array. /// -/// Prefer nvim_buf_set_lines when adding or deleting entire lines. +/// Prefer nvim_buf_set_lines when adding or deleting entire lines only. /// /// @param channel_id /// @param buffer Buffer handle, or 0 for current buffer /// @param start_row First line index /// @param start_column Last column -/// @param end_row Last line index (inclusive) -/// @param end_column Last column (exclusive) +/// @param end_row Last line index +/// @param end_column Last column /// @param replacement Array of lines to use as replacement /// @param[out] err Error details, if any -void nvim_buf_set_text(uint64_t channel_id, - Buffer buffer, - Integer start_row, - Integer start_col, - Integer end_row, - Integer end_col, - ArrayOf(String) replacement, - Error *err) +void nvim_buf_set_text(uint64_t channel_id, Buffer buffer, + Integer start_row, Integer start_col, + Integer end_row, Integer end_col, + ArrayOf(String) replacement, Error *err) FUNC_API_SINCE(7) { + FIXED_TEMP_ARRAY(scratch, 1); if (replacement.size == 0) { - String s = { .data = "", .size = 0 }; - ADD(replacement, STRING_OBJ(s)); - replacement.size = 1; + scratch.items[0] = STRING_OBJ(STATIC_CSTR_AS_STRING("")); + replacement = scratch; } buf_T *buf = find_buffer_by_handle(buffer, err); @@ -586,18 +584,13 @@ void nvim_buf_set_text(uint64_t channel_id, } else { const char *bufline; old_byte += (bcount_t)strlen(str_at_start) - start_col; - for (int64_t i = 0; i < end_row - start_row; i++) { + for (int64_t i = 1; i < end_row - start_row; i++) { int64_t lnum = start_row + i; - if (lnum >= MAXLNUM) { - api_set_error(err, kErrorTypeValidation, "Index value is too high"); - goto end; - } - bufline = (char *)ml_get_buf(buf, lnum, false); - old_byte += (bcount_t)(strlen(bufline)); + old_byte += (bcount_t)(strlen(bufline))+1; } - old_byte += (bcount_t)end_col; + old_byte += (bcount_t)end_col+1; } String first_item = replacement.items[0].data.string; @@ -631,11 +624,11 @@ void nvim_buf_set_text(uint64_t channel_id, // NL-used-for-NUL. lines[i] = xmemdupz(l.data, l.size); memchrsub(lines[i], NUL, NL, l.size); - new_byte += (bcount_t)(l.size); + new_byte += (bcount_t)(l.size)+1; } if (replacement.size > 1) { lines[replacement.size-1] = last; - new_byte += (bcount_t)(last_item.size); + new_byte += (bcount_t)(last_item.size)+1; } try_start(); @@ -715,8 +708,6 @@ void nvim_buf_set_text(uint64_t channel_id, // Adjust marks. Invalidate any which lie in the // changed range, and move any in the remainder of the buffer. - // Only adjust marks if we managed to switch to a window that holds - // the buffer, otherwise line numbers will be invalid. mark_adjust((linenr_T)start_row, (linenr_T)end_row, MAXLNUM, @@ -724,7 +715,7 @@ void nvim_buf_set_text(uint64_t channel_id, kExtmarkNOOP); colnr_T col_extent = (colnr_T)(end_col - - ((end_col > start_col) ? start_col : 0)); + - ((end_row == start_row) ? start_col : 0)); extmark_splice(buf, (int)start_row-1, (colnr_T)start_col, (int)(end_row-start_row), col_extent, old_byte, (int)new_len-1, (colnr_T)last_item.size, new_byte, diff --git a/test/functional/lua/buffer_updates_spec.lua b/test/functional/lua/buffer_updates_spec.lua index 9f2b1b6e52..67dc5f5a16 100644 --- a/test/functional/lua/buffer_updates_spec.lua +++ b/test/functional/lua/buffer_updates_spec.lua @@ -288,7 +288,8 @@ describe('lua: nvim_buf_attach on_bytes', function() -- TODO: while we are brewing the real strong coffe, -- verify should check buf_get_offset after every check_events if verify then - meths.buf_get_offset(0, meths.buf_line_count(0)) + local len = meths.buf_get_offset(0, meths.buf_line_count(0)) + eq(len == -1 and 1 or len, string.len(shadowbytes)) end exec_lua("return test_register(...)", 0, "test1", false, false, true) meths.buf_get_changedtick(0) @@ -510,6 +511,70 @@ describe('lua: nvim_buf_attach on_bytes', function() { "test1", "bytes", 1, 3, 0, 1, 1, 0, 3, 3, 0, 1, 1 }; } end) + + it('nvim_buf_set_text insert', function() + local check_events = setup_eventcheck(verify, {"bastext"}) + meths.buf_set_text(0, 0, 3, 0, 3, {"fiol","kontra"}) + check_events { + { "test1", "bytes", 1, 3, 0, 3, 3, 0, 0, 0, 1, 6, 11 }; + } + + meths.buf_set_text(0, 1, 6, 1, 6, {"punkt","syntgitarr","övnings"}) + check_events { + { "test1", "bytes", 1, 4, 1, 6, 14, 0, 0, 0, 2, 8, 25 }; + } + + eq({ "basfiol", "kontrapunkt", "syntgitarr", "övningstext" }, + meths.buf_get_lines(0, 0, -1, true)) + end) + + it('nvim_buf_set_text replace', function() + local check_events = setup_eventcheck(verify, origlines) + + meths.buf_set_text(0, 2, 3, 2, 8, {"very text"}) + check_events { + { "test1", "bytes", 1, 3, 2, 3, 35, 0, 5, 5, 0, 9, 9 }; + } + + meths.buf_set_text(0, 3, 5, 3, 7, {" splitty","line "}) + check_events { + { "test1", "bytes", 1, 4, 3, 5, 57, 0, 2, 2, 1, 5, 14 }; + } + + meths.buf_set_text(0, 0, 8, 1, 2, {"JOINY"}) + check_events { + { "test1", "bytes", 1, 5, 0, 8, 8, 1, 2, 10, 0, 5, 5 }; + } + + meths.buf_set_text(0, 4, 0, 6, 0, {"was 5,6",""}) + check_events { + { "test1", "bytes", 1, 6, 4, 0, 75, 2, 0, 32, 1, 0, 8 }; + } + + eq({ "originalJOINYiginal line 2", "orivery text line 3", "origi splitty", + "line l line 4", "was 5,6", " indented line" }, + meths.buf_get_lines(0, 0, -1, true)) + + end) + + it('nvim_buf_set_text delete', function() + local check_events = setup_eventcheck(verify, origlines) + + -- really {""} but accepts {} as a shorthand + meths.buf_set_text(0, 0, 0, 1, 0, {}) + check_events { + { "test1", "bytes", 1, 3, 0, 0, 0, 1, 0, 16, 0, 0, 0 }; + } + + -- TODO(bfredl): this works but is not as convenient as set_lines + meths.buf_set_text(0, 4, 15, 5, 17, {""}) + check_events { + { "test1", "bytes", 1, 4, 4, 15, 79, 1, 17, 18, 0, 0, 0 }; + } + eq({ "original line 2", "original line 3", "original line 4", + "original line 5", "original line 6" }, + meths.buf_get_lines(0, 0, -1, true)) + end) end describe('(with verify) handles', function()