On Monday 05 August 2013 19:00:40 Jan Kundrát wrote: > On Saturday, 13 July 2013 12:45:18 CEST, Pali Rohár wrote: > >> - Why are TrojitaPluginJob::start/stop postponing the > >> actual operation via the event loop? I'm not saying it's > >> bad, but I'm wondering why. A comment explaining this > >> would be great. > > > > Kevin suggested that. To have async behaviour and also make > > sure that start() method does not emit job finished signals > > (which simplify usage). > > e-mails : Jan -> 1:0. Sorry, I missed this mail. > > Could you please add a comment about this to the generic > plugin .cpp? >
Ok. > >> - Why are you employing the singletton (anti)pattern within > >> the PluginLoader? What are the advantages of using it (i.e. > >> a global variable in disguise) over an ordinary member of > >> the MainWindow? > > > > Access to PluginLoader is needed also from other classes, > > not only from MainWindow. > > I'd prefer to have this access explicit -- i.e. the other > places should be provided with either a pointer to MainWindow > or to the plugin loader itself. Hiding this into a singleton > pattern is only making the whole thing less opaque. > Ok, I will try to change trojita code. > > Plugins are shared libraries (.so, .dll, ...) and on more > > systems are installed to common library directory. > > > > And problem is: How to get this common library directory > > from Qt? > > > > QLibraryInfo::LibrariesPath is directory of qt libraries not > > our trojita plugins! On some systems are qt libraries not > > installed in common library directory. > > > > Maybe better solution could be to get prefix/install library > > directory from cmake and pass it to cpp code via compiler > > define. > > Yup, that's what I'd like to have. Again, sorry for note > seeing this mail earlier :(. > Ok, I will look at cmake and try to use cmake library path for plugins dir. > >> - The plugin loader contains extra qDebug; these shall be > >> removed or #ifdefed. > > > > These qDebugs are usefull for debugging, so I'd like to have > > them. Is there any common #ifdef name for qDebug? > > In Trojita, these debugging stanzas are done in an ad-hoc, > per-file manner. Perhaps a commented-out #define PLUGIN_DEBUG > at the top of the PluginLoader.cpp file? > Ok > > My current code allows that library can support more plugins > > via one object. But I do not know if it is usefull or > > not... > > Interesting, but would that work in practice -- wiht one > create() method and all? > I do not know... Maybe it is not good idea to use plugins in this way... > >> - Why is there that feature for "reloading plugins"? > > > > It is called from SettingsDialog when user change plugin > > from menu. It is needed because if he change plugin in menu > > also passwords should be reloaded/stored to new plugin. > > OK, so it's for changing the "current implementation" of each > feature -- makes sense, thanks. > > >> - I guess packagers would appreciate a flag for only > >> building a single plugin at a time (or am I wrong?) > > > > Maybe CMakeLists.txt can start using option() for all > > switches? > > I'm not familiar with this, but it looks good -- patches > welcome. > Ok, I will change cmake configuration into option blocks. > >> - I agree with only showing the plugin description in the > >> settings dialog, but I'm not sure why do you need to store > >> both name and description in the settings file. > > > > This was already discussed. When user select plugin A, then > > close trojita and uninstall plugin A. Trojita will show in > > settings dialog that plugin A is missing. But Trojita > > should not show internal plugin id, but transtaled > > description. So this is reason why in config file is stored > > also last transtaled description. If user install plugin A > > again, then Trojita after next (re)start will use it. > > I'm not convinced that it's worth the extra code to keep track > of the human-readable descriptions. But well, the code is > written already, so I guess it shall be left in now. > > > Hm, I do not know qt test api. I will look at existing > > tests. What is needed to test? If addressbook and password > > jobs return correct data? > > I'm more interested with the *interaction* of the individual > bits -- it's good to know whether the IMAP code can actually > request the password from the plugin or what happens when the > plugin takes too long to respond. > > Sorry for missing this mail again. > > Cheers, > Jan -- Pali Rohár pali.ro...@gmail.com
signature.asc
Description: This is a digitally signed message part.
