<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/112099/">http://git.reviewboard.kde.org/r/112099/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 15th, 2013, 2:16 p.m. UTC, <b>Albert Astals Cid</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;">Thanks for the patch!
The enum thing seems a bit overkill to me, i mean, how "expensive" is the updateSample function after all? Personally, I'd just call it and that's it. Also makes sure you don't introduce subtle bugs in the future if someone expands a function to do something else but forgets to add an | enum
Anyone else has an opinion?</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;">Without the filtering I had some severish delay when inputting, like half a second with every key press since it had to update and compute so much stuff. The enum does help things.
> Also makes sure you don't introduce subtle bugs in the future if someone expands a function to do something else but forgets to add an | enum
This already kind of exists from how the updates already need to be called from various slots.. if someone figures that they should update the sample, figuring out that an enumerator needs to be added gets kind of obvious.
In retrospect though it does seem a bit overkill but I don't know of a better way to filter things like that... 4 bools would just be flat out confusing IMO.</pre>
<br />
<p>- Santeri</p>
<br />
<p>On August 15th, 2013, 2:07 p.m. UTC, Santeri Piippo 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 KDE Runtime and Albert Astals Cid.</div>
<div>By Santeri Piippo.</div>
<p style="color: grey;"><i>Updated Aug. 15, 2013, 2:07 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;">when the input texts are changed the sample did not update and only was updated with the 1 second timer. with applied patch it updates right away.
for performance reasons updateSample() was given a flags parameter for filtering out what to update, defaults to all, i.e. original behavior.</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;">replaced kcm_locale.so in kubuntu 13.04 with git-compiled one, works fine for me.</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>kcontrol/locale/kcmlocale.h <span style="color: grey">(c7b29bc)</span></li>
<li>kcontrol/locale/kcmlocale.cpp <span style="color: grey">(62d5aef)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/112099/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>