Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
patch 8.1.0271: 'incsearch' doesn't work for :s, :g or :v
Problem:    'incsearch' doesn't work for :s, :g or :v.
Solution:   Also use 'incsearch' for other commands that use a pattern.
  • Loading branch information
brammool committed Aug 11, 2018
1 parent b31a3ac commit b0acacd
Show file tree
Hide file tree
Showing 5 changed files with 180 additions and 20 deletions.
129 changes: 112 additions & 17 deletions src/ex_getln.c
Expand Up @@ -264,11 +264,78 @@ set_search_match(pos_T *t)

/*
* Return TRUE when 'incsearch' highlighting is to be done.
* Sets search_first_line and search_last_line to the address range.
*/
static int
do_incsearch_highlighting(int firstc)
do_incsearch_highlighting(int firstc, incsearch_state_T *is_state,
int *skiplen, int *patlen)
{
return p_is && !cmd_silent && (firstc == '/' || firstc == '?');
*skiplen = 0;
*patlen = ccline.cmdlen;

if (p_is && !cmd_silent)
{
// by default search all lines
search_first_line = 0;
search_last_line = MAXLNUM;

if (firstc == '/' || firstc == '?')
return TRUE;
if (firstc == ':')
{
char_u *cmd = skip_range(ccline.cmdbuff, NULL);
char_u *p;
int delim;
char_u *end;

if (*cmd == 's' || *cmd == 'g' || *cmd == 'v')
{
// Skip over "substitute" to find the pattern separator.
for (p = cmd; ASCII_ISALPHA(*p); ++p)
;
if (*p != NUL)
{
delim = *p++;
end = skip_regexp(p, delim, p_magic, NULL);
if (end > p)
{
char_u *dummy;
exarg_T ea;
pos_T save_cursor = curwin->w_cursor;

// found a non-empty pattern
*skiplen = (int)(p - ccline.cmdbuff);
*patlen = (int)(end - p);

// parse the address range
vim_memset(&ea, 0, sizeof(ea));
ea.line1 = 1;
ea.line2 = 1;
ea.cmd = ccline.cmdbuff;
ea.addr_type = ADDR_LINES;
parse_cmd_address(&ea, &dummy);
curwin->w_cursor = is_state->search_start;
if (ea.addr_count > 0)
{
search_first_line = ea.line1;
search_last_line = ea.line2;
}
else if (*cmd == 's')
{
// :s defaults to the current line
search_first_line = curwin->w_cursor.lnum;
search_last_line = curwin->w_cursor.lnum;
}

curwin->w_cursor = save_cursor;
return TRUE;
}
}
}
}
}

return FALSE;
}

