[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-pim
Subject:    Re: [Kde-pim] KDE 3.5.final?
From:       Ingo =?iso-8859-1?q?Kl=F6cker?= <kloecker () kde ! org>
Date:       2008-09-05 18:53:16
Message-ID: 200809052053.17207 () thufir ! ingo-kloecker ! de
[Download RAW message or body]

[Attachment #2 (multipart/signed)]

[Attachment #4 (multipart/mixed)]


On Friday 05 September 2008, Jonathan Marten wrote:
> Ingo Klöcker <kloecker@kde.org> writes:
> > Frankly, the following piece of code looks pretty strange to me.
> >
> > +    KMFolder *folder = account->folder();               // init
> > folder's account list
> > +    if ( folder && !folder->hasAccounts() )
> > +      account->setFolder( folder, true );
> >
> > Why set the folder of an account to the folder it is already set
> > to? The code looks very fragile because it seems to make
> > assumptions about the internals of other classes. I'll need more
> > time to investigate whether this really is the best way to fix the
> > problem.
>
> That was my code.  It works because KMAccount::setFolder has a side
> effect, it calls KMAcctFolder::addAccount which initialises the
> folder's account list (KMFolder::mAcctList). This list is then
> checked in FolderTreeBase::hideLocalInbox (via KMFolder::hasAccounts)
> to decide whether the inbox is to be shown.
>
> Unless this is done, the folder's account list is never initialised
> with the newly created account.

So it would be much cleaner to call KMAcctFolder::addAccount() instead 
of relying on this side effect of KMAccount::setFolder(). See attached 
patch which should be equivalent to your patch without relying on 
KMAccount::setFolder()'s side effect.


Regards,
Ingo

["accountmanager.diff" (text/x-diff)]

Index: accountmanager.cpp
===================================================================
--- accountmanager.cpp	(revision 831099)
+++ accountmanager.cpp	(working copy)
@@ -7,6 +7,7 @@
 #include "accountmanager.h"
 
 #include "kmaccount.h"
+#include "kmacctfolder.h"
 #include "kmacctmaildir.h"
 #include "kmacctlocal.h"
 #include "popaccount.h"
@@ -239,6 +240,11 @@
 {
   if ( account ) {
     mAcctList.append( account );
+    // init folder's account list
+    KMAcctFolder *folder = static_cast<KMAcctFolder*>( account->folder() );
+    if ( folder && !folder->hasAccounts() ) {
+      folder->addAccount( account );
+    }
     emit accountAdded( account );
     account->installTimer();
   }

["signature.asc" (application/pgp-signature)]

_______________________________________________
KDE PIM mailing list kde-pim@kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/

[prev in list] [next in list] [prev in thread] [next in thread]