aacid added a comment.
Two more small comments from my side, otherwise looks cool :) Thanks for the
perseverance!
I can't approve it because the review is on my name ^_^
Maybe @apol can give it the ship it?
INLINE COMMENTS
> klanguagenametest.cpp:80
> + }
> +};
> +
Do you think it makes sense adding this?
void testNoString()
{
// Qt doesn't have za support so no string at all
QCOMPARE(KLanguageName::nameForCode("za"), QString());
}
> sitter wrote in klanguagename.cpp:30
> Do you mean both being **a** code or both being **the** code?
>
> a code
> ======
>
> `parts.at(0)` is the code of the current locale. The only way I found to get
> the 639 code from qlocale is by splitting name
> http://doc.qt.io/qt-5/qlocale.html#name
>
> Perhaps a comment is needed to explain this?
>
> the code
> ========
>
> Both params being the variable `code` wouldn't be right I think. The
> documentation for `nameForCode` says it will ideally give you the localized
> name of code in the system language.
>
> LANGUAGE=en nameForCode('en') => 'English'
> LANGUAGE=fr nameForCode('en') => 'anglaise'
>
> which internally is
>
> nameForCodeInLocale('en', 'en')
> nameForCodeInLocale('en', 'fr')
Argggg, ignore me i was reading the code wrong. Sorry for the noise
> klanguagename.h:34
> + *
> + * @since 5.44
> + *
Needs to be 5.55 i think
REPOSITORY
R265 KConfigWidgets
REVISION DETAIL
https://phabricator.kde.org/D10446
To: aacid
Cc: hein, kde-frameworks-devel, sitter, markg, apol, michaelh, ngraham, bruns
