patch 9.0.1729: screenpos() wrong when w_skipcol and cpoptions+=n

Problem:    screenpos() wrong result with w_skipcol and cpoptions+=n
Solution:   Use adjust_plines_for_skipcol() instead of subtracting
            w_skipcol.

closes: #12625

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

src/drawscreen.c

@@ -1792,7 +1792,8 @@ win_update(win_T *wp)
 		j = wp->w_lines[0].wl_lnum - wp->w_topline;
 	    if (j < wp->w_height - 2)		// not too far off
 	    {
-		i = plines_m_win(wp, wp->w_topline, wp->w_lines[0].wl_lnum - 1);
+		i = plines_m_win(wp, wp->w_topline, wp->w_lines[0].wl_lnum - 1,
+									 TRUE);
 #ifdef FEAT_DIFF
 		// insert extra lines for previously invisible filler lines
 		if (wp->w_lines[0].wl_lnum != wp->w_topline)

src/misc1.c

@@ -342,12 +342,13 @@ plines(linenr_T lnum)
 plines_win(
     win_T	*wp,
     linenr_T	lnum,
-    int		winheight)	// when TRUE limit to window height
+    int		limit_winheight)	// when TRUE limit to window height
 {
 #if defined(FEAT_DIFF) || defined(PROTO)
     // Check for filler lines above this buffer line.  When folded the result
     // is one line anyway.
-    return plines_win_nofill(wp, lnum, winheight) + diff_check_fill(wp, lnum);
+    return plines_win_nofill(wp, lnum, limit_winheight)
+						   + diff_check_fill(wp, lnum);
 }
 
 /*
@@ -364,7 +365,7 @@ plines_nofill(linenr_T lnum)
 plines_win_nofill(
     win_T	*wp,
     linenr_T	lnum,
-    int		winheight)	// when TRUE limit to window height
+    int		limit_winheight)	// when TRUE limit to window height
 {
 #endif
     int		lines;
@@ -389,7 +390,7 @@ plines_win_nofill(
     else
 	lines = plines_win_nofold(wp, lnum);
 
-    if (winheight > 0 && lines > wp->w_height)
+    if (limit_winheight && lines > wp->w_height)
 	return wp->w_height;
     return lines;
 }
@@ -497,7 +498,7 @@ plines_win_col(win_T *wp, linenr_T lnum, long column)
 }
 
     int
-plines_m_win(win_T *wp, linenr_T first, linenr_T last)
+plines_m_win(win_T *wp, linenr_T first, linenr_T last, int limit_winheight)
 {
     int		count = 0;
 
@@ -519,10 +520,11 @@ plines_m_win(win_T *wp, linenr_T first, linenr_T last)
 	{
 #ifdef FEAT_DIFF
 	    if (first == wp->w_topline)
-		count += plines_win_nofill(wp, first, TRUE) + wp->w_topfill;
+		count += plines_win_nofill(wp, first, limit_winheight)
+							       + wp->w_topfill;
 	    else
 #endif
-		count += plines_win(wp, first, TRUE);
+		count += plines_win(wp, first, limit_winheight);
 	    ++first;
 	}
     }

src/move.c

@@ -1356,7 +1356,10 @@ curs_columns(
 	    // don't skip more than necessary
 	    if (n > p_lines - curwin->w_height + 1)
 		n = p_lines - curwin->w_height + 1;
-	    curwin->w_skipcol = n * width2;
+	    if (n > 0)
+		curwin->w_skipcol = width1 + (n - 1) * width2;
+	    else
+		curwin->w_skipcol = 0;
 	}
 	else if (extra == 1)
 	{
@@ -1443,7 +1446,6 @@ textpos2screenpos(
 {
     colnr_T	scol = 0, ccol = 0, ecol = 0;
     int		row = 0;
-    int		rowoff = 0;
     colnr_T	coloff = 0;
 
     if (pos->lnum >= wp->w_topline && pos->lnum <= wp->w_botline)
@@ -1456,7 +1458,10 @@ textpos2screenpos(
 
 	is_folded = hasFoldingWin(wp, lnum, &lnum, NULL, TRUE, NULL);
 #endif
-	row = plines_m_win(wp, wp->w_topline, lnum - 1) + 1;
+	row = plines_m_win(wp, wp->w_topline, lnum - 1, FALSE) + 1;
+	// "row" should be the screen line where line "lnum" begins, which can
+	// be negative if "lnum" is "w_topline" and "w_skipcol" is non-zero.
+	row = adjust_plines_for_skipcol(wp, row);
 
 #ifdef FEAT_DIFF
 	// Add filler lines above this buffer line.
@@ -1481,32 +1486,30 @@ textpos2screenpos(
 	    col += off;
 	    width = wp->w_width - off + win_col_off2(wp);
 
-	    if (lnum == wp->w_topline)
-		col -= wp->w_skipcol;
-
 	    // long line wrapping, adjust row
 	    if (wp->w_p_wrap
 		    && col >= (colnr_T)wp->w_width
 		    && width > 0)
 	    {
 		// use same formula as what is used in curs_columns()
-		rowoff = ((col - wp->w_width) / width + 1);
+		int rowoff = ((col - wp->w_width) / width + 1);
 		col -= rowoff * width;
+		row += rowoff;
 	    }
 	    col -= wp->w_leftcol;
 	    if (col >= wp->w_width)
 		col = -1;
-	    if (col >= 0 && row + rowoff <= wp->w_height)
+	    if (col >= 0 && row > 0 && row <= wp->w_height)
 	    {
 		coloff = col - scol + wp->w_wincol + 1;
 		row += W_WINROW(wp);
 	    }
 	    else
-		// character is left, right or below of the window
+		// character is out of the window
-		row = rowoff = scol = ccol = ecol = 0;
+		row = scol = ccol = ecol = 0;
 	}
     }
-    *rowp = row + rowoff;
+    *rowp = row;
     *scolp = scol + coloff;
     *ccolp = ccol + coloff;
     *ecolp = ecol + coloff;

src/popupwin.c

@@ -654,7 +654,7 @@ popup_show_curline(win_T *wp)
 	    wp->w_topline = wp->w_buffer->b_ml.ml_line_count;
 	while (wp->w_topline < wp->w_cursor.lnum
 		&& wp->w_topline < wp->w_buffer->b_ml.ml_line_count
-		&& plines_m_win(wp, wp->w_topline, wp->w_cursor.lnum)
+		&& plines_m_win(wp, wp->w_topline, wp->w_cursor.lnum, TRUE)
 								> wp->w_height)
 	    ++wp->w_topline;
     }

src/proto/misc1.pro

@@ -2,12 +2,12 @@
 int get_leader_len(char_u *line, char_u **flags, int backward, int include_space);
 int get_last_leader_offset(char_u *line, char_u **flags);
 int plines(linenr_T lnum);
-int plines_win(win_T *wp, linenr_T lnum, int winheight);
+int plines_win(win_T *wp, linenr_T lnum, int limit_winheight);
 int plines_nofill(linenr_T lnum);
-int plines_win_nofill(win_T *wp, linenr_T lnum, int winheight);
+int plines_win_nofill(win_T *wp, linenr_T lnum, int limit_winheight);
 int plines_win_nofold(win_T *wp, linenr_T lnum);
 int plines_win_col(win_T *wp, linenr_T lnum, long column);
-int plines_m_win(win_T *wp, linenr_T first, linenr_T last);
+int plines_m_win(win_T *wp, linenr_T first, linenr_T last, int limit_winheight);
 int gchar_pos(pos_T *pos);
 int gchar_cursor(void);
 void pchar_cursor(int c);

src/testdir/test_cursor_func.vim

@@ -130,38 +130,71 @@ func Test_screenpos()
 	\ winid->screenpos(line('$'), 22))
 
   1split
-  normal G$
-  redraw
-  " w_skipcol should be subtracted
-  call assert_equal({'row': winrow + 0,
-	\ 'col': wincol + 20 - 1,
-	\ 'curscol': wincol + 20 - 1,
-	\ 'endcol': wincol + 20 - 1},
-	\ screenpos(win_getid(), line('.'), col('.')))
 
   " w_leftcol should be subtracted
   setlocal nowrap
-  normal 050zl$
+  normal G050zl$
+  redraw
   call assert_equal({'row': winrow + 0,
 	\ 'col': wincol + 10 - 1,
 	\ 'curscol': wincol + 10 - 1,
 	\ 'endcol': wincol + 10 - 1},
 	\ screenpos(win_getid(), line('.'), col('.')))
 
-  " w_skipcol should only matter for the topline
+  " w_skipcol should be taken into account
-" FIXME: This fails because pline_m_win() does not take w_skipcol into
+  setlocal wrap
-" account.  If it does, then other tests fail.
+  normal $
-"  wincmd +
+  redraw
-"  setlocal wrap smoothscroll
+  call assert_equal({'row': winrow + 0,
-"  call setline(line('$') + 1, 'last line')
+	\ 'col': wincol + 20 - 1,
-"  exe "normal \<C-E>G$"
+	\ 'curscol': wincol + 20 - 1,
-"  redraw
+	\ 'endcol': wincol + 20 - 1},
-"  call assert_equal({'row': winrow + 1,
+	\ screenpos(win_getid(), line('.'), col('.')))
-"	\ 'col': wincol + 9 - 1,
+  call assert_equal({'row': 0, 'col': 0, 'curscol': 0, 'endcol': 0},
-"	\ 'curscol': wincol + 9 - 1,
+	\ screenpos(win_getid(), line('.'), col('.') - 20))
-"	\ 'endcol': wincol + 9 - 1},
+  setlocal number
-"	\ screenpos(win_getid(), line('.'), col('.')))
+  redraw
-  close
+  call assert_equal({'row': winrow + 0,
+	\ 'col': wincol + 16 - 1,
+	\ 'curscol': wincol + 16 - 1,
+	\ 'endcol': wincol + 16 - 1},
+	\ screenpos(win_getid(), line('.'), col('.')))
+  call assert_equal({'row': 0, 'col': 0, 'curscol': 0, 'endcol': 0},
+	\ screenpos(win_getid(), line('.'), col('.') - 16))
+  set cpoptions+=n
+  redraw
+  call assert_equal({'row': winrow + 0,
+	\ 'col': wincol + 4 - 1,
+	\ 'curscol': wincol + 4 - 1,
+	\ 'endcol': wincol + 4 - 1},
+	\ screenpos(win_getid(), line('.'), col('.')))
+  call assert_equal({'row': 0, 'col': 0, 'curscol': 0, 'endcol': 0},
+	\ screenpos(win_getid(), line('.'), col('.') - 4))
+
+  wincmd +
+  call setline(line('$') + 1, 'last line')
+  setlocal smoothscroll
+  normal G$
+  redraw
+  call assert_equal({'row': winrow + 1,
+	\ 'col': wincol + 4 + 9 - 1,
+	\ 'curscol': wincol + 4 + 9 - 1,
+	\ 'endcol': wincol + 4 + 9 - 1},
+	\ screenpos(win_getid(), line('.'), col('.')))
+  set cpoptions-=n
+  redraw
+  call assert_equal({'row': winrow + 1,
+	\ 'col': wincol + 4 + 9 - 1,
+	\ 'curscol': wincol + 4 + 9 - 1,
+	\ 'endcol': wincol + 4 + 9 - 1},
+	\ screenpos(win_getid(), line('.'), col('.')))
+  setlocal nonumber
+  redraw
+  call assert_equal({'row': winrow + 1,
+	\ 'col': wincol + 9 - 1,
+	\ 'curscol': wincol + 9 - 1,
+	\ 'endcol': wincol + 9 - 1},
+	\ screenpos(win_getid(), line('.'), col('.')))
 
   close
   call assert_equal({}, screenpos(999, 1, 1))

src/version.c

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