Windows start menu item support for kickoff

Fabian Aichele faichele at primusnetz.de
Sun Jun 7 10:36:00 CEST 2009


Hello!

> First of all, thanks for trying to contribute to KDE, this is greatly
> appreciated.
>
> Now about criticism and suggestions:
> - please do not modify the *_EXPORT unless you really must do that. As
> far as I have seen you have replaced class wide exports with function
> exports, which is really messy and should be avoided. Please try to take
> out these parts of the patch and if you experience any problems we can
> fix that in a much shorter way together, I am sure.
I'll keep that in mind.

> - You modify kickoff as far as I can see, but this means that you would
> have to apply your changes to every single menu application again (there
> are multiple ones!). A better place for your changes would be the
> generation of the menu entries in the sycoca database the "normal" Linux
>  uses. This happens in the following files when running kbuildsycoca4:
> http://lxr.kde.org/source/KDE/kdelibs/kded/kbuildsycoca.cpp#335
> http://lxr.kde.org/source/KDE/kdelibs/kded/vfolder_menu.cpp#1577
> So instead of getting this done in kickoff itself, you should rather
> take the functions you already made and move them over to vfolder_menu.
You're right, this is kickoff-specific; I knew about ksycoca, but not about 
vfolder_menu.cpp. I'll definitely take a look at this.

> - in applicationmodel.cpp you made quite a lot of changes that are
> windows specific and are not really interesting to non-Windows
> developers. To keep the code clean, bigger changes are put into a
> windows only file which would be called applicationmodel_win.cpp and
> which would only be compiled on windows. As I have said before, the
> correct patch should go into vfolder_menu though, so it would be
> vfolder_menu_win.cpp instead.
Valid point; it's of course less messy to factor out platform-specific code 
into separate compilation units and let the build system cope with these 
specifics (I believe Qt itself uses the same approach whenever differences 
between supportet platforms are handled).

> - last I have a small idea, which might be needed later: the generation
> of the menus is only needed if plasma is installed. So it would good to
> be able to switch the generation of the menus on and off.
This is also right; currently, I'm recursing through the start menu folders 
each time (!) the menu is activated, which of course leaves a lot of space 
for more a optimized approach.

> This is a really big feature you are working on, I still hope you can
> get it done cleanly, and I will help as much as I can.
> So good luck...
Thanks!

I'll reorganize the EXPORT macros and will try to modify the menu generation 
so it works with other launchers besides kickoff, then report back (or 
already earlier, in the likely case of further questions arising).

With best regards,
Fabian Aichele 



More information about the Kde-windows mailing list