D10446: Add KLanguageName

[Albert Astals Cid]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190108/e56b9270/attachment.html>