<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, 8:31 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;">two problems i didn't see before, this add a virtual and complex types that require a constructor won't work, since are not registered yet</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;">a solution could be assuming that code would be javascript, and use a new engine just for that with all the needed extra types registered. it would be a fair amount of duplicate code tough (and a bit overkill perhaps)</pre>
<br />
<p>- Marco</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>