D28192: WIP: Refactor dictionary runner

[Harald Sitter]

sitter added inline comments.

INLINE COMMENTS

> alex wrote in dictionaryrunner.cpp:90
> The current behavior is that KRunner closes when a match is selected.
> But if you set the data, for instance:
> `match.setData(QStringLiteral("Hello There!"));`
> The text `Hello There!` will show up in KRunner (like in the converter runner).
> The run method won't be called, because the match type is set to InformationalMatch.
> 
> So the runner would benefit from setData, or am I missing something?

Entirely possible, @broulik knows way more about the UI code. If data is used in the UI then that would run counter to its purpose though.

https://api.kde.org/frameworks/krunner/html/classPlasma_1_1QueryMatch.html#aad806b18a5fbec942f1258f6b4436cd8

  Sets data to be used internally by the associated AbstractRunner. 

putting a string into the UI is not internal anymore ^^

> alex wrote in dictionaryrunner_config.cpp:29
> The baseclass implementations for these methods handle the KConfigDialogManager widgets and the corresponding changed signals.
> I removed them, because the KConfigDialogManager class is not used in this kcm and the signals are manually handled.

That's an implementation detail though, is it not? From the outside we shouldn't make assumption about what the implementation does unless the documentation says what we can assume.
Today the baseclass may be useless, in 10 years it may not be.

Long-winded way of saying that I would leave the base class calls in. If nothing else it's at least better form in terms of API contracts.

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D28192

To: alex, broulik, ngraham, sitter, mlaurent
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200406/45cfc263/attachment-0001.html>