Review Request 124885: Fix issues in translation control module

2015-08-22 Thread Albert Astals Cid

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124885/
---

Review request for Plasma.


Bugs: 345761 and 347956
https://bugs.kde.org/show_bug.cgi?id=345761
https://bugs.kde.org/show_bug.cgi?id=347956


Repository: plasma-desktop


Description
---

Fixes two problems:
 * Variants not being shown up, i.e. ca ca@valencia showing up both as "català"
 * pt showing up as "português do Brasil"

For the first one i've went the easy route of adding the languageCode if 
there's an @ in it
For pt i hard to hard code it since i found no other way to make Qt understand 
that for "pt" we mean portuguese from portugal


Diffs
-

  kcms/translations/kcmtranslations.cpp e5024e2 

Diff: https://git.reviewboard.kde.org/r/124885/diff/


Testing
---


Thanks,

Albert Astals Cid

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 124885: Fix issues in translation control module

2015-08-22 Thread Jeremy Whiting

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124885/#review84202
---

Ship it!


Ship It!

- Jeremy Whiting


On Aug. 22, 2015, 6:34 p.m., Albert Astals Cid wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124885/
> ---
> 
> (Updated Aug. 22, 2015, 6:34 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 345761 and 347956
> https://bugs.kde.org/show_bug.cgi?id=345761
> https://bugs.kde.org/show_bug.cgi?id=347956
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> Fixes two problems:
>  * Variants not being shown up, i.e. ca ca@valencia showing up both as 
> "català"
>  * pt showing up as "português do Brasil"
> 
> For the first one i've went the easy route of adding the languageCode if 
> there's an @ in it
> For pt i hard to hard code it since i found no other way to make Qt 
> understand that for "pt" we mean portuguese from portugal
> 
> 
> Diffs
> -
> 
>   kcms/translations/kcmtranslations.cpp e5024e2 
> 
> Diff: https://git.reviewboard.kde.org/r/124885/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 124885: Fix issues in translation control module

2015-08-22 Thread Jeremy Whiting


> On Aug. 22, 2015, 7:47 p.m., Jeremy Whiting wrote:
> > Ship It!

Incidentally, the pt issue sounds like a QLocale bug, do we need to find one or 
file a new one for that probably?


- Jeremy


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124885/#review84202
---


On Aug. 22, 2015, 6:34 p.m., Albert Astals Cid wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124885/
> ---
> 
> (Updated Aug. 22, 2015, 6:34 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 345761 and 347956
> https://bugs.kde.org/show_bug.cgi?id=345761
> https://bugs.kde.org/show_bug.cgi?id=347956
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> Fixes two problems:
>  * Variants not being shown up, i.e. ca ca@valencia showing up both as 
> "català"
>  * pt showing up as "português do Brasil"
> 
> For the first one i've went the easy route of adding the languageCode if 
> there's an @ in it
> For pt i hard to hard code it since i found no other way to make Qt 
> understand that for "pt" we mean portuguese from portugal
> 
> 
> Diffs
> -
> 
>   kcms/translations/kcmtranslations.cpp e5024e2 
> 
> Diff: https://git.reviewboard.kde.org/r/124885/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 124885: Fix issues in translation control module

2015-08-23 Thread John Layt


> On Aug. 23, 2015, 2:47 a.m., Jeremy Whiting wrote:
> > Ship It!
> 
> Jeremy Whiting wrote:
> Incidentally, the pt issue sounds like a QLocale bug, do we need to find 
> one or file a new one for that probably?

Yeah, looks a Qt bug, it has a table from CLDR of default countries to use so 
there's something wrong with the lookup. I'm not aware of a bug report for that 
so go ahead.

That, or Thiago has rigged it so he always gets pt_BR :-)


- John


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124885/#review84202
---


On Aug. 23, 2015, 1:34 a.m., Albert Astals Cid wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124885/
> ---
> 
> (Updated Aug. 23, 2015, 1:34 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 345761 and 347956
> https://bugs.kde.org/show_bug.cgi?id=345761
> https://bugs.kde.org/show_bug.cgi?id=347956
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> Fixes two problems:
>  * Variants not being shown up, i.e. ca ca@valencia showing up both as 
> "català"
>  * pt showing up as "português do Brasil"
> 
> For the first one i've went the easy route of adding the languageCode if 
> there's an @ in it
> For pt i hard to hard code it since i found no other way to make Qt 
> understand that for "pt" we mean portuguese from portugal
> 
> 
> Diffs
> -
> 
>   kcms/translations/kcmtranslations.cpp e5024e2 
> 
> Diff: https://git.reviewboard.kde.org/r/124885/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 124885: Fix issues in translation control module

2015-08-23 Thread Albert Astals Cid


> On ago. 23, 2015, 1:47 a.m., Jeremy Whiting wrote:
> > Ship It!
> 
> Jeremy Whiting wrote:
> Incidentally, the pt issue sounds like a QLocale bug, do we need to find 
> one or file a new one for that probably?
> 
> John Layt wrote:
> Yeah, looks a Qt bug, it has a table from CLDR of default countries to 
> use so there's something wrong with the lookup. I'm not aware of a bug report 
> for that so go ahead.
> 
> That, or Thiago has rigged it so he always gets pt_BR :-)

It's a "feature", gives you the countries in order of speakers, in this case 
portugal being the country with less portuguese speakers gets returned last 
after Brazil, Angola, etc. I do think it's a misfeature and weird since for 
"es" gives you Spain instead of Mexico or USA where there's more Spanish 
speakers. I'm going to try to open a bug upstream claiming that giving "pt" 
gives back "portugues do brazil" instead of "portugues" as 
https://www.loc.gov/standards/iso639-2/php/code_list.php says but not sure it's 
going to be easy so i'll commit this anyway.


- Albert


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124885/#review84202
---


On ago. 23, 2015, 12:34 a.m., Albert Astals Cid wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124885/
> ---
> 
> (Updated ago. 23, 2015, 12:34 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 345761 and 347956
> https://bugs.kde.org/show_bug.cgi?id=345761
> https://bugs.kde.org/show_bug.cgi?id=347956
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> Fixes two problems:
>  * Variants not being shown up, i.e. ca ca@valencia showing up both as 
> "català"
>  * pt showing up as "português do Brasil"
> 
> For the first one i've went the easy route of adding the languageCode if 
> there's an @ in it
> For pt i hard to hard code it since i found no other way to make Qt 
> understand that for "pt" we mean portuguese from portugal
> 
> 
> Diffs
> -
> 
>   kcms/translations/kcmtranslations.cpp e5024e2 
> 
> Diff: https://git.reviewboard.kde.org/r/124885/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 124885: Fix issues in translation control module

2015-08-23 Thread Albert Astals Cid

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124885/
---

(Updated Aug. 23, 2015, 11:42 a.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Changes
---

Submitted with commit adb75ec59296ff17ef9d07a7cae0614235c592a6 by Albert Astals 
Cid to branch Plasma/5.4.


Bugs: 345761 and 347956
https://bugs.kde.org/show_bug.cgi?id=345761
https://bugs.kde.org/show_bug.cgi?id=347956


Repository: plasma-desktop


Description
---

Fixes two problems:
 * Variants not being shown up, i.e. ca ca@valencia showing up both as "català"
 * pt showing up as "português do Brasil"

For the first one i've went the easy route of adding the languageCode if 
there's an @ in it
For pt i hard to hard code it since i found no other way to make Qt understand 
that for "pt" we mean portuguese from portugal


Diffs
-

  kcms/translations/kcmtranslations.cpp e5024e2 

Diff: https://git.reviewboard.kde.org/r/124885/diff/


Testing
---


Thanks,

Albert Astals Cid

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel