KIO::Job, JobUiDelegate strangeness (was: Preparing terrain for kio_uiserver)

[Rafael Fernández López]

Hi,

First of all, thanks for checking it out. Your commit seems to fix the
troubles, and it works as well as mine :P, but yours is probably more
correct ;).

I just did this (now is not necessary):

>  void KJob::setUiDelegate( KJobUiDelegate *delegate )
>  {
> -    if ( delegate == 0 || delegate->setJob( this ) )
> +    if ( d->uiDelegate )
>      {
>          delete d->uiDelegate;
> +    }
> +
>          d->uiDelegate = delegate;
> +
> +    if ( d->uiDelegate )
> +    {
> +        d->uiDelegate->setJob( this );
>      }
>  }

because before were called setJob before the d->uiDelegate was set, so as
you know, that lead to problems when trying to read from job->ui(), since it
wasn't set yet. Now is fixed with your patch, of course ;)

The option of returning boolean for setUiDelegate makes more sense to me.
Now or later, silent decisions are the worst, since they are hard to debug
and to remember (and as you know, my head is not the best for remembering
things :P).

About this:

> This change is wrong don't commit it. OTOH It would be right to
> change "d->job" by "job" on line 92. I also commited this a minute ago
with
> the other fix.

Now is not the same, but before (on the code before you committed), you
could see just before the call to connectJob the next statement:

d->job = job; (or something similar, can't remember really),

so it was the same doing the connect to job, or d->job, since they were
pointing to the same place, but I did that change just to make it more
pretty (the two connections with d->job or with job). But now you fixed it
and is no needed at all. And now is not the same, and as you said my patch
is wrong right now.

Working perfectly, thanks ;)

Bye,
Rafael Fernández López.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20061223/ce32774d/attachment.htm>