"Replace next" incorrect behavior

[gusmanb]

The "replace next" function of the search panel implements an incorrect behavior when used more than once.

• Version:

AvaloniaEdit 11.0.0

• Simptoms:

If you have a text that contains multiple matches and replace one by one using the "Replace next" button it skips one of the matches each time.

• Cause:

Internally the "Replace next" function calls "FindNext", which searches one char ahead of the caret position, this behavior makes sense for the "FindNext" functionality but not for the replace as once you replace a match it moves the caret to the next one so the next time you press the "Replace next" button that match is skipped and the next one is replaced.

• Solution:

"Replace next" should use its own search criteria that searches from the caret position, not from the next position.

• Considerations:

"FindNext" also may present an erratic behavior because of the search-ahead, for example, if an user places the caret at the beginning of a file and the searched text is present starting at the first char the function will skip it and go to the next. The search-ahead behavior should be used only after the first "FindNext" is issued and if the user has not moved manually the caret.

[KrzysztofDusko]

@danipen will PR with "fast and dirty" fix will be accepted ?
Idea is simple, at this time we have

        public void ReplaceNext()
        {
            if (!IsReplaceMode) return;

            FindNext();
            if (!_textArea.Selection.IsEmpty)
            {
                _textArea.Selection.ReplaceSelectionWithText(ReplacePattern ?? string.Empty);
            }

            UpdateSearch();
        }
(...)
        public void FindNext()
        {
            var result = _renderer.CurrentResults.FindFirstSegmentWithStartAfter(_textArea.Caret.Offset + 1) ??
                         _renderer.CurrentResults.FirstSegment;
            if (result != null)
            {
                _currentSearchResultIndex = GetSearchResultIndex(_renderer.CurrentResults, result);
                SelectResult(result);
                UpdateSearchLabel();
            }
        }

we can fix this issue with allowing to add additional offset (negative) in FindNext like this:

        public void FindNext(int additionallOffset = 0)
        {
            var result = _renderer.CurrentResults.FindFirstSegmentWithStartAfter(_textArea.Caret.Offset + 1 + additionallOffset) ??
                         _renderer.CurrentResults.FirstSegment;
            if (result != null)
            {
                _currentSearchResultIndex = GetSearchResultIndex(_renderer.CurrentResults, result);
                SelectResult(result);
                UpdateSearchLabel();
            }
        }

and ReplaceNext :

        public void ReplaceNext()
        {
            if (!IsReplaceMode) return;

            FindNext(-1);
            if (!_textArea.Selection.IsEmpty)
            {
                _textArea.Selection.ReplaceSelectionWithText(ReplacePattern ?? string.Empty);
            }

            UpdateSearch();
        }

to be 100% precise

[danipen]

Haha It's a quite dirty hack... A better solution would be desired...

[KrzysztofDusko]

FindNextOrAcual method will be OK ? and this method will be really very simillar to Find next .. only difference will be no this "+1"

this approach leads to code duplication..

if this is bad idea too can you guide me a little ?

[KrzysztofDusko]

or insead off
public void FindNext(int additionallOffset = 0)
this ?
public void FindNext(bool startWithNoOffset = false)
and false = no +1 :)

[danipen]

IMO first step is taking a look to the upstream repo to see if the bug is there. If not we should review the port. If the bug is there too we can think a fix for it.

[KrzysztofDusko]

orginal implemenation do not have replace option.. or i cant find it..

and
but findnext have similar implementation witch is wrong for ReplaceNext purpose (but ok for finding and only this usage exists in AvalonEdit)

[KrzysztofDusko]

i maked PR for this issue
#392

[danipen]

@gusmanb please could you elaborate a little bit more on this, showing a video or an example?

"FindNext" also may present an erratic behavior because of the search-ahead, for example, if an user places the caret at the beginning of a file and the searched text is present starting at the first char the function will skip it and go to the next. The search-ahead behavior should be used only after the first "FindNext" is issued and if the user has not moved manually the caret.

[danipen]

Started PR #393.

[gusmanb]

Here is an example of what I meant. As you can see the first occurence of the word is skipped and it selects the next one.

search2.online-video-cutter.com.mp4

[danipen]

@gusmanb got it. Other editors place the caret at the end of the search result, so they then FindNext just from the caret position. I will mimic that behavior in AvaloniaEdit.

[danipen]

@gusmanb it should be fixed now. Could you please give it a try to #393. Thanks!

[gusmanb]

@danipen Search works as expected, selects the first occurence of the word, but replace next still skips one occurence each time (should it also be fixed in that pr?)

Desktop.2024.01.09.-.17.52.08.02.online-video-cutter.com.mp4

[danipen]

Yes, it should work in this PR. Taking a look again...

[danipen]

@gusmanb ok, it was broken with latest changes, it should work as expected now. Please can you give it a try? Thanks!

[gusmanb]

@danipen Excellent! Now everything works as expected, find next and replace next 👍