[PATCH] RunnerManager

[Aaron J. Seigo]

On Tuesday 29 April 2008, Jordi Polo wrote:
> > ignoredType is wrong. it should be:
> >
> > SearchContext::Types ignoredTypes() const
> >
> > using the flags (e.g. ignoredTypes & type) would done on the consumer's
> > end
> > (e.g. in RunnerManager)
>
> Done
>
> > the Private copy ctor shouldn't copy the matches (it used only on detach,
> > right? so we don't want the matches). this makes the call to
> > removeAllMatches
> > unecessary in reset(). that's the whole point of this approach: avoid
> > copying, deleting, etc.
>
> Ok, the ctor should initialize all to empty objects as it is only called in
> reset. I'll change that. See my next commentary.
>
> > +    if (this !=d->q) {
> > +    //FIXME: this line must be uncommented.
> > +        d.detach();
> > +    }
> >
> > that looks completely backwards to me. it also looks completely
> > unecessary.
> > whenever reset is called it should detach. in practice, reset will only
> > be called on the real context object, and that one should detach...
> > again, this
> > all about preventing copies and deletions.
>
> Detaching always is ok. 

not sure exactly what you mean by "always"; what i'm suggesting is to always 
detach when reset is called because reset is only called from the query 
source (e.g. the krunner UI). never detach otherwise. voila.

> Detaching will create a new empty object, so we 
> don't need the rest of the method when we have indeed detached (we still
> need to clean our state if we don't detach).

the semantics of "detach()" are actually to create a copy of the underlying 
object. detach is COW, where the 'c' stands for copy. it should not create an 
empty object. in our case, we only want to create the following items new:

 * the mutex for locking (can't be copied)
 * the match list (since in this case, writing applies to the state of the 
SearchContext, making the matches obsolete)

everything else should be copied in detach().

> But detach() will only effectively detach if other object references the
> data,  as detach() returns void, we can not know if in fact detached.  I am

that's because detach always detaches. there is no "couldn't detach".

> > the rest looks ok ... work needed in places still, but close enough ...
> > i'm
> > tempted to say "commit it, and then i'll polish it up"
>
> Puff, at last we got close, after the detach issue is solved I'll comment
> kDebug() and commit.

cool.

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Trolltech
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 194 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/panel-devel/attachments/20080430/d155ccfa/attachment.pgp