Review Request: kdelibs: "default"-values in config files should be able to handle code-generated default values

[Martin Blumenstingl]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5525/
-----------------------------------------------------------

Review request for Plasma.


Summary
-------

For some projects it's necessary to have code-generated default values for some of the settings.
Just an example:
say a plasmoid prints text somewhere. The font (including the color) is configurable.
Now there's one problem: the developer might run into trouble when setting a hardcoded default color in the config file.

Here's my solution:
KConfigXT already support code-generated default values, see: http://techbase.kde.org/Development/Tutorials/Using_KConfig_XT (take a look at the "Font" example).
With my patch it's possible to add code to the default tags.
The ConfigLoader will parse it (with the current script's script engine) if the "code" attribute is set to "true".

This patch provides the kdelibs part.
The patch for the JavaScript engine (and probably other engines) will be part of a separate review:
http://svn.reviewboard.kde.org/r/5526/


Please note that my patch is not finished yet.
I still need to write unit tests (but before that I need to think about how to write them..).

There is one issue which I'd like to fix soon:
The ConfigLoader does not handle QVariants yet.
This means: if the user specifies "type=color" then the code-generated value must return a QString (which is then passed to the ctor of QColor()).
But a developer would expect the following: if the code-generated value returns a QString then that QString will be passed to the ctor of QColor(). If the code-generated value returns a QColor() then that one is directly used.

PS: The reason why I opened a review request for an unfinished patch is quite simple:
Communication is the key to success.
Thus I'd like to ask you about comments before I finish it (otherwise someone might tell me after I'm done with it: "the architecture of your patch sucks. Your patch can go to hell. kthxbye" ;)).


Diffs
-----

  /trunk/KDE/kdelibs/plasma/applet.cpp 1182146 
  /trunk/KDE/kdelibs/plasma/configloader.h 1182146 
  /trunk/KDE/kdelibs/plasma/configloader.cpp 1182146 
  /trunk/KDE/kdelibs/plasma/private/configloader_p.h 1182146 
  /trunk/KDE/kdelibs/plasma/scripting/scriptengine.h 1182146 
  /trunk/KDE/kdelibs/plasma/scripting/scriptengine.cpp 1182146 

Diff: http://svn.reviewboard.kde.org/r/5525/diff


Testing
-------

The ConfigLoader unit-test did not fail :)
I also wrote a test-plasmoid: http://filebin.ca/abcwza/codedefault.tar.bz2


Thanks,

Martin

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20101003/e36911af/attachment.htm