On Monday, 2013-08-12, Jan Kundrát wrote: > On Sunday, 11 August 2013 11:57:59 CEST, Pali Rohár wrote: > > Both plugins passes information in signals params. This is > > unified. But only in ComposeWidget.cpp code I need sender() > > because pointer of Job is removed from queue. This is needed > > because ComposeWidget had sync API for completion and it was not > > easy to rewrite code to use new async addressbook job without > > sender(). > > Now I took a second look. I was under impression that you followed Kevin's > advice about always including the Job *job parameter in the jobs' signals.
My initial suggestion was strongly influenced by KJob's design [1] which only has one result signal. KJobs can have additional signals that provide result parts as they come in for example, but any use of any job can always rely on result(KJob*) being emitted. In both error and success cases. Since the jobs here do not follow any common pattern it should be OK handle this differently. > > I do not like this automagic idea of selecting plugins. I bet > > that in some specific situation & configuration it will not work as > > user expect... > > That's a very good point -- shipping a code which is as robust as possible > makes a lot of sense, and limitting needless options is a way to increase > this robustness. However, in this situation, I'm worried about whether the > plugins will get any usage when they are disabled by default. > > What do you propose? IMHO "autodetect" is fine as long as the result is presented to the user and the final choice being the user's. I guess it depends on the startup scenario: 1) startup finds a working plugin configuration in Trojita's config -> no detect, just continue to use configured plugins 2) plugin no longer available -> plugin detection, display usable plugins, include message that this was done because the previously configured plugin is unavailable. Allow exit without changing config (user might want to install missing plugin instead). 3) no plugin configuration but Trojita config found -> could be an upgrade, check existance of old storage. if exists and not empty, autoconfigure the default plugin 4) no Trojita config -> assume first time user, plugin detection Detection result should allow selection of any of the valid plugins, but should list the "most appropriate" first or preselected. Cheers, Kevin [1] Qt classes that are job like, e.g. QNetworkReply, also have only one result signal (QNetworkReply::finished()). In case of QNetworkManager/-Reply the reply's signal is parameter-less because there parametrized version is already in QNAM itself. -- Kevin Krammer, KDE developer, xdg-utils developer KDE user support, developer mentoring
signature.asc
Description: This is a digitally signed message part.
