patch 8.2.5014: byte offsets are wrong when using text properties

Problem:    Byte offsets are wrong when using text properties.
Solution:   Make sure text properties do not affect the byte counts.
            (Paul Ollis, closes #10474)

src/memline.c

@@ -4004,6 +4004,8 @@ ml_flush_line(buf_T *buf)
 	    {
 #if defined(FEAT_BYTEOFF) && defined(FEAT_PROP_POPUP)
 		int old_prop_len = 0;
+		if (buf->b_has_textprop)
+		    old_prop_len = old_len - (int)STRLEN(old_line) - 1;
 #endif
 		// if the length changes and there are following lines
 		count = buf->b_ml.ml_locked_high - buf->b_ml.ml_locked_low + 1;
@@ -4023,10 +4025,6 @@ ml_flush_line(buf_T *buf)
 		// adjust free space
 		dp->db_free -= extra;
 		dp->db_txt_start -= extra;
-#if defined(FEAT_BYTEOFF) && defined(FEAT_PROP_POPUP)
-		if (buf->b_has_textprop)
-		    old_prop_len = old_len - (int)STRLEN(new_line) - 1;
-#endif
 
 		// copy new line into the data block
 		mch_memmove(old_line - extra, new_line, (size_t)new_len);

src/testdir/test_textprop.vim

@@ -2094,4 +2094,97 @@ func Test_prop_blockwise_change()
   bwipe!
 endfunc
 
+func Do_test_props_do_not_affect_byte_offsets(ff, increment)
+  new
+  let lcount = 410
+
+  " File format affects byte-offset calculations, so make sure it is known.
+  exec 'setlocal fileformat=' . a:ff
+
+  " Fill the buffer with varying length lines. We need a suitably large number
+  " to force Vim code through paths wehere previous error have occurred. This
+  " is more 'art' than 'science'.
+  let text = 'a'
+  call setline(1, text)
+  let offsets = [1]
+  for idx in range(lcount)
+      call add(offsets, offsets[idx] + len(text) + a:increment)
+      if (idx % 6) == 0
+          let text = text . 'a'
+      endif
+      call append(line('$'), text)
+  endfor
+
+  " Set a property that spans a few lines to cause Vim's internal buffer code
+  " to perform a reasonable amount of rearrangement.
+  call prop_type_add('one', {'highlight': 'ErrorMsg'})
+  call prop_add(1, 1, {'type': 'one', 'end_lnum': 6, 'end_col': 2})
+
+  for idx in range(lcount)
+      let boff = line2byte(idx + 1)
+      call assert_equal(offsets[idx], boff, 'Bad byte offset at line ' . (idx + 1))
+  endfor
+
+  call prop_type_delete('one')
+  bwipe!
+endfunc
+
+func Test_props_do_not_affect_byte_offsets()
+  call Do_test_props_do_not_affect_byte_offsets('unix', 1)
+endfunc
+
+func Test_props_do_not_affect_byte_offsets_dos()
+  call Do_test_props_do_not_affect_byte_offsets('dos', 2)
+endfunc
+
+func Test_props_do_not_affect_byte_offsets_editline()
+  new
+  let lcount = 410
+
+  " File format affects byte-offset calculations, so make sure it is known.
+  setlocal fileformat=unix
+
+  " Fill the buffer with varying length lines. We need a suitably large number
+  " to force Vim code through paths wehere previous error have occurred. This
+  " is more 'art' than 'science'.
+  let text = 'aa'
+  call setline(1, text)
+  let offsets = [1]
+  for idx in range(lcount)
+      call add(offsets, offsets[idx] + len(text) + 1)
+      if (idx % 6) == 0
+          let text = text . 'a'
+      endif
+      call append(line('$'), text)
+  endfor
+
+  " Set a property that just covers the first line. When this test was
+  " developed, this did not trigger a byte-offset error.
+  call prop_type_add('one', {'highlight': 'ErrorMsg'})
+  call prop_add(1, 1, {'type': 'one', 'end_lnum': 1, 'end_col': 3})
+
+  for idx in range(lcount)
+      let boff = line2byte(idx + 1)
+      call assert_equal(offsets[idx], boff,
+          \ 'Confounding bad byte offset at line ' . (idx + 1))
+  endfor
+
+  " Insert text in the middle of the first line, keeping the property
+  " unchanged.
+  :1
+  normal aHello
+  for idx in range(1, lcount)
+      let offsets[idx] = offsets[idx] + 5
+  endfor
+
+  for idx in range(lcount)
+      let boff = line2byte(idx + 1)
+      call assert_equal(offsets[idx], boff,
+          \ 'Bad byte offset at line ' . (idx + 1))
+  endfor
+
+  call prop_type_delete('one')
+  bwipe!
+endfunc
+
 " vim: shiftwidth=2 sts=2 expandtab

src/textprop.c

@@ -344,6 +344,10 @@ f_prop_add_list(typval_T *argvars, typval_T *rettv UNUSED)
     if (get_bufnr_from_arg(&argvars[0], &buf) == FAIL)
 	return;
 
+    // This must be done _before_ we start adding properties because property
+    // changes trigger buffer (memline) reorganisation, which needs this flag
+    // to be correctly set.
+    buf->b_has_textprop = TRUE;  // this is never reset
     FOR_ALL_LIST_ITEMS(argvars[1].vval.v_list, li)
     {
 	if (li->li_tv.v_type != VAR_LIST || li->li_tv.vval.v_list == NULL)
@@ -368,7 +372,6 @@ f_prop_add_list(typval_T *argvars, typval_T *rettv UNUSED)
 	    return;
     }
 
-    buf->b_has_textprop = TRUE;  // this is never reset
     redraw_buf_later(buf, VALID);
 }
 
@@ -441,9 +444,13 @@ prop_add_common(
     if (dict_arg != NULL && get_bufnr_from_arg(dict_arg, &buf) == FAIL)
 	return;
 
+    // This must be done _before_ we add the property because property changes
+    // trigger buffer (memline) reorganisation, which needs this flag to be
+    // correctly set.
+    buf->b_has_textprop = TRUE;  // this is never reset
+
     prop_add_one(buf, type_name, id, start_lnum, end_lnum, start_col, end_col);
 
-    buf->b_has_textprop = TRUE;  // this is never reset
     redraw_buf_later(buf, VALID);
 }
 

src/version.c

@@ -734,6 +734,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    5014,
 /**/
     5013,
 /**/