On Monday 08 July 2013 19:56:54 Kevin Krammer wrote: > On Monday, 2013-07-08, Pali Rohár wrote: > > On Monday 08 July 2013 16:37:31 Kevin Krammer wrote: > > > On Monday, 2013-07-08, Pali Rohár wrote: > > > > On Thursday 04 July 2013 18:38:40 Kevin Krammer wrote: > > > > > Hi, > > > > > > > > > > I had a look at the current state of the integration > > > > > clone. > > > > > > > > > > TrojitaPluginJob.h > > > > > > > > > > - autoDelete() and isRunning() should be const and > > > > > normal public method (not slots). > > > > > > > > Ok, changed. > > > > > > const > > > > > > You could keep setAutoDelete a slot, I was only referring > > > to the two getter methods :) > > > > It is really needed? > > Up to the Trojita developers I guess, since I don't know about > coding style in this code base. In KDE PIM getters are const > so that's why I mentioned it. > > > > I would advise against using unstranslatable strings in > > > the user interface. A better approach is to have an > > > identifier (used internally) and a name (displayed to the > > > user). > > > > And what to show when user choose plugin A and then plugin A > > will be deleted from system? In config file is store only > > plugin name > > If you want to display the plugin that has become unavailable > you could additionally store the name, no? > > The main problem with using the identifier as a user visible > string is that it is a foreign collection of symbols for > anyone with non-latin script. >
Ok, I changed code to store also description of prefered plugins to config file. Now only description is visible to user and name is only internal identifier. Patches pushed to my merge branch. > > > > > === > > > > > > > > > > KDE AddressBook > > > > > > > > > > - using deprecated KABC API parts, e.g. > > > > > KABC::AddressBook. Should use Akonadi API instead > > > > > > > > KABC::AddressBook still working fine (on KDE4.10) and is > > > > faster on my notebook than akonadi. I do not want hard > > > > dependency in Trojita on akonadi which slow down > > > > notebook-like devices. > > > > > > Well, the plugin is called "KDE", suggesting its purpose > > > is KDE integration :) This is also the topic of the GSoC > > > project. > > > > As I wrote in previous mail. I can create another KDE > > addressbook plugin which will use akonadi (and not kabc). > > If you want to have a vcard addressbook plugin then by all > means. If you want to use KDE's VCard library, again, by all > means. > > StdAddressBook is quite some overhead for just working with a > vcard file. > > Anyway, I was mostly commenting on this because the plugin in > the "KDE" directory, which I assume is for KDE integration. > > > I'm not using akonadi, but traditional kabc kresource (not > > kresource <--> akonadi compatibility plugin!). So I will use > > this non-akonadi KABC::AddressBook plugin in Trojita. > > Sure, there is no need to use Akonadi for a vcard addressbook. > I was talking about the KDE addressbook integration :) > > Additonally, as I wrote above, using KABC::AddressBook is, > first, an unnecessary overhead for handling a simple vcard > file and, second, at some point just a compatibility API on > top of Akonadi. > This plugin has suffix traditional. KABC::AddressBook is not limited to VCard directory, it also support other (remote) storages. Because it using KABC library for addressbook which is still part of KDE, I put it to KDE subdir. > > private: > > QHash<Job *, QEventLoop *> m_loops; > > QHash<Job *, result> m_results; > > > > New Password/Addressbook job with associated eventloop is > > inserted to m_loops. When job finish it will call slot. Slot > > will insert pair <job, result> to m_results, remove > > eventloop from m_loops and stop it. Then continue execution > > and result will be removed from m_results. Job, eventloop > > and this (ptr) are stored in QPointer for checking if > > somebody did not deleted them. > > Sounds better, but I wonder if it doesn't get into a similar > effort as doing it properly asynchronous. > > Cheers, > Kevin -- Pali Rohár [email protected]
signature.asc
Description: This is a digitally signed message part.
