Review Request 112288: fillApplicationLanguages: Do not add the system locale

[Kevin Ottens]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112288/#review38639
-----------------------------------------------------------



staging/xmlgui/src/kswitchlanguagedialog_p.cpp
<http://git.reviewboard.kde.org/r/112288/#comment28572>

    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.


- Kevin Ottens


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/cd1fed42/attachment.html>