[Panel-devel] [PATCH] Package refactoring

[Aaron J. Seigo]

On Sunday 02 December 2007, Paolo Capriotti wrote:
> Aaron J. Seigo wrote:
> > On Saturday 01 December 2007, Paolo Capriotti wrote:
> >> Hi,
> >> this is a refactoring of the Package classes, as previously discussed
> >> with Aaron. A list of changes:
> >> - Icon, preview and screenshots are gone from the metadata. The icon
> >> field can still be written in the desktop file by
> >> PackageMetadata::write. Packages don't have associated icon, previews
> >> and screenshots anymore,
> >
> > this means that we now have to open each package to get at these things.
> > this is going to be too slow for use in package browsers (such as the
> > applet engine). we don't need both "preview" and "screenshot" (not even
> > sure what the difference was supposed to be in the first place, may be
> > they both got added unintentionally), but we do need both an icon and a
> > screenshot available in the .desktop file.
>
> I don't know... opening a package (once we do the optimization of not
> loading the metadata on construction) is essentially a no-op. The actual
> problem is locating it, but I don't think this could be a bottleneck.

it's a bit more than a no-op.

you know, what would probably be the smartest way of going about this would be 
use a KPixmapCache and load the actual Package if we don't have the 
screenshot in the cache. that was there is a one time penalty incurred for 
Package construction for the screen shot.

alright, nevermind about special casing the screenshot then. i can put 
together a KPixmapCache for them later that will help avoid even the use of 
KStandardDirs. hoorah.

> >> Comments?
> >
> > is there a reason why the plasmoidpackgetest includes packages.cpp
> > instead of the header? it should probably just include the header and
> > link to libplasma so as to provide a more realistic test, no?
>
> It should, but PlasmoidPackage is not exported. I'll leave it as it is

ah, right, you actually need access to the package class itself. we will run 
into this issue in other places, i'm sure. we could rather easily add a 
factory method for plasma package classes. so... yes, leave it as is for now, 
i suppose, and when the first app comes along that needs to create a 
PlasmoidPackage on its own, i'll add a method and enum to the Pasma 
namespace.

> > i'm also a little concerned about how easy it will be to write creation
> > tools around this; as it stands they can use the PackageStructure to
> > create the on-disk layout and then call createPackage to have it bundled
> > up. it might be worthwhile, however, to allow absolute paths in
> > PackageStructure for the point of creating packages and have a
> > createPackage that works off of a PackageStructure object.
>
> I'm not sure that having a class serve two purposes is a good idea. The
> plasmagik thing can have a class which behaves like that using
> composition with PackageStructure.

yes, it can. it is, as noted, all about ease of use.

> Generally speaking, I'm also not convinced about having functionality in
>   the Package* classes that is specifically thought for the package
> creation process. Even createPackage is a bit out of place there, and
> should be moved to plasmagik, I guess.

and then we have parts of the implementation that need to work together in two 
separate bodies of code. no thanks.

> > for the purpose of use in creation tools, we may also want to add a
> > PackageStructure::save(KConfigBase*) and
> > PackageStructure::open(KConfigBase*) so that they can be used as part of
> > project files.
>
> Yes, that's something I definitely wanted to do.

see, this is one of those things can also be in a creation tool without any 
help from libplasma. it's just the right place to put these kinds of things 
=)

> > that can wait for plasmagik and/or other design tools to come about
> > though, at which point their needs can push the design a bit more here.
>
> Of course, but let's try not to expose too much of the package creation
> stuff to Package users: all applications are potential users of Package,
> but only one (plasmagik) needs to use createPackage and friends.

that's an incorrect assumption. any app could have a package creator, as the 
contents of them are application specific.

-- 
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: 189 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/panel-devel/attachments/20071202/fc2a4010/attachment.pgp