patch 9.1.0423: getregionpos() wrong with blockwise mode and multibytev9.1.0423

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: #14805

Signed-off-by: zeertzjq <zeertzjq@outlook.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>

5 files changed, 135 insertions, 65 deletions

diff --git a/runtime/doc/builtin.txt b/runtime/doc/builtin.txt
index 46819d6b9f..b9dd4d20bf 100644
--- a/runtime/doc/builtin.txt
+++ b/runtime/doc/builtin.txt
@@ -1,4 +1,4 @@
-*builtin.txt*	For Vim version 9.1.  Last change: 2024 May 18
+*builtin.txt*	For Vim version 9.1.  Last change: 2024 May 20
 
 
 		  VIM REFERENCE MANUAL	  by Bram Moolenaar
@@ -4341,10 +4341,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
-		it is the offset in screen columns from the start of the
-		character.  E.g., a position within a <Tab> or after the last
-		character.
+		If the "off" number of a starting position is non-zero, it is
+		the offset in screen columns from the start of the character.
+		E.g., a position within a <Tab> or after the last 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.
 
 		Can also be used as a |method|: >
 			getpos('.')->getregionpos(getpos("'a"))
diff --git a/src/evalfunc.c b/src/evalfunc.c
index cd3a6fd80a..571ac08626 100644
--- a/src/evalfunc.c
+++ b/src/evalfunc.c
@@ -5488,11 +5488,11 @@ block_def2str(struct block_def *bd)
 getregionpos(
     typval_T	*argvars,
     typval_T	*rettv,
-    pos_T	*p1, pos_T *p2,
+    pos_T	*p1,
+    pos_T	*p2,
     int		*inclusive,
     int		*region_type,
-    oparg_T	*oa,
-    int		*fnum)
+    oparg_T	*oa)
 {
     int		fnum1 = -1, fnum2 = -1;
     char_u	*type;
@@ -5542,7 +5542,6 @@ getregionpos(
     }
 
     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));
@@ -5658,7 +5657,6 @@ f_getregion(typval_T *argvars, typval_T *rettv)
     int			inclusive = TRUE;
     int			region_type = -1;
     oparg_T		oa;
-    int			fnum;
 
     buf_T		*save_curbuf;
     int			save_virtual;
@@ -5669,7 +5667,7 @@ f_getregion(typval_T *argvars, typval_T *rettv)
     save_virtual = virtual_op;
 
     if (getregionpos(argvars, rettv,
-		&p1, &p2, &inclusive, &region_type, &oa, &fnum) == FAIL)
+		&p1, &p2, &inclusive, &region_type, &oa) == FAIL)
 	return;
 
     for (lnum = p1.lnum; lnum <= p2.lnum; lnum++)
@@ -5706,31 +5704,18 @@ f_getregion(typval_T *argvars, typval_T *rettv)
 	}
     }
 
