If the review is accepted and you want to close the bug, replace

Fixes bug 381080
https://bugs.kde.org/show_bug.cgi?id=381080

with simply

BUG: 381080

In D6473#121136, @ltoscano wrote:

If the review is accepted and you want to close the bug, replace

Fixes bug 381080
https://bugs.kde.org/show_bug.cgi?id=381080

with simply

BUG: 381080

Thanks!

I'm aware of that. I just wanted to make the lives of the reviewers a little bit easier :)

In D6473#121145, @jsalatas wrote:

In D6473#121136, @ltoscano wrote:

If the review is accepted and you want to close the bug, replace

Fixes bug 381080
https://bugs.kde.org/show_bug.cgi?id=381080

with simply

BUG: 381080

Thanks!

I'm aware of that. I just wanted to make the lives of the reviewers a little bit easier :)

But if the review is accepted as it is now, you would have to change it before committing. Better prepare it in the final form.

ping?

:)

Could you add a test in e.g. autotests/src/searchbar_test.cpp?

In D6473#124287, @kfunk wrote:

Could you add a test in e.g. autotests/src/searchbar_test.cpp?

Sure! will do it later...

I'm not an expert in the search code - in general your fix looks sane (had a look into the KateRegExp class). A unit test indeed would be helpful, since it will allow us to not break the search code again.

...thining a bit more about this: What I dislike is the fact that we don't seem to understand why it crashes in the first place, right? The problem is that the cursor of the view is not updated, so it is behind the document end in an invalid line. And moving the cursor up will again put it into an invalid line, which crashes.

So the question is: Why is the cursor not updated? If we can answer this, we may be able to either find that this patch is correct, or that the root of the real issue lies somewhere else...

In D6473#124300, @dhaumann wrote:

So the question is: Why is the cursor not updated? If we can answer this, we may be able to either find that this patch is correct, or that the root of the real issue lies somewhere else...

Hmmmm..... I need to go through that again and let you know....

https://phabricator.kde.org/source/ktexteditor/browse/master/src/render/katelayoutcache.cpp;ad51cece443bc6bb643fa4ca94293a13d3c2f852$321
The problem is `realLine >= m_renderer->doc()->lines()` it return a nullptr. First of all https://phabricator.kde.org/source/ktexteditor/browse/master/src/render/katelayoutcache.cpp;ad51cece443bc6bb643fa4ca94293a13d3c2f852$368 should be checked against nullptr, even all other places that line(..) is used or better handling of corner case.

ping? can this be merged? can a unit test be written?

About me when line(...) is accessed it should check for nullptr as well

if (auto l = line(a)) {
    return l->accessor();
}

https://phabricator.kde.org/source/ktexteditor/browse/master/src/render/katelayoutcache.cpp;ad51cece443bc6bb643fa4ca94293a13d3c2f852$322 because m_renderer->doc()->lines() is bigger than lines

Hm, can you send a patch that fixes this? :-)

I'm aware of that it will fix the crash but functionally still will be missing, but i'll give a try.