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.

1 files changed, 71 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);
 }