vim-patch:9.0.1729: screenpos() wrong when w_skipcol and cpoptions+=n (#24773)

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

closes: vim/vim#12625

bfe377b8f2

src/nvim/drawscreen.c

@@ -1657,7 +1657,7 @@ static void win_update(win_T *wp, DecorProviders *providers)
         j = wp->w_lines[0].wl_lnum - wp->w_topline;
       }
       if (j < wp->w_grid.rows - 2) {               // not too far off
-        int i = plines_m_win(wp, wp->w_topline, wp->w_lines[0].wl_lnum - 1);
+        int i = plines_m_win(wp, wp->w_topline, wp->w_lines[0].wl_lnum - 1, true);
         // insert extra lines for previously invisible filler lines
         if (wp->w_lines[0].wl_lnum != wp->w_topline) {
           i += win_get_fill(wp, wp->w_lines[0].wl_lnum) - wp->w_old_topfill;

src/nvim/move.c

@@ -69,7 +69,7 @@ int adjust_plines_for_skipcol(win_T *wp, int n)
   if (wp->w_skipcol >= width) {
     off++;
     int skip = wp->w_skipcol - width;
-    width -= win_col_off2(wp);
+    width += win_col_off2(wp);
     while (skip >= width) {
       off++;
       skip -= width;
@@ -996,7 +996,11 @@ void curs_columns(win_T *wp, int may_scroll)
       if (n > plines - wp->w_height_inner + 1) {
         n = plines - wp->w_height_inner + 1;
       }
-      wp->w_skipcol = n * width2;
+      if (n > 0) {
+        curwin->w_skipcol = width1 + (n - 1) * width2;
+      } else {
+        curwin->w_skipcol = 0;
+      }
     } else if (extra == 1) {
       // less than 'scrolloff' lines above, decrease skipcol
       assert(so <= INT_MAX);
@@ -1064,7 +1068,6 @@ void textpos2screenpos(win_T *wp, pos_T *pos, int *rowp, int *scolp, int *ccolp,
 {
   colnr_T scol = 0, ccol = 0, ecol = 0;
   int row = 0;
-  int rowoff = 0;
   colnr_T coloff = 0;
   bool visible_row = false;
   bool is_folded = false;
@@ -1072,7 +1075,10 @@ void textpos2screenpos(win_T *wp, pos_T *pos, int *rowp, int *scolp, int *ccolp,
   linenr_T lnum = pos->lnum;
   if (lnum >= wp->w_topline && lnum <= wp->w_botline) {
     is_folded = hasFoldingWin(wp, lnum, &lnum, NULL, true, NULL);
-    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);
     // Add filler lines above this buffer line.
     row += lnum == wp->w_topline ? wp->w_topfill : win_get_fill(wp, lnum);
     visible_row = true;
@@ -1098,20 +1104,17 @@ void textpos2screenpos(win_T *wp, pos_T *pos, int *rowp, int *scolp, int *ccolp,
       col += off;
       int width = wp->w_width_inner - 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_inner && width > 0) {
         // use same formula as what is used in curs_columns()
-        rowoff = visible_row ? ((col - wp->w_width_inner) / width + 1) : 0;
+        int rowoff = visible_row ? ((col - wp->w_width_inner) / width + 1) : 0;
         col -= rowoff * width;
+        row += rowoff;
       }
 
       col -= wp->w_leftcol;
 
-      if (col >= 0 && col < wp->w_width_inner && row + rowoff <= wp->w_height_inner) {
+      if (col >= 0 && col < wp->w_width_inner && row > 0 && row <= wp->w_height_inner) {
         coloff = col - scol + (local ? 0 : wp->w_wincol + wp->w_wincol_off) + 1;
         row += local ? 0 : wp->w_winrow + wp->w_winrow_off;
       } else {
@@ -1120,12 +1123,12 @@ void textpos2screenpos(win_T *wp, pos_T *pos, int *rowp, int *scolp, int *ccolp,
         if (local) {
           coloff = col < 0 ? -1 : wp->w_width_inner + 1;
         } else {
-          row = rowoff = 0;
+          row = 0;
         }
       }
     }
   }
-  *rowp = row + rowoff;
+  *rowp = row;
   *scolp = scol + coloff;
   *ccolp = ccol + coloff;
   *ecolp = ecol + coloff;

src/nvim/plines.c

@@ -35,11 +35,11 @@
 /// Return the number of window lines occupied by buffer line "lnum".
 /// Includes any filler lines.
 ///
-/// @param winheight when true limit to window height
+/// @param limit_winheight  when true limit to window height
-int plines_win(win_T *wp, linenr_T lnum, bool winheight)
+int plines_win(win_T *wp, linenr_T lnum, bool limit_winheight)
 {
   // Check for filler lines above this buffer line.
-  return plines_win_nofill(wp, lnum, winheight) + win_get_fill(wp, lnum);
+  return plines_win_nofill(wp, lnum, limit_winheight) + win_get_fill(wp, lnum);
 }
 
 /// Return the number of filler lines above "lnum".
@@ -71,8 +71,8 @@ bool win_may_fill(win_T *wp)
 /// Return the number of window lines occupied by buffer line "lnum".
 /// Does not include filler lines.
 ///
-/// @param winheight when true limit to window height
+/// @param limit_winheight  when true limit to window height
-int plines_win_nofill(win_T *wp, linenr_T lnum, bool winheight)
+int plines_win_nofill(win_T *wp, linenr_T lnum, bool limit_winheight)
 {
   if (!wp->w_p_wrap) {
     return 1;
@@ -88,7 +88,7 @@ int plines_win_nofill(win_T *wp, linenr_T lnum, bool winheight)
   }
 
   const int lines = plines_win_nofold(wp, lnum);
-  if (winheight && lines > wp->w_height_inner) {
+  if (limit_winheight && lines > wp->w_height_inner) {
     return wp->w_height_inner;
   }
   return lines;
@@ -189,17 +189,17 @@ int plines_win_col(win_T *wp, linenr_T lnum, long column)
 /// @param[out] nextp            if not NULL, the line after a fold
 /// @param[out] foldedp          if not NULL, whether lnum is on a fold
 /// @param[in]  cache            whether to use the window's cache for folds
-/// @param[in]  winheight when true limit to window height
+/// @param[in]  limit_winheight  when true limit to window height
 ///
 /// @return the total number of screen lines
 int plines_win_full(win_T *wp, linenr_T lnum, linenr_T *const nextp, bool *const foldedp,
-                    const bool cache, const bool winheight)
+                    const bool cache, const bool limit_winheight)
 {
   bool folded = hasFoldingWin(wp, lnum, &lnum, nextp, cache, NULL);
   if (foldedp != NULL) {
     *foldedp = folded;
   }
-  return ((folded ? 1 : plines_win_nofill(wp, lnum, winheight)) +
+  return ((folded ? 1 : plines_win_nofill(wp, lnum, limit_winheight)) +
           (lnum == wp->w_topline ? wp->w_topfill : win_get_fill(wp, lnum)));
 }
 
@@ -207,19 +207,19 @@ int plines_win_full(win_T *wp, linenr_T lnum, linenr_T *const nextp, bool *const
 /// This takes care of both folds and topfill.
 ///
 /// XXX: Because of topfill, this only makes sense when first >= wp->w_topline.
-/// XXX: This limits the size of each line to current window height.
 ///
 /// @param first            first line number
 /// @param last             last line number
+/// @param limit_winheight  when true limit each line to window height
 ///
 /// @see win_text_height
-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, bool limit_winheight)
 {
   int count = 0;
 
   while (first <= last) {
     linenr_T next = first;
-    count += plines_win_full(wp, first, &next, NULL, false, true);
+    count += plines_win_full(wp, first, &next, NULL, false, limit_winheight);
     first = next + 1;
   }
   return count;

test/old/testdir/test_cursor_func.vim

@@ -128,38 +128,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))