[PATCH] Spring loading folder behaviour (redux)

[Kévin 'ervin' Ottens]

Le lundi 24 Mai 2004 14:06, David Faure a écrit :
> But that was exactly the problem. Static objects lead to lots of problems
> due to undefined order of construction/destruction. With pointers you can
> control when things are being created and destroyed.

Ok, it sounds logical...

> I don't know where we should say it. In the developer FAQ? or rather in
> mistakes.html? History of the KDE project is full of problems due to those
> - like a recent crash report on kde-pim...

The mistakes.html is a very good place for this IMHO. It is so good that it's 
already in! :-)
Sorry I missed it... I don't know why...

> This patch looks quite good.

Thanks. ;-)

> One thing I'm wondering, is whether we could save a bit of memory by
> getting rid of the SpringLoadingManager instance at the end of a dnd
> operation.

Not a bad idea at all...

> Would it be enough to do "deleteLater(); ms_self=0;" in finished()?

Implemented.

> The deleteLater avoids deleting the object from a slot.
> In fact if we do this we don't really need the staticdeleter anymore, since
> one has to stop a dnd before exiting Konqueror anyway :)
>
> But what happens on a timeout? The dragFinished slot will be called
> after the timeout cleaned up everything, so finished() is effectively
> called twice? Then my idea of "destroying in finished()" would lead
> to slotDragFinished() creating the manager again, just to destroy it
> again... Not too wise, I guess it could check
> SpringLoadingManager::exists() then, which would be a static bool exists()
> { return ms_self != 0; }

Implemented too.

I've kept the KStaticDeleter because of the exists method. At least if the 
code using the SpringLoadingManager is modified we can guaranty that it's 
always properly deleted even if the user code forget to test exists().

In the worst scenario we allocate a SpringLoadingManager for nothing...

> (BTW static variables are usually called s_* in KDE, no 'm')

Maybe it's stated somewhere and I missed it (again)?

> > +    emit view->extension()->setLocationBarURL( url.url() );
>
> Should be url.prettyURL()  (to hide passwords etc.)

Ok I modify this.

You'll find the last version attached.
Please test it, if there's no showstopper I'll commit it as is. If it's the 
case, I'm officially in time for KDE 3.3! ;-)

Regards.
-- 
Kévin 'ervin' Ottens, http://ervin.ipsquad.net
"Ni le maître sans disciple, Ni le disciple sans maître,
Ne font reculer l'ignorance."
-------------- next part --------------
A non-text attachment was scrubbed...
Name: konq_iconview.patch3
Type: text/x-diff
Size: 8489 bytes
Desc: not available
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20040524/079df8ac/attachment.diff>