vim / vim

The official Vim repository
https://www.vim.org
Vim License
36.44k stars 5.45k forks source link

Incsearch doesn't highlight '/' properly in some cases. #3637

Closed tacahiroy closed 4 years ago

tacahiroy commented 5 years ago

Incsearch highlights strings in the buffer incorrectly if you use other (non '/') separator for ex-commands. For example, Vim highlights only 'http:' with the pattern below:

:%s@http://

In this case, 'http://' supposed to be highlighted if it is found in the current buffer.

If the slashes are escaped, :%s@http:\/\/, it is highlighted as expected, however, in this case slashes are not needed to be escaped, so slashes should be highlighted with this pattern.

This behaviour can be seen any other ex-commands such as :global, :vglobal, :vimgrep, sort, etc.

Vim version

VIM - Vi IMproved 8.1 (2018 May 18, compiled Nov 26 2018 09:13:28)
Included patches: 1-547
chrisbra commented 5 years ago

I think we need to escape the cmdline pattern, when firstc == ':'.

--- a/src/ex_getln.c
+++ b/src/ex_getln.c
@@ -512,6 +512,7 @@ may_do_incsearch_highlighting(
     else
     {
        int search_flags = SEARCH_OPT + SEARCH_NOOF + SEARCH_PEEK;
+       char_u *p;

        cursor_off();   // so the user knows we're busy
        out_flush();
@@ -525,8 +526,13 @@ may_do_incsearch_highlighting(
        if (search_first_line != 0)
            search_flags += SEARCH_START;
        ccline.cmdbuff[skiplen + patlen] = NUL;
-       found = do_search(NULL, firstc == ':' ? '/' : firstc,
-                                ccline.cmdbuff + skiplen, count, search_flags,
+       if (firstc == ':')
+           p = vim_strsave_escaped(ccline.cmdbuff + skiplen, (char_u *)"/");
+       else
+           p = ccline.cmdbuff;
+
+       found = do_search(NULL, firstc == ':' ? '/' : firstc, p,
+                                 count, search_flags,
 #ifdef FEAT_RELTIME
                &tm, NULL
 #else
@@ -534,6 +540,9 @@ may_do_incsearch_highlighting(
 #endif
                );
        ccline.cmdbuff[skiplen + patlen] = next_char;
+       if (firstc == ':')
+           // need to free p again
+           vim_free(p);
        --emsg_off;

        if (curwin->w_cursor.lnum < search_first_line
tacahiroy commented 5 years ago

Thanks for the quick patch. Although this patch shouldn't be the final version, it fixes highlight for :s@http://, but it breaks highlight for the normal case like :s/http:\/\/.

chrisbra commented 5 years ago

ah, that makes it double escape the backslashes

brammool commented 5 years ago

Takahiro Yoshihara wrote:

Thanks for the quick patch. Although this patch shouldn't be the final version, it fixes highlight for :s@http://, but it breaks highlight for the normal case like :s/http:\/\/.

The problem appears to be that do_search() only knows about the / and ? delimiters, while :s allows for many others. Escaping is not going to fix that properly. We should probably pass the delimiter down to do_search() somehow.

-- How do you know when you have run out of invisible ink?

/// Bram Moolenaar -- Bram@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 ///

chrisbra commented 4 years ago

fixed by https://github.com/vim/vim/commit/c036e87bd7001238ab7cc5d9e30e59bbf989a5fd