make FindNext and FindPrevious work with empty matches

[matthias314]

This is a companion to #3566, as suggested by @dmaluka in this comment. If there is an empty match, then FindNext will move one rune to the right before starting the search and Findprevious will move one rune to the left.

The new field LastMatch of Buffer keeps track of the last match. I only initialize it after a search (that is, when LastSearch is non-empty).

When testing the PR, keep in mind that currently ^ always matches the current position when searching downwards and $ does the same when searching upwards. That should be addressed in a different PR.

[dmaluka]

This addresses FindNext and FindPrevious only, not Find. If the cursor is at the end of a line and we press Ctrl-f and search for $, the cursor still stays where it is rather than moves to the end of the next line.

Maybe that's ok, but maybe it means that we should take a simpler, "stateless" approach instead: if the match is empty and it is found at the current position (irrespective of the last match), advance the cursor and search again.

Or maybe an even simpler approach (which Vim seems to take): always start searching not from the current cursor location but from the character next to it. But that might be debatable whether that is what users want.

[matthias314]

Maybe that's ok

I would say it's OK. You start from the current location and find something that matches. If you allow empty matches, then you cannot complain when you get one.

[dmaluka]

I would say it's OK. You start from the current location and find something that matches. If you allow empty matches, then you cannot complain when you get one.

Ok, then irregardless of Find, what about the stateless approach instead of LastMatch? I'm hesitant to introduce this extra state that we need to carry around and remember about, if we can avoid it.

...Answering myself: it seems there is actually a good reason to keep it stateless and check the current cursor position rather than the last match. Press Ctrl-f and search for $, the cursor moves to the end of the line, now move the cursor a few characters to the left and press Ctrl-n -> the cursor unexpectedly moves to the next line instead of the end of the current line.

[matthias314]

what about the stateless approach instead of LastMatch?

I like that idea, but I see a problem: Suppose that you search for $. The first search brings you to the end of the current line. Then FindNext gets an empty match at the current cursor location and so moves right one rune. This brings you to the beginning of the next line. However, if that line is empty, then FindNext gets again an empty match at the current location and moves right again. As a result, empty lines will not be matched. If all lines are empty, then you are in an infinite loop.

If you search for ^ instead, then with the current (I would say, incorrect) behavior of ^, you get a match everywhere, and so you will be caught an infinite loop no matter what the buffer contains.

the cursor unexpectedly moves to the next line instead of the end of the current line

I think this is because there is no distinction between an empty selection and no selection. I agree that this is not good currently. However, as explained above, it seems to me that one needs some kind of state.

[dmaluka]

However, if that line is empty, then FindNext gets again an empty match at the current location and moves right again.

No, it doesn't move right again. I didn't mean that it moves right until it finds a non-empty match, I meant that it moves right just once, and searches again just once, not in a loop. Just so that it isn't stuck at the initial position.

And of course I didn't mean actually moving the cursor, I meant just moving the start location of search.

the cursor unexpectedly moves to the next line instead of the end of the current line

I think this is because there is no distinction between an empty selection and no selection.

I'm not sure what does this have to do with selection...

I agree that this is not good currently.

To be clear: this is not the current micro's behavior, this is a regression introduced by this PR.

[matthias314]

this is a regression introduced by this PR

Don't worry, that's what I meant. I've forced-pushed a new version along the lines of your idea.