mawww / kakoune

mawww's experiment for a better code editor
http://kakoune.org
The Unlicense
9.85k stars 711 forks source link

[REGRESSION] An empty string as RE replaces/overwrites/erases current content of the search register #5076

Closed schragge closed 7 months ago

schragge commented 8 months ago

Version of Kakoune

v2023.08.05-190-g9b166e80

Reproducer

Consider the sequence s\w+<ret> ... s<ret> ... s<ret>. The first s selects whole words, then the second s selects them again. The third s should have selected them once again (this is how it worked in v2023.08.05), instead it doesn't select anything as the empty string from the second s replaced the RE and shifted it to history. For this to work, now I have to recall the RE from history: s\w+<ret> ... s<ret> ... s<c-p><ret>. And if s<ret> were a part of a macro that macro would now fail after the first iteration.

More annoyingly, now fails the following, relatively common to me idiom: */<ret>n. * puts selections to the search register, then /<ret> selects the next occurrence after each selection, then n moves the main selection to the next occurrence after it. For this to work, now I have to repeat *: */<ret>*n.

It gets even more annoying if we're dealing with a more complicated RE that cannot be easily reproduced with *. E.g.

/\d+<ret>/<ret>N

/\d+<ret> selects a number, then /<ret> selects the next number after each selection, then N additionally selects another number after the main selection. For this to work, now I have to

/\d+<ret>/<ret>Z,/<c-p><ret><a-z>a

Outcome

After a search command with an empty RE was once used (s<ret>, /<ret>, etc.), subsequent similar searches don't find anything.

Expectations

The original RE is being kept in the search register, so that subsequent n, N, s<ret>, /<ret>, etc. continue to work.

Additional information

I guess either https://github.com/mawww/kakoune/commit/a2c41593aabcf0dee8c91c08845d0fda5fd856c0 or https://github.com/mawww/kakoune/commit/c2fb0738d3171956c133a2a48bbecd3b5ec30a3c may be relevant. Or both.

krobelus commented 7 months ago

thanks for finding this; I think I had hit this before but didn't track it down.

A fix is

diff --git a/src/normal.cc b/src/normal.cc
index 902c98e45..1032a776c 100644
--- a/src/normal.cc
+++ b/src/normal.cc
@@ -866,7 +866,8 @@ void regex_prompt(Context& context, String prompt, char reg, T func)
                         RegisterManager::instance()[reg].set(context, str.str());
                     break;
                 case PromptEvent::Validate:
-                    RegisterManager::instance()[reg].set(context, str.str());
+                    if (not str.empty())
+                        RegisterManager::instance()[reg].set(context, str.str());
                     context.push_jump();
                     break;
                 }
schragge commented 7 months ago

Confirming your patch fixes the issue, thank you.