[PATCH] Take care also of the KJob::finished signal in KPart - it also fixes Ark's BUG 187538

Alessandro Diaferia alediaferia at gmail.com
Mon Apr 6 21:29:45 BST 2009


2009/4/6 Alessandro Diaferia <alediaferia at gmail.com>

>
>
> 2009/4/6 Kevin Ottens <ervin at kde.org>
>
>> On Monday 6 April 2009 19:27:08 Alessandro Diaferia wrote:
>>
>> > For me the default behavior should be KJob::EmitResult and not
>> > KJob::Quietly.. but those are just my 2 cents :)
>>
>> That's your opinion now because you're looking at this particular problem.
>> The
>> thing to keep in mind is that most of the time kill() is called because of
>> user cancellation, and most of the code out there just doesn't treat this
>> special case of "error" (wouldn't make sense to duplicate the same if
>> again
>> and again right?). So what would happen if we made EmitResult by default
>> is
>> that in most cases of user cancellation, the user would then get a dialog
>> claiming that the job failed... which isn't true, and isn't desirable to
>> put
>> into the face of the user (he knows he killed the task, he clicked on the
>> button).
>>
>> That's why EmitResult is a possibility but not the default, and shall not
>> be
>> the default. Now the apidox of the signals should be more explicit about
>> that,
>> and refer to kill(). It should also perhaps be made more explicit that
>> finished() is not meant to be used outside of job trackers (although right
>> now
>> it's not gonna bite your head off if you do it).
>>
>> And again, looking at the patch it seems that the problem is simply
>> resetting
>> the m_job pointer to 0 when the job deletes itself. There's a proper tool
>> for
>> that and it is called QPointer. So why not simply using that?
>>
>> 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."
>>
>>
> Ok then, thanks for your clear explanation.. I'll go for QPointer and try
> :)
>
> --
> Alessandro Diaferia
> KDE Developer
>

Thanks to Kevin, it seems to work fine.. Ok to commit from you?

-- 
Alessandro Diaferia
KDE Developer
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20090406/33adda99/attachment.htm>
-------------- next part --------------
Index: part.cpp
===================================================================
--- part.cpp	(revision 949906)
+++ part.cpp	(working copy)
@@ -389,7 +389,7 @@ public:
     void _k_slotJobFinished( KJob * job );
     void _k_slotGotMimeType(KIO::Job *job, const QString &mime);
 
-    KIO::FileCopyJob * m_job;
+    QPointer<KIO::FileCopyJob> m_job;
     KIO::FileCopyJob * m_uploadJob;
     KUrl m_originalURL; // for saveAs
     QString m_originalFilePath; // for saveAs


More information about the kde-core-devel mailing list