Review Request: kdelibs: "default"-values in config files should be able to handle code-generated default values
Martin Blumenstingl
darklight.xdarklight at googlemail.com
Sun Oct 3 21:46:21 CEST 2010
> On 2010-10-03 19:01:08, Marco Martin wrote:
> > 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?
StringLists are supported (in a very ugly way) by the ConfigLoader.
Currently a string list is a comma-separated list of strings. Escaping commas does not work right now.
(But that is a part of the code which I didn't touch).
- Martin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5525/#review7951
-----------------------------------------------------------
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/2c4b5df1/attachment-0001.htm
More information about the Plasma-devel
mailing list