From kde-panel-devel  Wed Apr 30 19:26:31 2008
From: "Aaron J. Seigo" <aseigo () kde ! org>
Date: Wed, 30 Apr 2008 19:26:31 +0000
To: kde-panel-devel
Subject: Re: [PATCH] RunnerManager
Message-Id: <200804301326.31574.aseigo () kde ! org>
X-MARC-Message: https://marc.info/?l=kde-panel-devel&m=120958308114832
MIME-Version: 1
Content-Type: multipart/mixed; boundary="--===============1766484127=="

--===============1766484127==
Content-Type: multipart/signed;
  boundary="nextPart1404336.8PJiMPMuyz";
  protocol="application/pgp-signature";
  micalg=pgp-sha1
Content-Transfer-Encoding: 7bit

--nextPart1404336.8PJiMPMuyz
Content-Type: text/plain;
  charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline

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 detac=
h,
> > 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 !=3Dd->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.=20

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

> Detaching will create a new empty object, so we=20
> 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=
=20
object. detach is COW, where the 'c' stands for copy. it should not create =
an=20
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=
=20
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.

=2D-=20
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

--nextPart1404336.8PJiMPMuyz
Content-Type: application/pgp-signature; name=signature.asc 
Content-Description: This is a digitally signed message part.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.4-svn0 (GNU/Linux)

iD8DBQBIGMfn1rcusafx20MRAnNfAJ9hQ3wdpMAF/NxEsi5D4dDsaythHwCglfPX
uPNr4c2b7ee1+YHMabLXQ5w=
=oT9V
-----END PGP SIGNATURE-----

--nextPart1404336.8PJiMPMuyz--

--===============1766484127==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
Panel-devel mailing list
Panel-devel@kde.org
https://mail.kde.org/mailman/listinfo/panel-devel

--===============1766484127==--