KIO split (from the Job side)

[Kevin Ottens]

Le Lundi 17 Avril 2006 19:07, David Faure a écrit :
> I think that subjob handling belongs in KJob.

Ok, seeing that and Olivier mail, I'll add this to KJob.

> I'm not sure what's your plan in that area?
>  KJobUi::showErrorDialog(KJob*) ? looks strange :)

It was my first idea yes... It indeed looks strange. I'm sure that I need to 
collect more needs by looking at KIO::Job and friends, that might give me a 
better idea.
(and yes, written this way it looks a bit 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).

Ok, I should definitely investigate this one a bit more.

> 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.

I'll also take a closer look to it. I tend to be more optimistic than you 
though. ;-)

> OK (without committing yet, of course, or into the bleedingedge branch)

I'm not sure I understood what you meant. What's the bleedingedge branch? And 
in case I wasn't clear I meant committing my changes to kdelibs trunk.

> > 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.

Hm, ok. Just for the record this one comes from KIO::Job.

> >     /**
> >      * 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...

Sure. I noticed it, but I kept the bool but it's temporary I didn't want to 
break to many things at once. And since the signal signature changes implies 
already quite some work...

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: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20060418/d8b5da3e/attachment.sig>