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. > - I am not sure calling stop() in the destructor is a good > idea. It calls doStop() but at that stage the subclass has > already been destroyed (its destructor ran already). > Ok, I removed calling stop() from destructor. > === > > 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. > === > > 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. But if you (or other users/developers) want to see akonadi plugin in Trojita for addressbook, I will create one. Note that I'm *not* going to use any akonadi software/plugins due to big memory and cpu usage. Until akonadi client/server will not be fixed and ready for everyday use I think it is not good idea to slow down trojita only because akondi KDE integration. > - 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. > === > > 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. > - I think the best way forward is to run the jobs before > calling headerText(). for that you need to save the model > index as a QPersistantModelIndex. > > Then you create a list of email addresses to lookup and start > processing them. each lookup is stored in a hash, e.g. > QHash<QString, QStringList> (email -> names). once the list > has been processed, you call headerText() and > htmlizeAddresses() looks into the hash instead of running > jobs. > > When setText() has been called, you can clear the hash. > > Cheers, > Kevin Yes, this could work. I will look at it later. -- Pali Rohár [email protected]
signature.asc
Description: This is a digitally signed message part.
