Review Request 112288: fillApplicationLanguages: Do not add the system locale
Vishesh Handa
me at vhanda.in
Mon Aug 26 14:30:55 UTC 2013
> 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.
>
> Albert Astals Cid wrote:
> 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
@Kevin: One could also just do -
bool addedDefault = false;
for ( ... ) {
if (l == QLocale())
if (!addedDefault) {
addedDefault = true;
}
else
continue;
...
}
Or we could just add the default one in the beginning/end. Let me know which you prefer. I've already implemented the default swapping one.
- Vishesh
-----------------------------------------------------------
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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130826/8c8460bb/attachment-0001.html>
More information about the Kde-frameworks-devel
mailing list