Review Request 118063: New Formats KCM

John Layt jlayt at kde.org
Sun May 11 17:59:03 UTC 2014 

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



kcms/formats/formats.desktop
<https://git.reviewboard.kde.org/r/118063/#comment40181>

    s/language/formats



kcms/formats/formats.desktop
<https://git.reviewboard.kde.org/r/118063/#comment40182>

    "Language" shouldn't be there, perhaps "Number, time, currency and other formats for your particular region"



kcms/formats/formats.desktop
<https://git.reviewboard.kde.org/r/118063/#comment40183>

    accessibility?



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

    I think we should add a combo for Collation to the GUI.



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

    While this approach is easier and makes more sense to the user, I don't think it will work in the world of POSIX locales. I believe the LC_MEASUREMENT locale set may also affect the formatting of the numbers i.e. the decimal and grouping separators used, not just the units.  We probably need to check that and if so just use the same list of locales as the others.  If it doesn't, then there's actually 3 different possible values, Metric, US Imperial, and UK Imperial.



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

    Perhaps "Use Default Locale"?



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

    countryToString() doesn't do what you'd think, it just converts something an enum like QLocale::UnitedStates to the string "UnitedStates".  What you really want is nativeCountryName().



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

    I'm not sure defaulting to the German flag will be popular everywhere :-)  Perhaps if there's no country then we just don't try load the name, or perhaps have a default blank flag?



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

    Damn, I thought we'd made countryCode() and splitLocale() public api in QLocale already :-\ *adds to TODO list*



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

    The flag resource is actually from kdelibs4support so won't be around for long, and have recently moved to a different location (kf5/locale/countries/).  They will make a comeback in a new Framework in a few months time though, so are probably OK to stay this way for now.



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

    Hmmmm.  I do prefer listing by Country rather than Language (it's more natural to users I think), and I sort-of like showing the locale code so experts knwo what they are getting, but I think we do need to include the name of the language somehow (just ask your Catalans how they feel :-)



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

    Why are you writing all these entries here?  If all they have set is the global then all we need to export is LANG, so only writing out lcGlobal should be enough. If there's no overrides we should always be deleting them.



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

    Shouldn't lcGlobal just be exported as LANG?  The lcCollate and lcCtype should be read form the config and exported in their own right?



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

    Actually, Yen does have subunits called Sen, its just thanks to inflation they're not used anymore :-)  It's a valid concern though, but I think the format code takes care of it by using a setting of 0 decimal places, so the .99 is thrown away.  It's worth a test though.  Also, the toCurrencyString() call does the right thing in figuring out whether to use "," or ".", that's all part of the settings defined by choosing the locale to use rather than the currency code.



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

    We need to check what it actually affects and use that as an example.  Problem is QLocale doesn't have any methods that use this other than measurementSystem() returning Metric/ImperialUK/ImperialUS. If my comment above about needing to show the locale names in teh combo is correct, then just showing the resultign system name shoudl be enough.  Otherwise we need to find something that will format using LC_MEASUREMENT.  Or maybe we just leave it out for now.  It needs research.


- John Layt


On May 9, 2014, 5: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, 5: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/20140511/87e3cf8d/attachment-0001.html>

More information about the Plasma-devel mailing list