/*
Expand All @@ -280,14 +347,16 @@ may_do_incsearch_highlighting(
long count,
incsearch_state_T *is_state)
{
int skiplen, patlen;
int i;
pos_T end_pos;
struct cmdline_info save_ccline;
#ifdef FEAT_RELTIME
proftime_T tm;
#endif
int c;

if (!do_incsearch_highlighting(firstc))
if (!do_incsearch_highlighting(firstc, is_state, &skiplen, &patlen))
return;

// If there is a character waiting, search and redraw later.
Expand All @@ -298,12 +367,19 @@ may_do_incsearch_highlighting(
}
is_state->incsearch_postponed = FALSE;

// start at old position
curwin->w_cursor = is_state->search_start;
if (search_first_line == 0)
// start at the original cursor position
curwin->w_cursor = is_state->search_start;
else
{
// start at the first line in the range
curwin->w_cursor.lnum = search_first_line;
curwin->w_cursor.col = 0;
}
save_last_search_pattern();

// If there is no command line, don't do anything.
if (ccline.cmdlen == 0)
if (patlen == 0)
{
i = 0;
set_no_hlsearch(TRUE); // turn off previous highlight
Expand All @@ -322,15 +398,24 @@ may_do_incsearch_highlighting(
#endif
if (!p_hls)
search_flags += SEARCH_KEEP;
i = do_search(NULL, firstc, ccline.cmdbuff, count, search_flags,
c = ccline.cmdbuff[skiplen + patlen];
ccline.cmdbuff[skiplen + patlen] = NUL;
i = do_search(NULL, firstc == ':' ? '/' : firstc,
ccline.cmdbuff + skiplen, count, search_flags,
#ifdef FEAT_RELTIME
&tm, NULL
#else
NULL, NULL
#endif
);
ccline.cmdbuff[skiplen + patlen] = c;
--emsg_off;

if (curwin->w_cursor.lnum < search_first_line
|| curwin->w_cursor.lnum > search_last_line)
// match outside of address range
i = 0;

// if interrupted while searching, behave like it failed
if (got_int)
{
Expand Down Expand Up @@ -369,8 +454,11 @@ may_do_incsearch_highlighting(

// Disable 'hlsearch' highlighting if the pattern matches everything.
// Avoids a flash when typing "foo\|".
c = ccline.cmdbuff[skiplen + patlen];
ccline.cmdbuff[skiplen + patlen] = NUL;
if (empty_pattern(ccline.cmdbuff))
set_no_hlsearch(TRUE);
ccline.cmdbuff[skiplen + patlen] = c;

validate_cursor();
// May redraw the status line to show the cursor position.
Expand Down Expand Up @@ -398,25 +486,27 @@ may_do_incsearch_highlighting(
*/
static int
may_adjust_incsearch_highlighting(
int firstc,
long count,
int firstc,
long count,
incsearch_state_T *is_state,
int c)
int c)
{
int skiplen, patlen;
pos_T t;
char_u *pat;
int search_flags = SEARCH_NOOF;
int i;
int save;

if (!do_incsearch_highlighting(firstc))
if (!do_incsearch_highlighting(firstc, is_state, &skiplen, &patlen))
return OK;
if (ccline.cmdlen == 0)
if (patlen == 0 && ccline.cmdbuff[skiplen] == NUL)
return FAIL;

if (firstc == ccline.cmdbuff[0])
if (firstc == ccline.cmdbuff[skiplen])
pat = last_search_pattern();
else
pat = ccline.cmdbuff;
pat = ccline.cmdbuff + skiplen;

save_last_search_pattern();
cursor_off();
Expand All @@ -435,17 +525,20 @@ may_adjust_incsearch_highlighting(
if (!p_hls)
search_flags += SEARCH_KEEP;
++emsg_off;
save = pat[patlen];
pat[patlen] = NUL;
i = searchit(curwin, curbuf, &t,
c == Ctrl_G ? FORWARD : BACKWARD,
pat, count, search_flags,
RE_SEARCH, 0, NULL, NULL);
--emsg_off;
pat[patlen] = save;
if (i)
{
is_state->search_start = is_state->match_start;
is_state->match_end = t;
is_state->match_start = t;
if (c == Ctrl_T && firstc == '/')
if (c == Ctrl_T && firstc != '?')
{
// Move just before the current match, so that when nv_search
// finishes the cursor will be put back on the match.
Expand Down Expand Up @@ -493,7 +586,9 @@ may_adjust_incsearch_highlighting(
static int
may_add_char_to_search(int firstc, int *c, incsearch_state_T *is_state)
{
if (!do_incsearch_highlighting(firstc))
int skiplen, patlen;

if (!do_incsearch_highlighting(firstc, is_state, &skiplen, &patlen))
return FAIL;

// Add a character from under the cursor for 'incsearch'.
Expand All @@ -507,7 +602,7 @@ may_add_char_to_search(int firstc, int *c, incsearch_state_T *is_state)
// If 'ignorecase' and 'smartcase' are set and the
// command line has no uppercase characters, convert
// the character to lowercase.
if (p_ic && p_scs && !pat_has_uppercase(ccline.cmdbuff))
if (p_ic && p_scs && !pat_has_uppercase(ccline.cmdbuff + skiplen))
*c = MB_TOLOWER(*c);
if (*c != NUL)
{
Expand Down
10 changes: 7 additions & 3 deletions src/globals.h
Expand Up @@ -345,9 +345,13 @@ EXTERN int t_colors INIT(= 0); /* int value of T_CCO */
* a match within one line), search_match_endcol the column number of the
* character just after the match in the last line.
*/
EXTERN int highlight_match INIT(= FALSE); /* show search match pos */
EXTERN linenr_T search_match_lines; /* lines of of matched string */
EXTERN colnr_T search_match_endcol; /* col nr of match end */
EXTERN int highlight_match INIT(= FALSE); // show search match pos
EXTERN linenr_T search_match_lines; // lines of of matched string
EXTERN colnr_T search_match_endcol; // col nr of match end
#ifdef FEAT_SEARCH_EXTRA
EXTERN linenr_T search_first_line INIT(= 0); // for :{FIRST},{last}s/pat
EXTERN linenr_T search_last_line INIT(= MAXLNUM); // for :{first},{LAST}s/pat
#endif

EXTERN int no_smartcase INIT(= FALSE); /* don't use 'smartcase' once */

Expand Down
7 changes: 7 additions & 0 deletions src/screen.c
Expand Up @@ -7892,6 +7892,13 @@ next_search_hl(
long nmatched;
int save_called_emsg = called_emsg;

// for :{range}s/pat only highlight inside the range
if (lnum < search_first_line || lnum > search_last_line)
{
shl->lnum = 0;
return;
}

if (shl->lnum != 0)
{
/* Check for three situations:
Expand Down
52 changes: 52 additions & 0 deletions src/testdir/test_search.vim
Expand Up @@ -362,6 +362,58 @@ func Test_search_cmdline3()
bw!
endfunc

func Cmdline3_prep()
" need to disable char_avail,
" so that expansion of commandline works
call test_override("char_avail", 1)
new
call setline(1, [' 1', ' 2 the~e', ' 3 the theother'])
set incsearch
endfunc

func Cmdline3_cleanup()
set noincsearch
call test_override("char_avail", 0)
bw!
endfunc

func Test_search_cmdline3s()
if !exists('+incsearch')
return
endif
call Cmdline3_prep()
1
call feedkeys(":%s/the\<c-l>/xxx\<cr>", 'tx')
call assert_equal(' 2 xxxe', getline('.'))

call Cmdline3_cleanup()
endfunc

func Test_search_cmdline3g()
if !exists('+incsearch')
return
endif
call Cmdline3_prep()
1
call feedkeys(":g/the\<c-l>/d\<cr>", 'tx')
call assert_equal(' 3 the theother', getline(2))

call Cmdline3_cleanup()
endfunc

func Test_search_cmdline3v()
if !exists('+incsearch')
return
endif
call Cmdline3_prep()
1
call feedkeys(":v/the\<c-l>/d\<cr>", 'tx')
call assert_equal(1, line('$'))
call assert_equal(' 2 the~e', getline(1))

call Cmdline3_cleanup()
endfunc

func Test_search_cmdline4()
if !exists('+incsearch')
return
Expand Down
2 changes: 2 additions & 0 deletions src/version.c
Expand Up @@ -794,6 +794,8 @@ static char *(features[]) =

static int included_patches[] =
{ /* Add new patch number below this line */
/**/
271,
/**/
270,
/**/
Expand Down

11 comments on commit b0acacd

@chdiza
Copy link

@chdiza chdiza commented on b0acacd Aug 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a doc update.

People who want incsearch for / but not for the others are going to be very unpleasantly surprised.

@brammool
Copy link
Contributor Author

@brammool brammool commented on b0acacd Aug 11, 2018 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bstaletic
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's one reason why one would want incsearch for /, but not for :s (assuming it moves the cursor to the first occurrence of pattern):

  • The user sees something on the screen that he wants to use as the substitution string for a pattern that is in the clipboard.
  • The user types :s/CTRL_SHIFT_v.
  • The screen scrolls and <C-r><C-w> doesn't work now.

@tonymec
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want 'incsearch' for true seaches (/ and ?) but not for :s then what about

au CmdLineEnter : let s:incs = &incsearch
        \| set noincsearch
au CmdLineLeave : let &incsearch = s:incs

?

Best regards,
Tony.

@brammool
Copy link
Contributor Author

@brammool brammool commented on b0acacd Aug 14, 2018 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bstaletic
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does that do? CTRL-V starts literal insert.

Ctrl + Shift + V pastes inside a terminal. You could as well reach for <C-r>+.

Doesn't the same apply to "/"?

Kind of. The behaviour is the same, but in the example above the whole regex is typed by hand. So assumption is that it matters less for /.

@brammool
Copy link
Contributor Author

@brammool brammool commented on b0acacd Aug 14, 2018 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bstaletic
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this trade-off I would say that having :s/pat do the same as /pat is good.

That's a fair point.

We could add yet another option to specify exactly where 'incsearch' applies, but I'm really trying to avoid more options.

Yes, adding more options does sound like something that should be avoided. For the record, I'm already using a plugin that does what this patch does plus live substitution, so far I've been rehashing what I've discussed with other people.

I could easily rephrase the concerns about incsearch to point out that it comes down to whether the cursor is moved from the initial position or not. So another posibility is adding another pattern like \C or \V... Personally, I'm not a fan of this solution either.

I guess we should wait for further feedback.

@brammool
Copy link
Contributor Author

@brammool brammool commented on b0acacd Aug 14, 2018 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bstaletic
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brammool
Copy link
Contributor Author

@brammool brammool commented on b0acacd Aug 14, 2018 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.