[vim/vim] fix(incsearch): support `c_CTRL-{G,T}` with an offset (PR #19998)
Barrett Ruth
Closes #19991.
Problem
`c_CTRL-{G,T}` stop cycling once an `'incsearch'` command includes an offset such as `/blah/e`. `do_search()` already parses the pattern and offset separately but `may_adjust_incsearch_highlighting()` cycles on the raw cmdline text instead.
Solution
Extract the pattern/offset parsing block from `do_search()` into a new helper `parse_search_pattern_offset()` and use it in the command-line incsearch path so `c_CTRL-{G,T}` keep selecting matches.
Ports neovim/neovim#39097.
Note: this only fixes the cycling. The `'incsearch'` highlight still does not honour the |search-offset| — that's the second half of the original report and is left for a follow-up.
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/19998
Commit Summary
- 14ff242 fix(incsearch): support `c_CTRL-{G,T}` with an offset.
File Changes
(5 files)
- M runtime/doc/cmdline.txt (5)
- M src/ex_getln.c (59)
- M src/proto/search.pro (1)
- M src/search.c (167)
- M src/testdir/test_search.vim (22)
Patch Links:
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Barrett Ruth
@barrettruth commented on this pull request.
In src/search.c:
> @@ -1215,6 +1215,109 @@ first_submatch(regmmatch_T *rp) } #endif +/*
entire block copy-pasted, comments included (no changes)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Copilot
@Copilot commented on this pull request.
Pull request overview
Fixes command-line 'incsearch' match-cycling (c_CTRL-G / c_CTRL-T) when the search text includes a |search-offset| (e.g. /pat/e+1) by reusing the same pattern/offset parsing logic as do_search().
Changes:
- Extracted search pattern + optional offset parsing into a shared helper
parse_search_pattern_offset(). - Updated the command-line incsearch cycling path to use the parsed pattern (excluding offsets) when invoking
searchit(). - Added a regression test and updated cmdline help text for
c_CTRL-G/c_CTRL-T.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file| File | Description |
|---|---|
src/search.c |
Introduces parse_search_pattern_offset() and switches do_search() to call it. |
src/ex_getln.c |
Uses the new helper in may_adjust_incsearch_highlighting() so Ctrl-G/T cycling ignores offsets correctly. |
src/proto/search.pro |
Exposes the new helper prototype for cross-file use. |
src/testdir/test_search.vim |
Adds a test covering Ctrl-G/T cycling with a search offset. |
runtime/doc/cmdline.txt |
Updates documentation wording for Ctrl-G/T behavior with offsets. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
In src/ex_getln.c:
> + pat = ccline.cmdbuff + skiplen;
+ searchstr = pat;
+ searchstrlen = (size_t)patlen;
+ patlen_s = (size_t)patlen;
// do not search for the search end delimiter,
// unless it is part of the pattern
- if (patlen > 2 && firstc == pat[patlen - 1])
- {
- patlen--;
- if (pat[patlen - 1] == '\\')
- {
- pat[patlen - 1] = firstc;
- bslsh = TRUE;
- }
- }
+ (void)parse_search_pattern_offset(&pat, &patlen_s, search_delim,
+ SEARCH_OPT, &strcopy, &searchstr,
+ &searchstrlen, &dircp, &offset);
parse_search_pattern_offset() assumes *patlen covers the full remaining string (pattern + optional delimiter/offset + trailing text). Here patlen_s is initialized from patlen returned by do_incsearch_highlighting(), which for :-commands is only the pattern length (up to the delimiter). If the delimiter is ? and skip_regexp_ex() allocates strcopy, cmdlen += (int)(*patlen - len) will underflow because len becomes the length of the whole copied tail, not just the pattern. Suggest setting patlen_s to the length of the remaining cmdline text (e.g. STRLEN(ccline.cmdbuff + skiplen) or ccline.cmdlen - skiplen) before calling parse_search_pattern_offset(), instead of using the parsed pattern length.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Barrett Ruth
@barrettruth pushed 1 commit.
- 7032825 fix(incsearch): support `c_CTRL-{G,T}` with an offset
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
Barrett Ruth
@barrettruth commented on this pull request.
In src/ex_getln.c:
> + pat = ccline.cmdbuff + skiplen;
+ searchstr = pat;
+ searchstrlen = (size_t)patlen;
+ patlen_s = (size_t)patlen;
// do not search for the search end delimiter,
// unless it is part of the pattern
- if (patlen > 2 && firstc == pat[patlen - 1])
- {
- patlen--;
- if (pat[patlen - 1] == '\\')
- {
- pat[patlen - 1] = firstc;
- bslsh = TRUE;
- }
- }
+ (void)parse_search_pattern_offset(&pat, &patlen_s, search_delim,
+ SEARCH_OPT, &strcopy, &searchstr,
+ &searchstrlen, &dircp, &offset);
update call site to pass the remaining cmdline length into parse_search_pattern_offset()
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Barrett Ruth
@barrettruth pushed 1 commit.
- 5fe7810 fix(incsearch): support `c_CTRL-{G,T}` with an offset
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
h_east
@h-east commented on this pull request.
> @@ -466,14 +466,13 @@ CTRL-L A match is done on the pattern in front of the cursor. If *c_CTRL-G* */_CTRL-G* CTRL-G When 'incsearch' is set, entering a search pattern for "/" or "?" and the current match is displayed then CTRL-G will move - to the next match (does not take |search-offset| into account) + to the next match. The |search-offset| has no effect.
This sentence might be misleading, so how about something like this?
⬇️ Suggested change- to the next match. The |search-offset| has no effect. + to the next match. The |search-offset| is applied when <CR> + is pressed, but does not affect the preview highlighting.
> Use CTRL-T to move to the previous match. Hint: on a regular keyboard G is below T. *c_CTRL-T* */_CTRL-T* CTRL-T When 'incsearch' is set, entering a search pattern for "/" or "?" and the current match is displayed then CTRL-T will move - to the previous match (does not take |search-offset| into - account). + to the previous match. The |search-offset| has no effect.
This sentence might be misleading, so how about something like this?
⬇️ Suggested change- to the previous match. The |search-offset| has no effect. + to the previous match. The |search-offset| is applied when + <CR> is pressed, but does not affect the preview highlighting.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.