> On May 11, 2014, 5:59 p.m., John Layt wrote:
> > kcms/formats/formats.desktop, line 19
> > <https://git.reviewboard.kde.org/r/118063/diff/1/?file=272242#file272242line19>
> >
> >     accessibility?

That's the same as for the locale category. I think this category in the 
.desktop file only matters for the group, and as it seems consistent this way 
with the other options, I've left it as that. (There's a bit of discussion 
going on how to categorize, and I think this can in that case be revisitited.


> On May 11, 2014, 5:59 p.m., John Layt wrote:
> > kcms/formats/kcmformats.cpp, line 113
> > <https://git.reviewboard.kde.org/r/118063/diff/1/?file=272244#file272244line113>
> >
> >     Perhaps "Use Default Locale"?

I've pondered that, but we have "Default (C)" as the next option then, and that 
seems to be confusing. I've therefore chosen to tell what this setting will 
effect, and that is "don't change anything". It could also be "System locale", 
but then, if the LC_* env vars are set "from the outside", before logging in, 
in .xinitrc, or some crazy stuff like that, it's not the "System locale" 
anymore.

Better ideas of course welcome...


> On May 11, 2014, 5:59 p.m., John Layt wrote:
> > kcms/formats/kcmformats.cpp, line 204
> > <https://git.reviewboard.kde.org/r/118063/diff/1/?file=272244#file272244line204>
> >
> >     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.

Hm, LANG will be set from the Translations KCM, and affects that, no? (I might 
be off here, that's how I understand it.)

This brings me back a bit to the way this KCM works, it's used to override 
settings specified elsewhere, if necessary. I think in combination with the 
Translations KCM, a clean separation makes sense, but I'm not 100% sure that's 
achievable. Effectively, with the current version of code, we have a few 
scenarios:

- Language set from distro installer, however. User's happy, doesn't touch the 
KCM
- User sets Language in the Translations KCModule, which sets LANG (in the same 
fashion as we do here), LC_* is not set, so everything falls back to LANG -> 
we're peachy
- User sets Language, but wants something else changed, configures that in the 
Formats KCM, and it's applied by exporting LC_*, -> we're fine again

The Translations KCM probably the first one we should show when the category in 
systemsettings is opened, as it allows a very "Global" setting: LANG is 
changed, affects translations, and additionally all the formatting because LC_* 
not set means fall back to LANG.


> On May 11, 2014, 5:59 p.m., John Layt wrote:
> > kcms/formats/kcmformats.cpp, line 88
> > <https://git.reviewboard.kde.org/r/118063/diff/1/?file=272244#file272244line88>
> >
> >     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.

I've made it use the list of Locales, just like in the other combos.


> On May 11, 2014, 5:59 p.m., John Layt wrote:
> > kcms/formats/kcmformats.cpp, line 142
> > <https://git.reviewboard.kde.org/r/118063/diff/1/?file=272244#file272244line142>
> >
> >     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 :-)

The combo values are "locale.nativeCountryName() - locale.nativeLanguageName() 
(locale->name())" now, I'm sure our Catalan friends will be happy. :)


> On May 11, 2014, 5:59 p.m., John Layt wrote:
> > kcms/formats/kcmformats.cpp, line 294
> > <https://git.reviewboard.kde.org/r/118063/diff/1/?file=272244#file272244line294>
> >
> >     Shouldn't lcGlobal just be exported as LANG?  The lcCollate and lcCtype 
> > should be read form the config and exported in their own right?

lcCtype now checks if it's set from config, and if so, that's used, otherwise 
the global setting. I don't want to add another combo for this, as this setting 
is almost impossible to explain to the user, so it follows the global setting.


> On May 11, 2014, 5:59 p.m., John Layt wrote:
> > kcms/formats/kcmformats.cpp, line 363
> > <https://git.reviewboard.kde.org/r/118063/diff/1/?file=272244#file272244line363>
> >
> >     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.

I've made the example just show the measurement system. Good enough for now.


- Sebastian


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


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

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

Reply via email to