<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://svn.reviewboard.kde.org/r/5525/">http://svn.reviewboard.kde.org/r/5525/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 3rd, 2010, 7:01 p.m., <b>Marco Martin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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?</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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).</pre>
<br />
<p>- Martin</p>
<br />
<p>On October 3rd, 2010, 6:10 p.m., Martin Blumenstingl wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Plasma.</div>
<div>By Martin Blumenstingl.</div>
<p style="color: grey;"><i>Updated 2010-10-03 18:10:47</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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" ;)).</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The ConfigLoader unit-test did not fail :)
I also wrote a test-plasmoid: http://filebin.ca/abcwza/codedefault.tar.bz2</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/KDE/kdelibs/plasma/applet.cpp <span style="color: grey">(1182146)</span></li>
<li>/trunk/KDE/kdelibs/plasma/configloader.h <span style="color: grey">(1182146)</span></li>
<li>/trunk/KDE/kdelibs/plasma/configloader.cpp <span style="color: grey">(1182146)</span></li>
<li>/trunk/KDE/kdelibs/plasma/private/configloader_p.h <span style="color: grey">(1182146)</span></li>
<li>/trunk/KDE/kdelibs/plasma/scripting/scriptengine.h <span style="color: grey">(1182146)</span></li>
<li>/trunk/KDE/kdelibs/plasma/scripting/scriptengine.cpp <span style="color: grey">(1182146)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/5525/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>