vim-patch:9.1.0423: getregionpos() wrong with blockwise mode and multibyte

Problem:  getregionpos() wrong with blockwise mode and multibyte.
Solution: Use textcol and textlen instead of start_vcol and end_vcol.
          Handle coladd properly (zeertzjq).

Also remove unnecessary buflist_findnr() in add_regionpos_range(), as
getregionpos() has already switched buffer.

closes: vim/vim#14805

c95e64f41f

runtime/doc/builtin.txt

@@ -2981,10 +2981,12 @@ getregionpos({pos1}, {pos2} [, {opts}])                         *getregionpos()*
 		"bufnum" is the buffer number.
 		"lnum" and "col" are the position in the buffer.  The first
 		column is 1.
-		The "off" number is zero, unless 'virtualedit' is used.  Then
+		If the "off" number of a starting position is non-zero, it is
-		it is the offset in screen columns from the start of the
+		the offset in screen columns from the start of the character.
-		character.  E.g., a position within a <Tab> or after the last
+		E.g., a position within a <Tab> or after the last character.
-		character.
+		If the "off" number of an ending position is non-zero, it is
+		the character's number of cells included in the selection,
+		otherwise the whole character is included.
 
 getregtype([{regname}])                                           *getregtype()*
 		The result is a String, which is type of register {regname}.

runtime/lua/vim/_meta/vimfn.lua

@@ -3592,10 +3592,12 @@ function vim.fn.getregion(pos1, pos2, opts) end
 --- "bufnum" is the buffer number.
 --- "lnum" and "col" are the position in the buffer.  The first
 --- column is 1.
---- The "off" number is zero, unless 'virtualedit' is used.  Then
+--- If the "off" number of a starting position is non-zero, it is
---- it is the offset in screen columns from the start of the
+--- the offset in screen columns from the start of the character.
---- character.  E.g., a position within a <Tab> or after the last
+--- E.g., a position within a <Tab> or after the last character.
---- character.
+--- If the "off" number of an ending position is non-zero, it is
+--- the character's number of cells included in the selection,
+--- otherwise the whole character is included.
 ---
 --- @param pos1 table
 --- @param pos2 table

src/nvim/eval.lua

@@ -4429,10 +4429,12 @@ M.funcs = {
       "bufnum" is the buffer number.
       "lnum" and "col" are the position in the buffer.  The first
       column is 1.
-      The "off" number is zero, unless 'virtualedit' is used.  Then
+      If the "off" number of a starting position is non-zero, it is
-      it is the offset in screen columns from the start of the
+      the offset in screen columns from the start of the character.
-      character.  E.g., a position within a <Tab> or after the last
+      E.g., a position within a <Tab> or after the last character.
-      character.
+      If the "off" number of an ending position is non-zero, it is
+      the character's number of cells included in the selection,
+      otherwise the whole character is included.
     ]=],
     name = 'getregionpos',
     params = { { 'pos1', 'table' }, { 'pos2', 'table' }, { 'opts', 'table' } },

src/nvim/eval/funcs.c

