[Panel-devel] [PATCH] Package and PackageStructure cleanup

[Aaron J. Seigo]

On Monday 26 November 2007, Paolo Capriotti wrote:
> Aaron J. Seigo wrote:
> > On Sunday 25 November 2007, Paolo Capriotti wrote:
> >
> > my original idea was to exract the .desktop file on installation and put
> > it in the services directory (after giving the file a proper name, of
> > course, to prevent collisions with other files) so that ksycoca would
> > pick it up. eliminates the whole problem, really.
>
> Yes, that's the best option (it allows you to perform complex queries),
> but there is a subtle problem: once you get a service (or a service
> list), how do you discover the path to the associated package? If you

discovering the package exists and where the package actually are actually two 
separate steps with this approach.

> search it with KStandardDirs, you're back where you started... well, not
> really, but you have a list of names, when you actually want a list of
> paths. And I don't think that hardcoding the path in the .desktop file
> is sensible, is it?

that's why the application using Package provides the root path. the reason i 
didn't just default this to the appdata dir is that this will break with 
plugins that have their own KComponentData object. we could take a 
KComponentData instead of a root path, but then that would make it harder to 
access other app's/plugin's packages (since you'd need a KComponentData that 
represents that app/plugin).

i suppose another way of going about it is installing *all* packages into the 
plasmagik appdata directory and instead of taking a root path, taking a name 
(e.g. "plasma") which then gets passed like 
KStandardDirs::locate("appdata", "plasmagic/" + name);

> 1) The layout of files in a package is not consistently defined in the
> code: for example Package::createPackage puts all files in a 'contents'
> directory, which is not taken into account by all the functions
> returning a path.

yes, this should be fixed in package.cpp. putting things in a contents/ folder 
is the right thing to do (prevents packages from getting messed up just 
because they have a file named "metadata.desktop" or whatever =), so 
Package::filePath ::entryList, etc need to be fixed. PackageStructure 
shouldn't be touched though.

> 2) There's no API to access the icon, screenshot, preview and metadata
> of a package. You can access them, of course, but you have to do it
> manually with path manipulations.

perhaps these should be added to every PackageStructure object, e.g. have a 
set of addFile calls in the PackageStructure ctor. then they can be removed 
from the package metadata as well, with the exception of the Icon field which 
is required (and can be filled in using the PackageStructure)

> 3) Icon, screenshot and preview have fixed hardcoded paths (as in
> createPackage), but PackageMetadata has fields to store them.

yes, createPackage should be reading the values out of PackageMetadata or 
PackageStructure (probably better) instead of hard coding them. sane defaults  
should be provided and createPackage should follow whatever those are.

the big problem is that right now PackageMetadata is used to store the path to 
the icon on creation. this really is something that belongs in 
PackageStructure.

note that package metadata also has both screenshot() and preview().. it only 
needs one of those =)

> So, I think we have to decide a file layout and use it consistently (and
> document it somewhere), then add the missing API and tests.
> Our proposal is to stick with what createPackage currently assumes
> (metadata, icon, preview and screenshot on the root directory, all the
> other files in a directory named 'contents') and adapt the rest.
> If there are no objections, I'm going to do the necessary changes this
> week.

i wonder if the icon and preview shouldn't also go into contents/ to make it 
easier from a package creator's POV: you simply put all your files into that 
one directory; the design studio would then have an "Icon" entry listed along 
with things like (for plasmoids) "Main Script". should simplify things pretty 
nicely.

>  > the patch looks good, btw. pls apply
>
> Done. I've also merged your patch (const char* -> QByteArray) and added
> a test that was previously failing with const char* keys.

great...

-- 
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/20071126/63e50f47/attachment.pgp