> On Aug. 26, 2013, 1:35 p.m., Kevin Ottens wrote:
> > staging/xmlgui/src/kswitchlanguagedialog_p.cpp, line 333
> > <http://git.reviewboard.kde.org/r/112288/diff/1/?file=184765#file184765line333>
> >
> > From QLocale documentation if the ctor didn't find the language in the
> > database it then uses ::default(). It just happens in your case that
> > ::system() == ::default(), but I think we should test for default() not
> > system().
> >
> > Now that means that your default language will never get in the combo
> > box... Don't you end up with an empty combo box with that patch? That would
> > be a problem.
> >
> > I wonder if we shouldn't store the default locale before the loop, set
> > C to be the new default and test on that instead. And last restore the
> > previous default language after the loop.
> >
> > It's jumping through hoops a bit but we're out of the usual QLocale
> > uses it seems.
To be honest I've been thinking about this code and I don't like it (Aleix and
me did it together). Sure apply this patch, but a better way for us to find the
instaled languages other than
for ( int i = 2; i <= QLocale::LastLanguage; ++i )
which doesn't work for us anyway since there's pt and pt_BR that is not covered
here, basically my idea is to just iterate over
QStandardPaths::standardLocations(QStandardPaths::GenericDataLocation) +
QString::fromLatin1("locale/"); <-- note this does not compile :D
(i.e. /usr/share/locale) to get the languages and then pass that to
isApplicationTranslatedInto
That should be muuuch better than the current loop we have, but don't prevent
my suggestion for a better solution accept this if it is an improvement to the
current one
- Albert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112288/#review38639
-----------------------------------------------------------
On Aug. 26, 2013, 1:06 p.m., Vishesh Handa wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112288/
> -----------------------------------------------------------
>
> (Updated Aug. 26, 2013, 1:06 p.m.)
>
>
> Review request for KDE Frameworks and Aleix Pol Gonzalez.
>
>
> Description
> -------
>
> If QLocale cannot find the appropriate language, it then defaults to the
> system locale. When adding all possible languages it is possible that
> QLocale::system() is added multiple times. This results in a huge list
> for the default language being added.
>
> For me, "US English" gets added over 50 times.
>
>
> Diffs
> -----
>
> staging/xmlgui/src/kswitchlanguagedialog_p.cpp 894f2f4
>
> Diff: http://git.reviewboard.kde.org/r/112288/diff/
>
>
> Testing
> -------
>
> Ran kwindowtest and compared the list of languages shown in the switch
> languages dialog.
>
>
> Thanks,
>
> Vishesh Handa
>
>
_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
