Do not assume that list of documents is non-empty
ClosedPublic

Authored by tfry on Mon, Jan 13, 11:00 AM.

Details

Summary

Host applications may not always have an active view, and a non-empty list of documents. This would currently crash when clicking the "Search"-button with "Current File" or "Open Files".

Test Plan

Tested with plugin loaded in RKWard, and no documents present. Segfault without patch, no matches, with patch.

Diff Detail

Repository
R40 Kate
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
tfry created this revision.Mon, Jan 13, 11:00 AM
Restricted Application added a project: Kate. · View Herald TranscriptMon, Jan 13, 11:00 AM
Restricted Application added a subscriber: kwrite-devel. · View Herald Transcript
tfry requested review of this revision.Mon, Jan 13, 11:00 AM
cullmann accepted this revision.Mon, Jan 13, 11:11 AM

Seems reasonable for me.

This revision is now accepted and ready to land.Mon, Jan 13, 11:11 AM
sars accepted this revision.Mon, Jan 13, 11:13 AM
sars added a subscriber: sars.
sars added inline comments.
addons/search/plugin_search.cpp
1099

I would format the "if" like this even if it is only one line.

if () {

foo;

}

It is easier to set a breakpoint in a debugger...

sars added a comment.Mon, Jan 13, 11:14 AM

But I'm fine without the changes also :)

tfry updated this revision to Diff 73398.Mon, Jan 13, 12:23 PM

Updated diff

tfry added a comment.Mon, Jan 13, 12:27 PM

Hm, any hint, why I'm getting this?

remote: FATAL: W refs/heads/master kate tfry DENIED by refs/.*
remote: error: hook declined to update refs/heads/master
To git.kde.org:kate
 ! [remote rejected]     3b2561e34af7387b4722e108e7aae648563e5d69 -> master (hook declined)
error: failed to push some refs to 'git@git.kde.org:kate'

I think you pushed to the wrong url, you need to use the one from https://invent.kde.org/kde/kate

git@invent.kde.org:kde/kate.git

This revision was automatically updated to reflect the committed changes.
tfry added a comment.Mon, Jan 13, 12:38 PM

Thanks. That was it.

I was using the default push url from kdesrc-build (which seemed sane to me). Not sure if the push url is configurable, there.