Review Request: Reset time format upon user request

[Lamarque Vieira Souza]

> On Dec. 18, 2011, 8:33 a.m., Aaron J. Seigo wrote:
> > plasma/generic/applets/digital-clock/clock.cpp, lines 100-102
> > <http://git.reviewboard.kde.org/r/103434/diff/2/?file=43561#file43561line100>
> >
> >     this still realy ought to be done in klocale

Well, we could add a KLocale::commit() call to make any changes in KLocale visible to everybody. That call would replace the KGlobalSettings::self()->emitChange(KGlobalSettings::SettingsChanged, KGlobalSettings::SETTING_COMPLETION) that I am using in plasma-mobile. KLocale::commit() would use KGlobalSettings::self()->emitChange() internally, we can add KGlobalSetings::SETTINGS_LOCALE as David suggested below. There would be a private slot KLocale::updateChanges() listening to emitChange that would reparse the config files. Of course we will need to update the documentation about emitChange and that would not work for KDE SC <= 4.7.4 because of the addition of KGlobalSetings::SETTINGS_LOCALE.

In bug #289094 particular case the will be the problem of synchronizing KLocale::updateChanges() with the Clock::generatePixmap(); Clock::update() calls in the clock plasmoid. KLocale::updateChanges() would have to emit a signal to indicate the changes has been updated or the changes will take place only on the next update, which can take almost a minute in Plasma Active. The current patch update the time format instantaneously.

Another problem with this approach is that we cannot prevent anybody else from listening to KGlobalSettings::self()->emitChange(KGlobalSettings::SettingsChanged, KGlobalSettings::SETTING_LOCALE). Hmmm we could add a private enum to accomodate KGlobalSettings::SETTING_LOCALE?


- Lamarque Vieira


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


On Dec. 16, 2011, 9:13 p.m., Lamarque Vieira Souza wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103434/
> -----------------------------------------------------------
> 
> (Updated Dec. 16, 2011, 9:13 p.m.)
> 
> 
> Review request for kdelibs and Plasma.
> 
> 
> Description
> -------
> 
> The patch resets time format in digital clock plasmoid when the user changes the 24h configuration in active-settings.
> 
> The reset part is from kdelibs/kdecore/localization/klocale_kde.cpp. I am wondering if I should add this change to kdelibs instead of kde-workspace to avoid duplicating code. Anyway, I wanted someone to review the code to see if there can be any side effect.
> 
> 
> This addresses bug 289094.
>     http://bugs.kde.org/show_bug.cgi?id=289094
> 
> 
> Diffs
> -----
> 
>   plasma/generic/applets/digital-clock/clock.h 4aec3fd 
>   plasma/generic/applets/digital-clock/clock.cpp dd03692 
> 
> Diff: http://git.reviewboard.kde.org/r/103434/diff/diff
> 
> 
> Testing
> -------
> 
> Works in Plasma Active. In Plasma Desktop kcmlocale does not call KGlobalSettings::self()->emitChange(KGlobalSettings::SettingsChanged) so it does not take effect. Other kcm modules (e.g. keyboard), call emitChange.
> 
> 
> Thanks,
> 
> Lamarque Vieira Souza
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20111218/96d9af27/attachment.html>