[PATCH] RunnerManager

Jordi Polo mumismo at gmail.com
Wed Apr 30 06:14:37 CEST 2008


> 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. 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).
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
tempted to use a bool to indicate if we are in a clean state, but it looks
terrible ugly.



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




-- 
Jordi Polo Carres
NLP laboratory - NAIST
http://www.bahasara.org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/panel-devel/attachments/20080430/52f48cd5/attachment.html 


More information about the Panel-devel mailing list