[vim/vim] Conceal: search in a small window leaves cursor in wrong position (#5012)

Tom M

unread,

Oct 3, 2019, 4:32:20 PM10/3/19

to vim/vim, Subscribed

Describe the bug
This is about searching for a string in a window that has less then 11 lines and the line of the match contains characters that are concealed via syntax or via matchadd(). One can hit this issue typically when running vim in a 24x80 terminal and splitting the screen horizontally to have 3 or more normal windows plus a help window and having all windows have the same height. Searches in such a small help window are affected by this.

To Reproduce

vim --clean (or gvim --clean)
:help eval.txt
:10wincmd _
/expr-!=#
Observe: A match is found but the cursor position is wrong: It's not at the beginning of the match (the 'e' char) but 3 chars further (on the 'r of the matched string).
press CTRL-L
Observe: The cursor jumps to the correct position - to the 'e' char.

Expected behavior
The cursor should end up on the 'e' character right after after the search in step 4.

Screenshots
N/A

Environment (please complete the following information):

Vim 8.1.2090
OS: Debian 10
Terminal: gnome-terminal

Additional context

Jumping to the beginning of the line with the match and starting the search again (i.e. /expr-!=# + 0n) leaves the cursor in correct position.
There is no issue if the (help) window has more than 10 lines.
Windows with normal buffer types are affected too - it's not only help windows.
Attaching a test:

" testdir/test_search_conceal.vim
"
source shared.vim
source screendump.vim
source check.vim

func Test_search_conceal_3_lines_window()
  CheckScreendump

  let lines =<< trim END
    3new
    set conceallevel=2 concealcursor=nc
    call setline(1, ['1', '2', '3', '/==expr=='])
    let m = matchadd('Conceal', '=')
  END
  call writefile(lines, 'Xcolesearch')
  let buf = RunVimInTerminal('-S Xcolesearch', #{rows: 10})
  call term_sendkeys(buf, "/expr\<CR>")
  call term_wait(buf)
  call VerifyScreenDump(buf, 'Test_cole_search', #{rows: 3, columne: 5})
  call StopVimInTerminal(buf)
  call delete('Xcolesearch')
endfunc

" dumps/Test_cole_search.dump
|2+0&#ffffff0| @73
|3| @73
|/>e|x|p|r| @69

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub

Tom M

unread,

Oct 3, 2019, 5:00:00 PM10/3/19

to vim/vim, Subscribed

There's a typo in the test script. The dict in the "VerifyScreenDump" line should have columns, not columne. But the test works nevertheless. Should the typo be corrected one would have to do a corresponding correction to the dump file so that it looks like this:

dumps/Test_cole_search.dump

|2+0&#ffffff0| @3
|3| @3
|/>e|x|p|r

I was not able to come up with an automated test without using terminal/screendump feature. The wrong cursor is a "ghost" cursor. Internally it's in the correct position. Vim script functions report correct column and CTRL-L confirms this.

Tom M

unread,

Nov 11, 2019, 3:05:37 PM11/11/19

to vim/vim, Subscribed

So this issue is broader. The window size limitation is due to 'scrolloff'. The problem is not bound to searching. Generally whenever a cursor jump into a concealed line involves a scroll the symptom appears. To fix the issue I propose the following patch. It contains a test. For convenience, the attachment contains the patch too.

Tom

diff --git a/src/drawscreen.c b/src/drawscreen.c
index 275925727..0e274464d 100644
--- a/src/drawscreen.c
+++ b/src/drawscreen.c
@@ -1398,7 +1398,11 @@ win_update(win_T *wp)
     int		i;
     long	j;
     static int	recursive = FALSE;	// being called recursively
-    int		old_botline = wp->w_botline;
+    linenr_T	old_botline = wp->w_botline;
+#ifdef FEAT_CONCEAL
+    int		old_wrow = wp->w_wrow;
+    int		old_wcol = wp->w_wcol;
+#endif
 #ifdef FEAT_FOLDING
     long	fold_count;
 #endif
@@ -2567,18 +2571,52 @@ win_update(win_T *wp)
 	wp->w_valid |= VALID_BOTLINE;
 	if (wp == curwin && wp->w_botline != old_botline && !recursive)
 	{
+	    win_T	*wwp;
+#if defined(FEAT_CONCEAL)
+	    linenr_T	old_topline = wp->w_topline;
+	    int		new_wcol = wp->w_wcol;
+#endif
 	    recursive = TRUE;
 	    curwin->w_valid &= ~VALID_TOPLINE;
 	    update_topline();	// may invalidate w_botline again
-	    if (must_redraw != 0)
+
+#if defined(FEAT_CONCEAL)
+	    if (old_wcol != new_wcol && (wp->w_valid & (VALID_WCOL|VALID_WROW))
+						    != (VALID_WCOL|VALID_WROW))
+	    {
+		// A win_line() call applied a fix to screen cursor column to
+		// accomodate concealment of cursor line, but in this call to
+		// update_topline() the cursor's row or column got invalidated.
+		// If they are left invalid, setcursor() will recompute them
+		// but there won't be any further win_line() call to re-fix the
+		// column and the cursor will end up misplaced.  So we call
+		// cursor validation now and reapply the fix again (or call
+		// win_line() to do it for us).
+		validate_cursor();
+		if (wp->w_wcol == old_wcol && wp->w_wrow == old_wrow
+			&& old_topline == wp->w_topline)
+		    wp->w_wcol = new_wcol;
+		else
+		    redrawWinline(wp, wp->w_cursor.lnum);
+	    }
+#endif
+	    // New redraw either due to updated topline or due to wcol fix.
+	    if (wp->w_redr_type != 0)
 	    {
 		// Don't update for changes in buffer again.
 		i = curbuf->b_mod_set;
 		curbuf->b_mod_set = FALSE;
+		j = curbuf->b_mod_xlines;
+		curbuf->b_mod_xlines = 0;
 		win_update(curwin);
-		must_redraw = 0;
 		curbuf->b_mod_set = i;
+		curbuf->b_mod_xlines = j;
 	    }
+	    // Other windows might have w_redr_type raised in update_topline().
+	    must_redraw = 0;
+	    FOR_ALL_WINDOWS(wwp)
+		if (wwp->w_redr_type > must_redraw)
+		    must_redraw = wwp->w_redr_type;
 	    recursive = FALSE;
 	}
     }
diff --git a/src/testdir/test_matchadd_conceal.vim b/src/testdir/test_matchadd_conceal.vim
index 6684378f6..83eadce5a 100644
--- a/src/testdir/test_matchadd_conceal.vim
+++ b/src/testdir/test_matchadd_conceal.vim
@@ -8,6 +8,8 @@ if !has('gui_running') && has('unix')
 endif
 
 source shared.vim
+source term_util.vim
+source view_util.vim
 
 func Test_simple_matchadd()
   new
@@ -277,3 +279,40 @@ func Test_matchadd_and_syn_conceal()
   call assert_notequal(screenattr(1, 11) , screenattr(1, 12))
   call assert_equal(screenattr(1, 11) , screenattr(1, 32))
 endfunc
+
+func Test_cursor_column_in_concealed_line_after_window_scroll()
+  CheckRunVimInTerminal
+
+  " Test for issue #5012 fix.
+  " For a concealed line with cursor, there should be no window's cursor
+  " position invalidation during win_update() after scrolling attempt that is
+  " not successful and no real topline change happens. The invalidation would
+  " cause a window's cursor position recalc outside of win_line() where it's
+  " not possible to take conceal into account.
+  let lines =<< trim END
+    3split
+    let m = matchadd('Conceal', '=')
+    setl conceallevel=2 concealcursor=nc
+    normal gg
+    "==expr==
+  END
+  call writefile(lines, 'Xcolesearch')
+  let buf = RunVimInTerminal('Xcolesearch', {})
+
+  " Jump to something that is beyond the bottom of the window,
+  " so there's a scroll down.
+  call term_sendkeys(buf, ":so %\<CR>")
+  call term_sendkeys(buf, "/expr\<CR>")
+  call term_wait(buf)
+
+  " Are the concealed parts of the current line really hidden?
+  let cursor_row = term_scrape(buf, '.')->map({_, e -> e.chars})->join('')
+  call assert_equal('"expr', cursor_row)
+
+  " BugFix check: Is the window's cursor column properly updated for hidden
+  " parts of the current line?
+  call assert_equal(2, term_getcursor(buf)[1])
+
+  call StopVimInTerminal(buf)
+  call delete('Xcolesearch')
+endfunc

fix-conceal-cursor-pos.patch.txt

—
You are receiving this because you are subscribed to this thread.

Reply to this email directly, view it on GitHub, or unsubscribe.

Bram Moolenaar

unread,

Nov 11, 2019, 5:22:26 PM11/11/19

to vim/vim, Subscribed

Tom wrote:

> So this issue is broader. The window size limitation is due to
> '`scrolloff`'. The problem is not bound to searching. Generally
> whenever a cursor jump into a concealed line involves a scroll the
> symptom appears. To fix the issue I propose the following patch. It
> contains a test. For convenience, the attachment contains the patch
> too.

Can you create a pull request, so that it triggers the CI tests?

--
hundred-and-one symptoms of being an internet addict:
71. You wonder how people walk

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

—
You are receiving this because you are subscribed to this thread.

Reply to this email directly, view it on GitHub, or unsubscribe.