@@ -2824,8 +2824,7 @@ static char *block_def2str(struct block_def *bd)
 }
 
 static int getregionpos(typval_T *argvars, typval_T *rettv, pos_T *p1, pos_T *p2,
-                        bool *const inclusive, MotionType *region_type, oparg_T *oa,
+                        bool *const inclusive, MotionType *region_type, oparg_T *oa)
-                        int *const fnum)
   FUNC_ATTR_NONNULL_ALL
 {
   tv_list_alloc_ret(rettv, kListLenMayKnow);
@@ -2871,7 +2870,6 @@ static int getregionpos(typval_T *argvars, typval_T *rettv, pos_T *p1, pos_T *p2
   }
 
   buf_T *findbuf = fnum1 != 0 ? buflist_findnr(fnum1) : curbuf;
-  *fnum = fnum1 != 0 ? fnum1 : curbuf->b_fnum;
   if (findbuf == NULL || findbuf->b_ml.ml_mfp == NULL) {
     emsg(_(e_buffer_is_not_loaded));
     return FAIL;
@@ -2971,10 +2969,8 @@ static void f_getregion(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
   bool inclusive = true;
   MotionType region_type = kMTUnknown;
   oparg_T oa;
-  int fnum;
 
-  if (getregionpos(argvars, rettv,
+  if (getregionpos(argvars, rettv, &p1, &p2, &inclusive, &region_type, &oa) == FAIL) {
-                   &p1, &p2, &inclusive, &region_type, &oa, &fnum) == FAIL) {
     return;
   }
 
@@ -2999,20 +2995,14 @@ static void f_getregion(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
     tv_list_append_allocated_string(rettv->vval.v_list, akt);
   }
 
-  // getregionpos() breaks curbuf and virtual_op
+  // getregionpos() may change curbuf and virtual_op
   curbuf = save_curbuf;
   curwin->w_buffer = curbuf;
   virtual_op = save_virtual;
 }
 
-static void add_regionpos_range(typval_T *rettv, int bufnr, int lnum1, int col1, int coladd1,
+static void add_regionpos_range(typval_T *rettv, pos_T p1, pos_T p2)
-                                int lnum2, int col2, int coladd2)
 {
-  buf_T *findbuf = bufnr != 0 ? buflist_findnr(bufnr) : curbuf;
-  if (findbuf == NULL || findbuf->b_ml.ml_mfp == NULL) {
-    return;
-  }
-
   list_T *l1 = tv_list_alloc(2);
   tv_list_append_list(rettv->vval.v_list, l1);
 
@@ -3022,17 +3012,17 @@ static void add_regionpos_range(typval_T *rettv, int bufnr, int lnum1, int col1,
   list_T *l3 = tv_list_alloc(4);
   tv_list_append_list(l1, l3);
 
-  int max_col1 = ml_get_buf_len(findbuf, lnum1);
+  int max_col1 = ml_get_len(p1.lnum);
-  tv_list_append_number(l2, bufnr);
+  tv_list_append_number(l2, curbuf->b_fnum);
-  tv_list_append_number(l2, lnum1);
+  tv_list_append_number(l2, p1.lnum);
-  tv_list_append_number(l2, col1 > max_col1 ? max_col1 : col1);
+  tv_list_append_number(l2, p1.col > max_col1 ? max_col1 : p1.col);
-  tv_list_append_number(l2, coladd1);
+  tv_list_append_number(l2, p1.coladd);
 
-  int max_col2 = ml_get_buf_len(findbuf, lnum2);
+  int max_col2 = ml_get_len(p2.lnum);
-  tv_list_append_number(l3, bufnr);
+  tv_list_append_number(l3, curbuf->b_fnum);
-  tv_list_append_number(l3, lnum2);
+  tv_list_append_number(l3, p2.lnum);
-  tv_list_append_number(l3, col2 > max_col2 ? max_col2 : col2);
+  tv_list_append_number(l3, p2.col > max_col2 ? max_col2 : p2.col);
-  tv_list_append_number(l3, coladd2);
+  tv_list_append_number(l3, p2.coladd);
 }
 
 /// "getregionpos()" function
@@ -3045,34 +3035,45 @@ static void f_getregionpos(typval_T *argvars, typval_T *rettv, EvalFuncData fptr
   bool inclusive = true;
   MotionType region_type = kMTUnknown;
   oparg_T oa;
-  int fnum;
 
-  if (getregionpos(argvars, rettv,
+  if (getregionpos(argvars, rettv, &p1, &p2, &inclusive, &region_type, &oa) == FAIL) {
-                   &p1, &p2, &inclusive, &region_type, &oa, &fnum) == FAIL) {
     return;
   }
 
   for (linenr_T lnum = p1.lnum; lnum <= p2.lnum; lnum++) {
-    int start_col, end_col;
+    struct block_def bd;
+    pos_T ret_p1, ret_p2;
 
     if (region_type == kMTLineWise) {
-      start_col = 1;
+      ret_p1.col = 1;
-      end_col = MAXCOL;
+      ret_p1.coladd = 0;
-    } else if (region_type == kMTBlockWise) {
+      ret_p2.col = MAXCOL;
-      struct block_def bd;
+      ret_p2.coladd = 0;
-      block_prep(&oa, &bd, lnum, false);
-      start_col = bd.start_vcol + 1;
-      end_col = bd.end_vcol;
-    } else if (p1.lnum < lnum && lnum < p2.lnum) {
-      start_col = 1;
-      end_col = MAXCOL;
     } else {
-      start_col = p1.lnum == lnum ? p1.col + 1 : 1;
+      if (region_type == kMTBlockWise) {
-      end_col = p2.lnum == lnum ? p2.col + 1 : MAXCOL;
+        block_prep(&oa, &bd, lnum, false);
+      } else {
+        charwise_block_prep(p1, p2, &bd, lnum, inclusive);
+      }
+      if (bd.startspaces > 0) {
+        ret_p1.col = bd.textcol;
+        ret_p1.coladd = bd.start_char_vcols - bd.startspaces;
+      } else {
+        ret_p1.col = bd.textcol + 1;
+        ret_p1.coladd = 0;
+      }
+      if (bd.endspaces > 0) {
+        ret_p2.col = bd.textcol + bd.textlen + 1;
+        ret_p2.coladd = bd.endspaces;
+      } else {
+        ret_p2.col = bd.textcol + bd.textlen;
+        ret_p2.coladd = 0;
+      }
     }
 
-    add_regionpos_range(rettv, fnum, lnum, start_col,
+    ret_p1.lnum = lnum;
-                        p1.coladd, lnum, end_col, p2.coladd);
+    ret_p2.lnum = lnum;
+    add_regionpos_range(rettv, ret_p1, ret_p2);
   }
 
   // getregionpos() may change curbuf and virtual_op

src/nvim/ops.c

@@ -4258,6 +4258,7 @@ void charwise_block_prep(pos_T start, pos_T end, struct block_def *bdp, linenr_T
   char *p = ml_get(lnum);
   bdp->startspaces = 0;
   bdp->endspaces = 0;
+  bdp->start_char_vcols = 0;
 
   if (lnum == start.lnum) {
     startcol = start.col;
@@ -4265,7 +4266,8 @@ void charwise_block_prep(pos_T start, pos_T end, struct block_def *bdp, linenr_T
       getvcol(curwin, &start, &cs, NULL, &ce);
       if (ce != cs && start.coladd > 0) {
         // Part of a tab selected -- but don't double-count it.
-        bdp->startspaces = (ce - cs + 1) - start.coladd;
+        bdp->start_char_vcols = ce - cs + 1;
+        bdp->startspaces = bdp->start_char_vcols - start.coladd;
         if (bdp->startspaces < 0) {
           bdp->startspaces = 0;
         }
@@ -4303,6 +4305,7 @@ void charwise_block_prep(pos_T start, pos_T end, struct block_def *bdp, linenr_T
   } else {
     bdp->textlen = endcol - startcol + inclusive;
   }
+  bdp->textcol = startcol;
   bdp->textstart = p + startcol;
 }
 

test/old/testdir/test_visual.vim

@@ -1644,6 +1644,7 @@ func Test_visual_getregion()
     #" Visual mode
     call cursor(1, 1)
     call feedkeys("\<ESC>vjl", 'tx')
+
     call assert_equal(['one', 'tw'],
           \ 'v'->getpos()->getregion(getpos('.')))
     call assert_equal([
@@ -1651,6 +1652,7 @@ func Test_visual_getregion()
           \   [[bufnr('%'), 2, 1, 0], [bufnr('%'), 2, 2, 0]]
           \ ],
           \ 'v'->getpos()->getregionpos(getpos('.')))
+
     call assert_equal(['one', 'tw'],
           \ '.'->getpos()->getregion(getpos('v')))
     call assert_equal([
@@ -1658,18 +1660,21 @@ func Test_visual_getregion()
           \   [[bufnr('%'), 2, 1, 0], [bufnr('%'), 2, 2, 0]]
           \ ],
           \ '.'->getpos()->getregionpos(getpos('v')))
+
     call assert_equal(['o'],
           \ 'v'->getpos()->getregion(getpos('v')))
     call assert_equal([
           \   [[bufnr('%'), 1, 1, 0], [bufnr('%'), 1, 1, 0]],
           \ ],
           \ 'v'->getpos()->getregionpos(getpos('v')))
+
     call assert_equal(['w'],
           \ '.'->getpos()->getregion(getpos('.'), {'type': 'v' }))
     call assert_equal([
           \   [[bufnr('%'), 2, 2, 0], [bufnr('%'), 2, 2, 0]],
           \ ],
           \ '.'->getpos()->getregionpos(getpos('.'), {'type': 'v' }))
+
     call assert_equal(['one', 'two'],
           \ getpos('.')->getregion(getpos('v'), {'type': 'V' }))
     call assert_equal([
@@ -1677,6 +1682,7 @@ func Test_visual_getregion()
           \   [[bufnr('%'), 2, 1, 0], [bufnr('%'), 2, 3, 0]],
           \ ],
           \ getpos('.')->getregionpos(getpos('v'), {'type': 'V' }))
+
     call assert_equal(['on', 'tw'],
           \ getpos('.')->getregion(getpos('v'), {'type': "\<C-v>" }))
     call assert_equal([
@@ -1819,6 +1825,12 @@ func Test_visual_getregion()
         call assert_equal(range(10)->mapnew('string(v:val)'),
               \ getregion([g:buf, 10, 2, 0], [g:buf, 1, 1, 0],
               \ {'type': type, 'exclusive': exclusive }))
+        call assert_equal(range(1, 10)->mapnew('repeat([[g:buf, v:val, 1, 0]], 2)'),
+              \ getregionpos([g:buf, 1, 1, 0], [g:buf, 10, 2, 0],
+              \ {'type': type, 'exclusive': exclusive }))
+        call assert_equal(range(1, 10)->mapnew('repeat([[g:buf, v:val, 1, 0]], 2)'),
+              \ getregionpos([g:buf, 10, 2, 0], [g:buf, 1, 1, 0],
+              \ {'type': type, 'exclusive': exclusive }))
       endfor
     endfor
 
@@ -1852,28 +1864,55 @@ func Test_visual_getregion()
           \   "\U0001f1e6\u00ab\U0001f1e7\u00ab\U0001f1e8\u00ab\U0001f1e9",
           \   "1234567890"
           \ ])
+
     call cursor(1, 3)
     call feedkeys("\<Esc>\<C-v>ljj", 'xt')
     call assert_equal(['cd', "\u00ab ", '34'],
           \ getregion(getpos('v'), getpos('.'), {'type': "\<C-v>" }))
+    call assert_equal([
+          \   [[bufnr('%'), 1, 3, 0], [bufnr('%'), 1, 4, 0]],
+          \   [[bufnr('%'), 2, 5, 0], [bufnr('%'), 2, 7, 1]],
+          \   [[bufnr('%'), 3, 3, 0], [bufnr('%'), 3, 4, 0]],
+          \ ],
+          \ getregionpos(getpos('v'), getpos('.'), {'type': "\<C-v>" }))
+
     call cursor(1, 4)
     call feedkeys("\<Esc>\<C-v>ljj", 'xt')
     call assert_equal(['de', "\U0001f1e7", '45'],
           \ getregion(getpos('v'), getpos('.'), {'type': "\<C-v>" }))
+    call assert_equal([
+          \   [[bufnr('%'), 1, 4, 0], [bufnr('%'), 1, 5, 0]],
+          \   [[bufnr('%'), 2, 7, 0], [bufnr('%'), 2, 10, 0]],
+          \   [[bufnr('%'), 3, 4, 0], [bufnr('%'), 3, 5, 0]],
+          \ ],
+          \ getregionpos(getpos('v'), getpos('.'), {'type': "\<C-v>" }))
+
     call cursor(1, 5)
     call feedkeys("\<Esc>\<C-v>jj", 'xt')
     call assert_equal(['e', ' ', '5'],
           \ getregion(getpos('v'), getpos('.'), {'type': "\<C-v>" }))
+    call assert_equal([
+          \   [[bufnr('%'), 1, 5, 0], [bufnr('%'), 1, 5, 0]],
+          \   [[bufnr('%'), 2, 10, 1], [bufnr('%'), 2, 10, 0]],
+          \   [[bufnr('%'), 3, 5, 0], [bufnr('%'), 3, 5, 0]],
+          \ ],
+          \ getregionpos(getpos('v'), getpos('.'), {'type': "\<C-v>" }))
     call assert_equal([
           \   [[bufnr('%'), 1, 5, 0], [bufnr('%'), 1, 13, 0]],
           \   [[bufnr('%'), 2, 1, 0], [bufnr('%'), 2, 22, 0]],
           \   [[bufnr('%'), 3, 1, 0], [bufnr('%'), 3, 5, 0]],
           \ ],
           \ getregionpos(getpos('v'), getpos('.'), {'type': 'v' }))
+
     call cursor(1, 1)
     call feedkeys("\<Esc>vj", 'xt')
     call assert_equal(['abcdefghijk«', "\U0001f1e6"],
           \ getregion(getpos('v'), getpos('.'), {'type': 'v' }))
+    call assert_equal([
+          \   [[bufnr('%'), 1, 1, 0], [bufnr('%'), 1, 13, 0]],
+          \   [[bufnr('%'), 2, 1, 0], [bufnr('%'), 2, 4, 0]],
+          \ ],
+          \ getregionpos(getpos('v'), getpos('.'), {'type': 'v' }))
 
     #" marks on multibyte chars
     :set selection=exclusive
@@ -1881,12 +1920,29 @@ func Test_visual_getregion()
     call setpos("'b", [0, 2, 16, 0])
     call setpos("'c", [0, 2, 0, 0])
     call cursor(1, 1)
+
     call assert_equal(['ghijk', '🇨«🇩'],
           \ getregion(getpos("'a"), getpos("'b"), {'type': "\<C-v>" }))
+    call assert_equal([
+          \   [[bufnr('%'), 1, 7, 0], [bufnr('%'), 1, 11, 0]],
+          \   [[bufnr('%'), 2, 13, 0], [bufnr('%'), 2, 22, 0]],
+          \ ],
+          \ getregionpos(getpos("'a"), getpos("'b"), {'type': "\<C-v>" }))
+
     call assert_equal(['k«', '🇦«🇧«🇨'],
           \ getregion(getpos("'a"), getpos("'b"), {'type': 'v' }))
+    call assert_equal([
+          \   [[bufnr('%'), 1, 11, 0], [bufnr('%'), 1, 13, 0]],
+          \   [[bufnr('%'), 2, 1, 0], [bufnr('%'), 2, 16, 0]],
+          \ ],
+          \ getregionpos(getpos("'a"), getpos("'b"), {'type': 'v' }))
+
     call assert_equal(['k«'],
           \ getregion(getpos("'a"), getpos("'c"), {'type': 'v' }))
+    call assert_equal([
+          \   [[bufnr('%'), 1, 11, 0], [bufnr('%'), 1, 13, 0]],
+          \ ],
+          \ getregionpos(getpos("'a"), getpos("'c"), {'type': 'v' }))
 
     #" use inclusive selection, although 'selection' is exclusive
     call setpos("'a", [0, 1, 11, 0])
@@ -1979,14 +2035,28 @@ func Test_visual_getregion()
     #" virtualedit
     set selection=exclusive
     set virtualedit=all
+
     call cursor(1, 1)
     call feedkeys("\<Esc>2lv2lj", 'xt')
     call assert_equal(['      c', 'x   '],
           \ getregion(getpos('v'), getpos('.'), {'type': 'v' }))
+    call assert_equal([
+          \   [[bufnr('%'), 1, 2, 1], [bufnr('%'), 1, 3, 0]],
+          \   [[bufnr('%'), 2, 1, 0], [bufnr('%'), 2, 2, 3]],
+          \ ],
+          \ getregionpos(getpos('v'), getpos('.'), {'type': 'v' }))
+
     call cursor(1, 1)
     call feedkeys("\<Esc>2l\<C-v>2l2j", 'xt')
     call assert_equal(['  ', '  ', '  '],
           \ getregion(getpos('v'), getpos('.'), {'type': "\<C-v>" }))
+    call assert_equal([
+          \   [[bufnr('%'), 1, 2, 5], [bufnr('%'), 1, 2, 0]],
+          \   [[bufnr('%'), 2, 2, 5], [bufnr('%'), 2, 2, 0]],
+          \   [[bufnr('%'), 3, 0, 0], [bufnr('%'), 3, 0, 2]],
+          \ ],
+          \ getregionpos(getpos('v'), getpos('.'), {'type': "\<C-v>" }))
+
     set virtualedit&
     set selection&