KIO split (from the Job side)

[David Faure]

On Mon, Apr 17, 2006 at 06:47:48PM +0200, Kevin Ottens wrote:
> Hello list,
> 
> I'd like to propose some refactoring in the KIO::Job area. It's in my opinion 
> particularly needed for the following reasons:
> 1) KIO::Jobs are tied to the UI and this depend could be removed
> 2) Other areas than KIO are starting to use Jobs (namely Akonadi and Solid), 
> so it's a good target for factoring

Agreed.

> My current plan is the following:
> 
> 1) Factor out a KJob class that will go in kdecore (find in attachment).
> I already have a first version of this class. It provides the necessary 
> interface to implement a job and track its progress, but doesn't enforce any 
> scheduling policy.
> The major drawback is that some signals signature changed which means quite 
> some changes in the user code. I already have modified the whole kdelibs to 
> use the new signals (I didn't attach the patch since it's big around 160KB). 
> kdebase requires a similar search and replace work.
> Of course there's several advantages, KJob is UI free and we'll have a common 
> job interface accross whole KDE.
> 
> Note that currently KJob doesn't support subjobs, I kept this in KIO::Job but 
> it could be moved if necessary.

I think that subjob handling belongs in KJob.

> 2) Factor out the UI parts of KIO::Job.
> That's the natural second step for such a refactoring... KJob being UI free we 
> need to reuse the UI features of KIO::Job.
> The showErrorDialog(), and window()/setWindow() methods would be removed from 
> KIO::Job and go in a kdeui counterpart of KJob.
I'm not sure what's your plan in that area?
 KJobUi::showErrorDialog(KJob*) ? looks strange :)

setWindow is difficult to move - notice that the window id, a ulong on X11,
is being passed to the slave as metadata. From there, it's passed to uiserver
via dcop calls - all this to give a correct parent window to dialogs from uiserver.
No idea how to do that on Windows (where window ids are not ulongs but some kind of pointer).

> The KIO::Observer class would move to kdeui after removing the KIO specific 
> bits and putting them in convenience classes.
Easy to say... in practice, observer (which needs to be renamed anyhow)
is the stub for the job to call uiserver to display progress information.
It's used internally by the jobs, I think it needs to remain with KIO::Job.
It's not really a GUI class anyway (except for some deprecated methods),
only a stub for talking to uiserver.

> In the end it means that we'll be able to use the uiserver for any job inside 
> of KDE.
Yes (that's already the case in KDE3 actually, but it's good to ensure that and
document it)

> If no one objects I'll commit KJob and the necessary kdelibs modification on 
> wednesday. In the mean time I'll start working on porting kdebase to the new 
> KJob signal signatures.
OK (without committing yet, of course, or into the bleedingedge branch)

> setObjectName( "job" );
Minor issue: I don't see the point in giving the same name to all KJobs.
If the name doesn't give more info than the classname already gives, it's
not really useful. Better let the subclasses, or the apps, name the job object.
Hmm, ok the above doesn't prevent that, but it just seems like an unnecessary QString.

>     /**
>      * Aborts this job.
>      * This kills and deletes the job.
>      *
>      * @param quietly if false, Job will emit signal result
>      * and ask uiserver to close the progress window.
>      * @p quietly is set to true for subjobs. Whether applications
>      * should call with true or false depends on whether they rely
>      * on result being emitted or not.
>      */
>     virtual void kill( bool quietly = true ) = 0;
Qt4/KDE4 tries to avoid booleans in APIs when they hurt readability,
this is such a case. This should be either an enum, or a virtual method kill()
and a non-virtual method killAndEmitResult which calls kill and emits result.
A bit of a long name though, but I don't have a better naming idea right now...


--
David Faure
faure at kde.org