-    // 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,
-    int		lnum2,
-    int		col2,
-    int		coladd2)
+add_regionpos_range(typval_T *rettv, pos_T p1, pos_T p2)
 {
     list_T	*l1, *l2, *l3;
-    buf_T	*findbuf;
     int		max_col1, max_col2;
 
-    findbuf = bufnr != 0 ? buflist_findnr(bufnr) : curbuf;
-    if (findbuf == NULL || findbuf->b_ml.ml_mfp == NULL)
-	return;
-
     l1 = list_alloc();
     if (l1 == NULL)
 	return;
@@ -5771,18 +5756,17 @@ add_regionpos_range(
 	return;
     }
 
+    max_col1 = ml_get_len(p1.lnum);
+    list_append_number(l2, curbuf->b_fnum);
+    list_append_number(l2, p1.lnum);
+    list_append_number(l2, p1.col > max_col1 ? max_col1 : p1.col);
+    list_append_number(l2, p1.coladd);
 
-    max_col1 = ml_get_buf_len(findbuf, lnum1);
-    list_append_number(l2, bufnr);
-    list_append_number(l2, lnum1);
-    list_append_number(l2, col1 > max_col1 ? max_col1 : col1);
-    list_append_number(l2, coladd1);
-
-    max_col2 = ml_get_buf_len(findbuf, lnum2);
-    list_append_number(l3, bufnr);
-    list_append_number(l3, lnum2);
-    list_append_number(l3, col2 > max_col2 ? max_col2 : col2);
-    list_append_number(l3, coladd2);
+    max_col2 = ml_get_len(p2.lnum);
+    list_append_number(l3, curbuf->b_fnum);
+    list_append_number(l3, p2.lnum);
+    list_append_number(l3, p2.col > max_col2 ? max_col2 : p2.col);
+    list_append_number(l3, p2.coladd);
 }
 
 /*
@@ -5795,7 +5779,6 @@ f_getregionpos(typval_T *argvars, typval_T *rettv)
     int		inclusive = TRUE;
     int		region_type = -1;
     oparg_T	oa;
-    int		fnum;
     int		lnum;
 
     buf_T	*save_curbuf;
@@ -5805,38 +5788,52 @@ f_getregionpos(typval_T *argvars, typval_T *rettv)
     save_virtual = virtual_op;
 
     if (getregionpos(argvars, rettv,
-		&p1, &p2, &inclusive, &region_type, &oa, &fnum) == FAIL)
+		&p1, &p2, &inclusive, &region_type, &oa) == FAIL)
 	return;
 
     for (lnum = p1.lnum; lnum <= p2.lnum; lnum++)
     {
 	struct block_def	bd;
-	int			start_col, end_col;
+	pos_T			ret_p1, ret_p2;
 
 	if (region_type == MLINE)
 	{
-	    start_col = 1;
-	    end_col = MAXCOL;
-	}
-	else if (region_type == MBLOCK)
-	{
-	    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;
+	    ret_p1.col = 1;
+	    ret_p1.coladd = 0;
+	    ret_p2.col = MAXCOL;
+	    ret_p2.coladd = 0;
 	}
 	else
 	{
-	    start_col = p1.lnum == lnum ? p1.col + 1 : 1;
-	    end_col = p2.lnum == lnum ? p2.col + 1 : MAXCOL;
+	    if (region_type == MBLOCK)
+		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,
-		p1.coladd, lnum, end_col, p2.coladd);
+	ret_p1.lnum = lnum;
+	ret_p2.lnum = lnum;
+	add_regionpos_range(rettv, ret_p1, ret_p2);
     }
 
     // getregionpos() may change curbuf and virtual_op
diff --git a/src/ops.c b/src/ops.c
index cdc30806ce..605a66494a 100644
--- a/src/ops.c
+++ b/src/ops.c
@@ -2451,6 +2451,7 @@ charwise_block_prep(
     p = ml_get(lnum);
     bdp->startspaces = 0;
     bdp->endspaces = 0;
+    bdp->start_char_vcols = 0;
 
     if (lnum == start.lnum)
     {
@@ -2462,8 +2463,8 @@ charwise_block_prep(
 	    {
 		// 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;
 		startcol++;
@@ -2483,19 +2484,16 @@ charwise_block_prep(
 			// of multi-byte char.
 			&& (*mb_head_off)(p, p + endcol) == 0))
 	    {
-		if (start.lnum == end.lnum
-			&& start.col == end.col)
+		if (start.lnum == end.lnum && start.col == end.col)
 		{
 		    // Special case: inside a single char
 		    is_oneChar = TRUE;
-		    bdp->startspaces = end.coladd
-			- start.coladd + inclusive;
+		    bdp->startspaces = end.coladd - start.coladd + inclusive;
 		    endcol = startcol;
 		}
 		else
 		{
-		    bdp->endspaces = end.coladd
-			+ inclusive;
+		    bdp->endspaces = end.coladd + inclusive;
 		    endcol -= inclusive;
 		}
 	    }
@@ -2507,6 +2505,7 @@ charwise_block_prep(
 	bdp->textlen = 0;
     else
 	bdp->textlen = endcol - startcol + inclusive;
+    bdp->textcol = startcol;
     bdp->textstart = p + startcol;
 }
 
diff --git a/src/testdir/test_visual.vim b/src/testdir/test_visual.vim
index b3049281b9..8c850d1ace 100644
--- a/src/testdir/test_visual.vim
+++ b/src/testdir/test_visual.vim
@@ -1640,6 +1640,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([
@@ -1647,6 +1648,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([
@@ -1654,18 +1656,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([
@@ -1673,6 +1678,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([
@@ -1815,6 +1821,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
 
@@ -1848,28 +1860,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
@@ -1877,12 +1916,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])
@@ -1975,14 +2031,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&
 
diff --git a/src/version.c b/src/version.c
index ce9627b449..d6f028d42a 100644
--- a/src/version.c
+++ b/src/version.c
@@ -705,6 +705,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    423,
+/**/
     422,
 /**/
     421,