<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="https://git.reviewboard.kde.org/r/116845/">https://git.reviewboard.kde.org/r/116845/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The unit test situation seems far from ideal, but since it fits with the current setup it can be left for now. Changing them can be left for another patch :)
Otherwise, I imagined readConfig would become a regular method, and read is the new virtual method to implement to override the KConfig parsing. I've opened some issues about that below.
Also, I'd like to get the usrReadConfig function renamed in this patch, as we were discussing in RR #116461, unless we decide to leave the name alone.</pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/116845/diff/1/?file=254551#file254551line1039" style="color: black; font-weight: bold; text-decoration: underline;">src/core/kcoreconfigskeleton.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1039</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * the registered items you can override readConfig() in a derived class.</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1039</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * the registered items you can override readConfig() in a derived class.</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Regarding the virtual methods, once they change this needs to be updated.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/116845/diff/1/?file=254551#file254551line1041" style="color: black; font-weight: bold; text-decoration: underline;">src/core/kcoreconfigskeleton.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1041</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">virtual</span> <span class="kt">void</span> <span class="nf">readConfig</span><span class="p">();</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1041</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">virtual</span> <span class="kt">void</span> <span class="nf">readConfig</span><span class="p">();</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Can you remove the virtual from readConfig, since there isn't a reason to override readConfig, and may in fact break if other code uses read() instead.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/116845/diff/1/?file=254551#file254551line1052" style="color: black; font-weight: bold; text-decoration: underline;">src/core/kcoreconfigskeleton.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1052</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * the registered items you can override readConfig() in a derived class.</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Regarding the virtual methods, once they change this needs to be updated.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/116845/diff/1/?file=254551#file254551line1055" style="color: black; font-weight: bold; text-decoration: underline;">src/core/kcoreconfigskeleton.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1055</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">void</span> <span class="nf">read</span><span class="p">();</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I can't find any instances (lxr.kde.org returns way too many results to thoroughly check) where readConfig() was overridden and I don't know why you would want to override it, but since some users may have anyways I'd prefer if read() stayed a virtual for compatibility, since it replaces readConfig.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/116845/diff/1/?file=254552#file254552line989" style="color: black; font-weight: bold; text-decoration: underline;">src/core/kcoreconfigskeleton.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">QVariant KCoreConfigSkeleton::ItemIntList::property() const</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">989</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">d</span><span class="o">-></span><span class="n">mConfig</span> <span class="o">=</span> <span class="n">KSharedConfig</span><span class="o">::</span><span class="n">openConfig</span><span class="p">(</span><span class="n">configname</span><span class="p">,</span> <span class="n">KConfig</span><span class="o">::</span><span class="n">FullConfig</span><span class="hl"> </span><span class="o"><span class="hl">|</span></span><span class="hl"> </span><span class="n"><span class="hl">KConfig</span></span><span class="o"><span class="hl">::</span></span><span class="n"><span class="hl">DelayedParsing</span></span><span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">989</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">d</span><span class="o">-></span><span class="n">mConfig</span> <span class="o">=</span> <span class="n">KSharedConfig</span><span class="o">::</span><span class="n">openConfig</span><span class="p">(</span><span class="n">configname</span><span class="p">,</span> <span class="n">KConfig</span><span class="o">::</span><span class="n">FullConfig</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'd prefer to keep the same behaviour we implemented before. This allows for applications to make use of the optimization if they haven't been ported yet.
It also reduces developer thought in the simple case, as they can just always call readConfig/loadConfig, and be ensured they get the latest values from disk, while avoiding an extra read off disk. That leaves the read() function there as an optimization for the more advanced cases.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/116845/diff/1/?file=254552#file254552line1066" style="color: black; font-weight: bold; text-decoration: underline;">src/core/kcoreconfigskeleton.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void KCoreConfigSkeleton::readConfig()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1066</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">// qDebug();</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1066</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">// qDebug();</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Since we are modifying this function, can you also kill this line please?</pre>
</div>
<br />
<p>- Matthew Dawson</p>
<br />
<p>On March 16th, 2014, 7:08 p.m. EDT, David Faure wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Frameworks and Matthew Dawson.</div>
<div>By David Faure.</div>
<p style="color: grey;"><i>Updated March 16, 2014, 7:08 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kconfig
</div>
<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;">Add KCoreConfigSkeleton::read() which doesn't call reparseConfiguration.
Call it from generated singletons, since the constructor creates
a KConfig from a filename, which already loads from disk.
This removes the need for using DelayedParsing.</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;">Added two unittests (which is how I discovered I had to remove DelayedParsing, so the tests were useful).
The "bool ok + qWarning" thing is a bit clumsy, maybe we want to port these main() to qtestlib too, even though they are themselves executed by a unittest :-)</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>autotests/kconfig_compiler/test4main.cpp <span style="color: grey">(8f1c1ec8d41ea123893a59526c52cdbd0b5ee075)</span></li>
<li>autotests/kconfig_compiler/test5.cpp.ref <span style="color: grey">(088cc78f4dc96a719628ece342e149553c1d22aa)</span></li>
<li>autotests/kconfig_compiler/test8b.cpp.ref <span style="color: grey">(dcd61693ff86f02eaeb93b4c4da9e6ed20463469)</span></li>
<li>autotests/kconfig_compiler/test_dpointer.cpp.ref <span style="color: grey">(e50bf8aa29a2d009c4156dabf429c3ffb74eb7ba)</span></li>
<li>autotests/kconfig_compiler/test_signal.cpp.ref <span style="color: grey">(35b5cba2736761753d1ba32baa6f9fc2e7808ba1)</span></li>
<li>autotests/kconfigskeletontest.cpp <span style="color: grey">(31278e767655f7e3d25a4ed9984345e8c4e67d82)</span></li>
<li>src/core/kcoreconfigskeleton.h <span style="color: grey">(a2b828a4ef3f881b551049901d4bc26bb23a014b)</span></li>
<li>src/core/kcoreconfigskeleton.cpp <span style="color: grey">(0c1a96faaa9c511d349a26ad8af4b7b2c9bf313e)</span></li>
<li>src/kconfig_compiler/kconfig_compiler.cpp <span style="color: grey">(7d84cfbc28b1648ca999116726455183d88a7f0a)</span></li>
<li>autotests/kconfig_compiler/test4.cpp.ref <span style="color: grey">(66d0357f114b0aca6148a2091880dd2f62fbf624)</span></li>
<li>autotests/kconfig_compiler/test1main.cpp <span style="color: grey">(d7dc038d91d2f8babcd281960100d1c6fa264d7c)</span></li>
<li>autotests/kconfig_compiler/test10.cpp.ref <span style="color: grey">(21aea9d87af5fce01e64032257283e0883af2d81)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/116845/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>