Problem using cgn to change a search hit when replacement includes hit

[John Beckett]

Thanks Christian for the excellent gn added in 7.3.610. However,
using Vim 7.4.972 I'm seeing this problem with cgn after search.

Put the cursor at beginning of line:

abc x def x ghi x jkl

Type /x and press Enter to find first x.

Type cgn"x" and press Esc to replace search hit with "x" (three
characters).

Press n to find next x then press . to repeat change.

Expected result:

abc "x" def "x" ghi x jkl

Actual result:

abc "x" def "x" jkl

The problem occurs when the search is for a single character,
and the replacement text ("x" above) includes the search.

The problem can also be seen by simply trying cgn on the
second x.

John

[Christian Brabandt]

Hm, so this happens, when the replacement also matches the search
expression.

I see the problem in is_one_char():

,----
| 4793 nmatched = vim_regexec_multi(&regmatch, curwin, curbuf,
| 4794 pos.lnum, (colnr_T)0, NULL);
`----

Note the 0? That is the problem, vim tries to match from the first
column. On a first thought, it looks like replacing 0 with pos.col would
be correct, and while this fixes your example, it breaks for more
complex search patterns like in the test:

a:0\@!\zs\d\+
and a sample input like this:
a:10

because then pos.col would be at the first digit and the search pattern
would not match from there on. I don't know the best way to fix this. A
fix is attached, but it is ugly.

Best,
Christian
--
Nur eine Kette ist es, die uns gebunden hält: Die Liebe zum Leben.
-- Lucius Annaeus Seneca (an Lucilius)

[JohnBeckett]

Christian Brabandt wrote:
> ...

> A fix is attached, but it is ugly.

Thanks! I applied the patch and did some tests trying to break
it, but it works well.

I don't understand the code, but I can see that:

flag + SEARCH_KEEP + SEARCH_END

works because of the possible values in flag, and '+' is how it
was done earlier in "SEARCH_KEEP + flag", but why not replace
each '+' with '|'?

John

[Christian Brabandt]

Hi JohnBeckett!

On Mo, 14 Dez 2015, JohnBeckett wrote:

> Christian Brabandt wrote:
> > ...
> > A fix is attached, but it is ugly.
>
> Thanks! I applied the patch and did some tests trying to break
> it, but it works well.

I think, the attached patch is slightly better (more consistent and does
not mix 2 different search functions).

> I don't understand the code, but I can see that:
>
> flag + SEARCH_KEEP + SEARCH_END
>
> works because of the possible values in flag, and '+' is how it
> was done earlier in "SEARCH_KEEP + flag", but why not replace
> each '+' with '|'?

I believe I did this, because this seemed to be the style used for the
searchit() function elsewhere, so I kept this style for consistency.

Best,
Christian
--
In der Weltpolitik spritzt mancher, der sich als Feuerwehrmann
ausgibt, mit Benzin.
-- Bertrand A. W. Russell