-----------------------------------------------------------
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
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to