Re: [trojita] Re: GSoC merge request
On Monday 08 July 2013 22:04:33 Caspar Schutijser wrote: > Hi Pali, > > On Monday, July 8, 2013 6:10:36 PM CEST, Pali Rohár wrote: > > today I cleaned my git tree and created new (rebased) branch > > which is ready for comments and then for merging to master. > > One piece of feedback for now. > > > If I do this: > 1) Go to Settings. > 2) Select "None - Disable passwords" at the "Passwords" > select box. 3) Click the "Save" button. > > Then I get two errors: > * "Storing IMAP password failed". > * "Storing SMTP password failed". > > I do not store my IMAP or SMTP password so the fields are > empty. > I fixed this problem. Also that one which caused that tests was not possible to compile. Patches pushed to my same git branch. > > Shortly I will have a look at the diff. If I find some > problems I will report them. Hopefully others have some time > as well to look at it :) > > On my computer I don't use KDE so I will see how I can test > some KDE-specific parts of the new code. Maybe I should fire > up a virtual machine for that. > > Best regards, > Caspar Schutijser -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: [trojita] GSoC review
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 m_loops; > > QHash 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 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 pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: [trojita] GSoC review
On Wednesday, 2013-07-10, Pali Rohár wrote: > On Monday 08 July 2013 19:56:54 Kevin Krammer wrote: > > 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. ok, great! > > 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. Hmm. I guess I am not writing it understandbly enough, let me try a different phrasing :) KABC::AddressBook is one of the frontend APIs of the KResource framework. The KResource framework has been deprecated a couple of years back, backends being gradually reimplemented in terms of the Akonadi client framework. It is already possible to build without KResources using a build time configure switch, but of course due to API and ABI compatibilty guarantees this is not applied on distributions packages until the next major revision. However, developing something against a deprecated framework is usually a waste of time, but at least needs to be done with understanding of the consequences. 1) Anyone with a KDE setup started with 4.4 or later will either not have addressbook access through KABC::AddressBook or through a compatibility plugin 2) KABC::AddressBook, while staying available as an API (see above), will be implemented in terms of the new framework once the new framework has reached backend availability equality. Then only backend accessor in KABC that is currently not available yet in Akoandi is LDAP addressbook. A prototype is being worked on right now, my guess is it will be releasable for 4.12. Whether KABC::AddressBook will be implemented directly using the new access framework or whether the existing compatibility plugin will be used is not yet decided as far as I know. My guess is the latter (already exists only trivial change to resource factory required). In my initial assessment I was assuming that the goal of the KDE plugin, especially in the context of the GSoC project, was to enable users of Trojita and using KDE PIM for addressbook to access addressbook data in Trojita. Most of those users will like not run something old as RHEL6 or Debian oldstable, but more likely something like recent Kubuntu, OpenSUSE, Fedora, Debian, etc. Creating a plugin for really old distribution versions or RHEL6 (or older) might be a worthwhile goal on its own, I am just not sure it should be the done as part of the GSoC scope. Do we have any opinion on that from the Trojita developers? Cheers, Kevin -- Kevin Krammer, KDE developer, xdg-utils developer KDE user support, developer mentoring signature.asc Description: This is a digitally signed message part.
[trojita] Re: GSoC review
On Wednesday, July 10, 2013 5:00:27 PM CEST, Kevin Krammer wrote: Creating a plugin for really old distribution versions or RHEL6 (or older) might be a worthwhile goal on its own, I am just not sure it should be the done as part of the GSoC scope. Do we have any opinion on that from the Trojita developers? Well, recently Trojita moved from qmake to CMake. When using Qt4, we require CMake 2.8.7 to be present. The CMake version in Debian oldstable is 2.8.2, so without modifications it is not possible to compile Trojita on Debian oldstable. I don't know what the story for some other distribution is, but I guess it is the same. So I think it does not make sense to create a plugin for really old distributions since it is probably not even possible to compile Trojita with the CMake versions available in these distributions. Best regards, Caspar Schutijser
Re: [trojita] Re: GSoC review
On Wednesday 10 July 2013 17:17:37 Caspar Schutijser wrote: > On Wednesday, July 10, 2013 5:00:27 PM CEST, Kevin Krammer wrote: > > Creating a plugin for really old distribution versions or > > RHEL6 (or older) might be a worthwhile goal on its own, I > > am just not sure it should be the done as part of the GSoC > > scope. > > > > Do we have any opinion on that from the Trojita developers? > > Well, recently Trojita moved from qmake to CMake. When using > Qt4, we require CMake 2.8.7 to be present. The CMake version > in Debian oldstable is 2.8.2, so without modifications it is > not possible to compile Trojita on Debian oldstable. I don't > know what the story for some other distribution is, but I > guess it is the same. So I think it does not make sense to > create a plugin for really old distributions since it is > probably not even possible to compile Trojita with the CMake > versions available in these distributions. > > Best regards, > Caspar Schutijser I already wrote that if you want I will create akonadi plugin too. So there is no problem that distributions will do not have plugin using new kde api. Reason why I created plugin which using kabc is because akonadi is too slow on my notebook and using too much CPU & memory. I really do not want to slow down Trojita client on my notebook. So kabc plugin is not (only) for old distributions... Working email client is for me really needed and at begin of this year I was forced to do something with kdepim for my usage. I really do not like akonadi which caused lot of problems to me. You can read more at my blog post: http://atrey.karlin.mff.cuni.cz/~pali/blog/2013-04-18-kdepim-without-akonadi.html I do not care that kabc is marked as deprecated. On my notebook it is faster than akonadi. If you do not want to see kabc plugin in trojita master git repo I will of course respect it and will prepare akonadi plugin. But you cannot force me to use akonadi plugin instead this kabc, if akonadi will be still slower. -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
[trojita] Re: GSoC review
On Wednesday, July 10, 2013 8:26:52 PM CEST, Pali Rohár wrote: On Wednesday 10 July 2013 17:17:37 Caspar Schutijser wrote: On Wednesday, July 10, 2013 5:00:27 PM CEST, Kevin Krammer wrote: Creating a plugin for really old distribution versions or RHEL6 (or older) might be a worthwhile goal on its own, I am just not sure it should be the done as part of the GSoC scope. Do we have any opinion on that from the Trojita developers? I already wrote that if you want I will create akonadi plugin too. So there is no problem that distributions will do not have plugin using new kde api. That's true. I replied to that question without thinking about your earlier emails. Sorry. I think it's nice to have a Akonadi plugin, regarding the deprecatedness of KABC That said: I do not care that kabc is marked as deprecated. On my notebook it is faster than akonadi. If you do not want to see kabc plugin in trojita master git repo I will of course respect it and will prepare akonadi plugin. But you cannot force me to use akonadi plugin instead this kabc, if akonadi will be still slower. I have nothing against a KABC plugin in the Trojita master git repo. The nice thing about plugins is that one can choose which one you use. Since you want to use KABC and you even are going to write the plugin, I don't see a problem with that. So my opinion summarized: if you would like to create two plugins (KABC and Akonadi), that would be nice. I don't know if someone has another opinion on this? Best regards, Caspar Schutijser
[trojita] Re: GSoC merge request
On Wednesday, July 10, 2013 1:16:39 PM CEST, Pali Rohár wrote: I fixed this problem. Also that one which caused that tests was not possible to compile. Patches pushed to my same git branch. Thanks :) I read through the diff (that is, everything that has changed between your version and the "master" git version) and I found some variables/functions which have a name with the string "prefered" in them. Shouldn't that be "preferred"? Best regards, Caspar Schutijser
[trojita] Re: GSoC review
On Mittwoch, 10. Juli 2013 20:52:20 CEST, Caspar Schutijser wrote: I don't know if someone has another opinion on this? The problem with the KABC plugin is that it seems to be unusable with a few months (but on older distros) - i don't have to sell that to google but it sounds like a pointless exercise nevertheless. (resp. means rotten code to get rid off in the near future) @Pali If you don't like akonadi, focus on whatever other addressbook, but unless you gain sufficient control over kaddressbook, it seems you can very likely kiss it bye-bye short term. @Kevin, I'll now make a public statement: If you've any influence, get rid of the name "akonadi". It's so much burned that a huge amount of ppl. will never use it, no matter how good it can get, just because of its name ;-) Name it PIM (plasma information middleman) or PIB(roker) or PIA(agent/s) but as long as google suggests "disable" or "abschalten" or "server abschalten" or "sucks" when you type akonadi, it's a dead horse. Now I said it ;-P Cheers, Thomas
[trojita] Re: GSoC merge request
Hi Pali, One other thing, related to the GUI: the minimumSize of the added QComboBoxes in the settings dialog cause a horizontal scrollbar to appear in the settings dialog, at least on my computer. How about lowering the width of the minimumSize for a bit? While we are at it, since the addition of the two comboboxes there also is a vertical scrollbar. So maybe we should increase the minimum-height of the whole window. Screenshot, if desired: http://temp.schutijser.com/~caspar/trojita-scrollbars.png Thanks. Best regards, Caspar Schutijser.
Re: [trojita] Re: GSoC merge request
On Wednesday 10 July 2013 21:35:09 Caspar Schutijser wrote: > Hi Pali, > > One other thing, related to the GUI: the minimumSize of the > added QComboBoxes in the settings dialog cause a horizontal > scrollbar to appear in the settings dialog, at least on my > computer. How about lowering the width of the minimumSize for > a bit? > > While we are at it, since the addition of the two comboboxes > there also is a vertical scrollbar. So maybe we should > increase the minimum-height of the whole window. > > Screenshot, if desired: > http://temp.schutijser.com/~caspar/trojita-scrollbars.png > > Thanks. > > Best regards, > Caspar Schutijser. Hi, in settings dialog I added two menus for selecting password and addressbook plugin. And to prevent hiding plugin description in menubar I set minimal size. But there is problem that main window of setting dialog and also first (general) tab does not want to autoresize, so window will be without scrollbars. I played a lot with minimal size and on my machine there are no scrollbars. But now I think that minimal size depends on used qt theme and so we cannot predicate correct minmal size. So is there any way to tell general settings tab widget to set minimal size to value when all horizontal & vertical scrollbars will not be shown? -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: [trojita] Re: GSoC merge request
On Wednesday 10 July 2013 20:59:21 Caspar Schutijser wrote: > On Wednesday, July 10, 2013 1:16:39 PM CEST, Pali Rohár wrote: > > I fixed this problem. Also that one which caused that tests > > was not possible to compile. Patches pushed to my same git > > branch. > > Thanks :) > > > I read through the diff (that is, everything that has changed > between your version and the "master" git version) and I > found some variables/functions which have a name with the > string "prefered" in them. Shouldn't that be "preferred"? > > Best regards, > Caspar Schutijser Fixed. -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
[trojita] Re: GSoC merge request
On Mittwoch, 10. Juli 2013 21:41:30 CEST, Pali Rohár wrote: I played a lot with minimal size and on my machine there are no scrollbars. But now I think that minimal size depends on used qt theme and so we cannot predicate correct minmal size. So is there any way to tell general settings tab widget to set minimal size to value when all horizontal & vertical scrollbars will not be shown? Scrollareas are not affected by the constraints of their viewport. That's actually the point about scrollareas and the reason why there are scrollbars. You'll have to filter the viewport events, intercept resizes and resize the scrollarea to the minimum size of the viewport + the scrollarea paddings (usually contentsMargins will do - unless there's a stylesheet in place wich breaks this API) Since any naive process in that regard would render scrollarea invocation pointless in the first place, you'll usually also invoke a maximum size (QSize::boundedTo(QSize())) - usually either a fixed value (640x480) or (a fraction of) the QApplication::desktop()->availableGeometry() Please do not operate on fixed values you somehow™ estimated - it's an instant fail as the final dimensions invoke the style, the environment and eventually even display resolution. Cheers, Thomas PS: if you never used eventFilter(QObject*, bool) before and feel confused: you want to return false and do *not* touch event acceptance =)
