<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://git.reviewboard.kde.org/r/112590/">http://git.reviewboard.kde.org/r/112590/</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;">Ah, turns out that I'm the one that misread the code. I mixed up CCSettings (the KConfigSkeleton) and CompletionSettings (the API for plugins to access these settings)</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="http://git.reviewboard.kde.org/r/112590/diff/2/?file=188213#file188213line49" style="color: black; font-weight: bold; text-decoration: underline;">shell/completionsettings.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">49</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">CompletionLevel</span> <span class="n">level</span><span class="p">(</span><span class="n">m_level</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">47</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="n">static_cast</span><span class="o"><</span><span class="n">CompletionLevel</span><span class="o">></span><span class="p">(</span><span class="n">readIntConfig</span><span class="p">(</span><span class="s">"completionLevel"</span><span class="p">,</span> <span class="n">static_cast</span><span class="o"><</span><span class="kt">int</span><span class="o">></span><span class="p">(</span><span class="n">MinimalWhenAutomatic</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;">You should not cast integers read from the config file like this. What happens if the config file contains an int that does not fall into the range used for the enum? IMO having the individual flags as separate boolean entries in the config is also more readable inside the config file. So that should be kept, which means there needs to be code added to actually change those boolean members in the ccpreferences class when another entry is selected.</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="http://git.reviewboard.kde.org/r/112590/diff/2/?file=188217#file188217line66" style="color: black; font-weight: bold; text-decoration: underline;">shell/settings/ccpreferences.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">CCPreferences::CCPreferences( QWidget *parent, const QVariantList &args )</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">63</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">connect</span><span class="p">(</span><span class="n">preferencesDialog</span><span class="o">-></span><span class="n">completionDetail</span><span class="p">,</span> <span class="n">SIGNAL</span><span class="p">(</span><span class="n">currentIndexChanged</span><span class="p">(</span><span class="kt">int</span><span class="p">)),</span> <span class="n">SLOT</span><span class="p">(</span><span class="n">save</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;">As I said above, this should be connecting to a custom slot which updates the 3 bools related to the level and then emits a settings-changed signal.</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="http://git.reviewboard.kde.org/r/112590/diff/2/?file=188217#file188217line89" style="color: black; font-weight: bold; text-decoration: underline;">shell/settings/ccpreferences.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">CCPreferences::CCPreferences( QWidget *parent, const QVariantList &args )</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">84</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">//set values manually, because automatically it becomes saved only on application exit.</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 don't think thats the reason, its rather that the CompletionSettings class exposes all these settings to plugins and hence needs to be updated manually in case the config changes.</pre>
</div>
<br />
<p>- Andreas</p>
<br />
<p>On September 8th, 2013, 3:05 p.m. UTC, Vlas Puhov wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://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 KDevelop.</div>
<div>By Vlas Puhov.</div>
<p style="color: grey;"><i>Updated Sept. 8, 2013, 3:05 p.m.</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;">CCPreferences class is written with use of KCModule, so all setting can be saved/loaded automatically. For that widget's names should start with kcfg_ prefix, but neither todoMarkerWords nor completionDetail had one, so this feature didn't work.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=324510">324510</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>shell/settings/ccpreferences.cpp <span style="color: grey">(0347fc6)</span></li>
<li>shell/settings/ccpreferences.h <span style="color: grey">(4ecf623)</span></li>
<li>shell/settings/ccconfig.kcfg <span style="color: grey">(1bc89b1)</span></li>
<li>shell/completionsettings.cpp <span style="color: grey">(6b6adf6)</span></li>
<li>shell/completionsettings.h <span style="color: grey">(b3f28ce)</span></li>
<li>shell/settings/ccsettings.ui <span style="color: grey">(64d24e2)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/112590/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>