Review Request 112099: [kcontrol/locale] update the sample when input changed

[Albert Astals Cid]

> On Aug. 15, 2013, 2:16 p.m., Albert Astals Cid wrote:
> > 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?
> 
> Santeri Piippo wrote:
>     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.

Really? half a second? That seems like a lot, what is taking so much?


- Albert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112099/#review37845
-----------------------------------------------------------


On Aug. 15, 2013, 2:07 p.m., Santeri Piippo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112099/
> -----------------------------------------------------------
> 
> (Updated Aug. 15, 2013, 2:07 p.m.)
> 
> 
> Review request for KDE Runtime and Albert Astals Cid.
> 
> 
> Description
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   kcontrol/locale/kcmlocale.h c7b29bc 
>   kcontrol/locale/kcmlocale.cpp 62d5aef 
> 
> Diff: http://git.reviewboard.kde.org/r/112099/diff/
> 
> 
> Testing
> -------
> 
> replaced kcm_locale.so in kubuntu 13.04 with git-compiled one, works fine for me.
> 
> 
> Thanks,
> 
> Santeri Piippo
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20130815/913d2f5f/attachment.htm>