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


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 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-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


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