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

Marco Martin notmart at gmail.com
Sun Oct 3 21:01:05 CEST 2010


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


I like the approach, as you said, supporting the extra types is needed,
so a similar binding like the js engine itself for the color ctor is needed as well here.
are stringlist supported atm?

- Marco


On 2010-10-03 18:10:47, Martin Blumenstingl wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5525/
> -----------------------------------------------------------
> 
> (Updated 2010-10-03 18:10:47)
> 
> 
> 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/412e0311/attachment.htm 


More information about the Plasma-devel mailing list