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? > > > === > > > > > > Plugins > > > > > > I take it name() is used as an identifier and not > > > displayed to the user? > > > > Name is internal trojita plugin name (identifier). It is > > stored in config file, so trojita know which plugin will > > load (at startup). > > > > But in settings dialog is visible to user: > > name() + " - " + description() > > > > When specified plugin name is not found in setting dialog > > is: name() + " (not found)" > > > > I think it is also usefull for user to see plugin name. > > 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 > > > === > > > > > > 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. > > Now, using the "KDE Addressbook" means using Akonadi, because > this is what KDE's main addressbook interface, KAddressBook, > is using. > > This can be done using the Akonadi API or compatibility > plugins for KResource based API (KABC::AddressBook). > > Naturally the direct approach not only allows more > flexibility, it also removes the need for a compatibility > layer that has a hard time[1] mapping a synchronous API > (KResource) onto an asynchronous system (Akonadi). > > You can trust me on the latter because I am the author of said > compatibility plugins :) > > Using them will also increase the memory usage of the > application because in order to fulfil the API contract (load > all contacts) it will, well, load *all* contacts. > As I wrote in previous mail. I can create another KDE addressbook plugin which will use akonadi (and not kabc). 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. > > > - calling KAddressBook for adding/editing is probably not > > > a good idea either. There is a contact editor widget and > > > related UI components in kdepimlibs > > > > Hm, I did not know that. I will look at it. > > One of the classes is called ContactEditor [2]. > This is akonadi library, so it can be used in KDE Akonadi trojita plugin. > > > === > > > > > > EnvelopeView.cpp > > > > > > - contains a rather hackish nested event loop in > > > htmlizeAddresses(). nested event loops are evil, source of > > > tons of hard to fix crashes in old KDE apps > > > > You are right, but I think this event loop is correct and > > could not crash. Rewrite method htmlizeAddresses() in > > correct way is really hard, so I decided to port old code > > with local event loop now. > > How do you protect against reentrance? > If htmlizeAddresses() is called while the nested event loop is > running, a new instance will be assigned to the m_loop > member. > The first job to return will exit and delete the second > instance, the first loop will be "lost". > > If you really need to fake synchronous behavior I would > strongly adise on creating a JobRunner that takes the job and > runs it and collects its result. For the only safe way of > doing it see [1]. > 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. > Cheers, > Kevin > > [1] needs to run asynchronous operations in a secondary thread > to ensure it can really block the calling thread on blocking > API calls. > > [2] http://api.kde.org/4.x-api/kdepimlibs-> > apidocs/akonadi/contact/html/classAkonadi_1_1ContactEditor.htm > l -- Pali Rohár pali.ro...@gmail.com
signature.asc
Description: This is a digitally signed message part.
