Review Request 118063: New Formats KCM

[David Edmundson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118063/#review57648
-----------------------------------------------------------



kcms/formats/kcmformats.cpp
<https://git.reviewboard.kde.org/r/118063/#comment40091>

    Leaks.



kcms/formats/kcmformats.cpp
<https://git.reviewboard.kde.org/r/118063/#comment40092>

    I assume you mean kcmshell ?
    If so please file a bug. This is quite critical.



kcms/formats/kcmformats.cpp
<https://git.reviewboard.kde.org/r/118063/#comment40093>

    Do you need the <p> tags here?
    



kcms/formats/kcmformats.cpp
<https://git.reviewboard.kde.org/r/118063/#comment40096>

    Defaults isn't meant to do the same as cancel and re-opening.
    It should reset to whatever system defaults should be.



kcms/formats/kcmformats.cpp
<https://git.reviewboard.kde.org/r/118063/#comment40095>

    I don't think we can hardcode this example.
    
    One wouldn't have 24.99 yen.
    and we have to take nloc into account to switch between . and ,



kcms/formats/kcmformats.cpp
<https://git.reviewboard.kde.org/r/118063/#comment40094>

    This seems wrong.
    
    No-one would say
    4km 4.3cm.
    
    It would either by 
    
    4000.0043m
    or 4km, 4cm, 3mm
    
    Same for imperial.
    
    Can we use KUnitConversion::Length  directly, which will do the right thing.


- David Edmundson


On May 9, 2014, 4:05 p.m., Sebastian Kügler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118063/
> -----------------------------------------------------------
> 
> (Updated May 9, 2014, 4:05 p.m.)
> 
> 
> Review request for Plasma and John Layt.
> 
> 
> Bugs: 331930
>     https://bugs.kde.org/show_bug.cgi?id=331930
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> -------
> 
> The current "Locale" KCM is almost entirely broken. The way forward is to split it into localization options (this, Formats), and translaticons. This patch implements a new Formats KCM which writes out an environment suitable for QLocale to pick it up.
> 
> It's a rewrite of the current KCM, since:
> - everything under the hood is different (KLocale is gone, QLocale takes over)
> - everything above the hood is different, in QLocale, everything is deduced from the country / region
> 
> Now this includes feature regressions compared to the old KCM, but not all of these features can be done at all on top of QLocale right now, so we have to set up what we can.
> 
> This KCM caches the settings in a config file, but most importantly, it writes out a script with export instructions, which can be picked up by startkde. This is all done according to John's suggestions, and I have a patch for startkde to pick up the settings here. It all works fine (on my machine).
> 
> Many more details about the implementation can be found in the plasma-devel thead "Locale settings in Plasma Next", started by John on February, 23rd 2014.
> 
> 
> Diffs
> -----
> 
>   kcms/formats/kcmformatswidget.ui PRE-CREATION 
>   kcms/formats/kcmformats.cpp PRE-CREATION 
>   kcms/formats/kcmformats.h PRE-CREATION 
>   kcms/formats/Messages.sh PRE-CREATION 
>   kcms/formats/formats.desktop PRE-CREATION 
>   kcms/formats/CMakeLists.txt PRE-CREATION 
>   kcms/CMakeLists.txt ed86efa 
> 
> Diff: https://git.reviewboard.kde.org/r/118063/diff/
> 
> 
> Testing
> -------
> 
> Tried all kinds of different combinations, applied them, made sure they're exported correctly in the script.
> 
> 
> File Attachments
> ----------------
> 
> new Formats KCM in kcmshell5
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/05/09/873980fe-04f2-4d75-9074-2aa676723061__formatskcm.png
> Formats KCM in systemsettings
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/05/09/86a830ed-2d5d-4bdd-ba82-71ec73e6ce2c__formatskcmss.png
> 
> 
> Thanks,
> 
> Sebastian Kügler
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20140509/fd27a2fa/attachment-0001.html>