Re: Review Request 114889: Fix KIO::convertSize(...) returning string with (I18N_ARGUMENT_MISSING) inside

2014-01-08 Thread Chusslove Illich

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

Ship it!


Ship It!

- Chusslove Illich


On Jan. 7, 2014, 5:24 p.m., Friedrich W. H. Kossebau wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114889/
 ---
 
 (Updated Jan. 7, 2014, 5:24 p.m.)
 
 
 Review request for kdelibs.
 
 
 Repository: kio
 
 
 Description
 ---
 
 Seems the code behind i18n() in kf5 does not like %-placeholders without any 
 arguments passed in the i18n call. And thus inserts the warning.
 (Effect can be seen e.g. in Okteta's File Info tool).
 
 Attached patch fixes that, by passing as argument a string with an argument 
 placeholder again.
 
 
 Diffs
 -
 
   src/core/global.cpp 42f453b 
 
 Diff: https://git.reviewboard.kde.org/r/114889/diff/
 
 
 Testing
 ---
 
 Results of KIO::convertSize(...) displays fine in Okteta with the patch.
 
 
 Thanks,
 
 Friedrich W. H. Kossebau
 




Re: Review Request 114889: Fix KIO::convertSize(...) returning string with (I18N_ARGUMENT_MISSING) inside

2014-01-08 Thread Commit Hook

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


This review has been submitted with commit 
149fd780c71b3d8d3d4098f43f6cb746a3ef6ada by Friedrich W. H. Kossebau to branch 
master.

- Commit Hook


On Jan. 7, 2014, 4:24 p.m., Friedrich W. H. Kossebau wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114889/
 ---
 
 (Updated Jan. 7, 2014, 4:24 p.m.)
 
 
 Review request for kdelibs.
 
 
 Repository: kio
 
 
 Description
 ---
 
 Seems the code behind i18n() in kf5 does not like %-placeholders without any 
 arguments passed in the i18n call. And thus inserts the warning.
 (Effect can be seen e.g. in Okteta's File Info tool).
 
 Attached patch fixes that, by passing as argument a string with an argument 
 placeholder again.
 
 
 Diffs
 -
 
   src/core/global.cpp 42f453b 
 
 Diff: https://git.reviewboard.kde.org/r/114889/diff/
 
 
 Testing
 ---
 
 Results of KIO::convertSize(...) displays fine in Okteta with the patch.
 
 
 Thanks,
 
 Friedrich W. H. Kossebau
 




Re: Review Request 114889: Fix KIO::convertSize(...) returning string with (I18N_ARGUMENT_MISSING) inside

2014-01-08 Thread Friedrich W. H. Kossebau

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

(Updated Jan. 8, 2014, 10:43 a.m.)


Status
--

This change has been marked as submitted.


Review request for kdelibs.


Repository: kio


Description
---

Seems the code behind i18n() in kf5 does not like %-placeholders without any 
arguments passed in the i18n call. And thus inserts the warning.
(Effect can be seen e.g. in Okteta's File Info tool).

Attached patch fixes that, by passing as argument a string with an argument 
placeholder again.


Diffs
-

  src/core/global.cpp 42f453b 

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


Testing
---

Results of KIO::convertSize(...) displays fine in Okteta with the patch.


Thanks,

Friedrich W. H. Kossebau



Re: Review Request 114889: Fix KIO::convertSize(...) returning string with (I18N_ARGUMENT_MISSING) inside

2014-01-07 Thread Chusslove Illich

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


This code was originally like this (more or less), but someone complained
about performance of unit formatting. Then the code was switched away from
normal i18n calls, to use an internal low-level method of KLocale that would
fetch translation only once and cache it, later only substituting the
argument. When splitting into frameworks, this internal method was no longer
available, so the quick switch back to normal i18n left it in this semi-
correct state. The question now is whether to use correct i18n, as in your
patch, or to go back to a caching solution.

Without that low-level method, the caching solution would go by substituting
right away the literal %1 as the argument (e.g.
i18nc(size in 10^6 bytes, %1 MB, QString::fromLatin1(%1));), caching
such translation, and subsequently using .arg on the cached translations
(similarly to that in klocale_kde.cpp in KDE 4).


- Chusslove Illich


On Jan. 6, 2014, 8 p.m., Friedrich W. H. Kossebau wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114889/
 ---
 
 (Updated Jan. 6, 2014, 8 p.m.)
 
 
 Review request for kdelibs.
 
 
 Repository: kio
 
 
 Description
 ---
 
 Seems the code behind i18n() in kf5 does not like %-placeholders without any 
 arguments passed in the i18n call. And thus inserts the warning.
 (Effect can be seen e.g. in Okteta's File Info tool).
 
 Attached patch fixes that, by delaying the argument substitution as proposed 
 in 
 http://api.kde.org/frameworks-5.x-api/frameworks-apidocs/ki18n/html/prg_guide.html#spec_usage
 
 
 Diffs
 -
 
   src/core/global.cpp 42f453b 
 
 Diff: https://git.reviewboard.kde.org/r/114889/diff/
 
 
 Testing
 ---
 
 Results of KIO::convertSize(...) displays fine in Okteta with the patch.
 
 
 Thanks,
 
 Friedrich W. H. Kossebau
 




Re: Review Request 114889: Fix KIO::convertSize(...) returning string with (I18N_ARGUMENT_MISSING) inside

2014-01-07 Thread Chusslove Illich

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



src/core/global.cpp
https://git.reviewboard.kde.org/r/114889/#comment33517

The messages are no longer dumb (i.e. they have proper argument 
substitution), so all these comments should be removed.


- Chusslove Illich


On Jan. 6, 2014, 8 p.m., Friedrich W. H. Kossebau wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114889/
 ---
 
 (Updated Jan. 6, 2014, 8 p.m.)
 
 
 Review request for kdelibs.
 
 
 Repository: kio
 
 
 Description
 ---
 
 Seems the code behind i18n() in kf5 does not like %-placeholders without any 
 arguments passed in the i18n call. And thus inserts the warning.
 (Effect can be seen e.g. in Okteta's File Info tool).
 
 Attached patch fixes that, by delaying the argument substitution as proposed 
 in 
 http://api.kde.org/frameworks-5.x-api/frameworks-apidocs/ki18n/html/prg_guide.html#spec_usage
 
 
 Diffs
 -
 
   src/core/global.cpp 42f453b 
 
 Diff: https://git.reviewboard.kde.org/r/114889/diff/
 
 
 Testing
 ---
 
 Results of KIO::convertSize(...) displays fine in Okteta with the patch.
 
 
 Thanks,
 
 Friedrich W. H. Kossebau
 




Re: Review Request 114889: Fix KIO::convertSize(...) returning string with (I18N_ARGUMENT_MISSING) inside

2014-01-07 Thread Friedrich W. H. Kossebau

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

(Updated Jan. 7, 2014, 4:24 p.m.)


Review request for kdelibs.


Changes
---

Updated patch to follow Chusslove's proposal for runtime performance 
improvement.


Repository: kio


Description (updated)
---

Seems the code behind i18n() in kf5 does not like %-placeholders without any 
arguments passed in the i18n call. And thus inserts the warning.
(Effect can be seen e.g. in Okteta's File Info tool).

Attached patch fixes that, by passing as argument a string with an argument 
placeholder again.


Diffs (updated)
-

  src/core/global.cpp 42f453b 

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


Testing
---

Results of KIO::convertSize(...) displays fine in Okteta with the patch.


Thanks,

Friedrich W. H. Kossebau



Review Request 114889: Fix KIO::convertSize(...) returning string with (I18N_ARGUMENT_MISSING) inside

2014-01-06 Thread Friedrich W. H. Kossebau

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

Review request for kdelibs.


Repository: kio


Description
---

Seems the code behind i18n() in kf5 does not like %-placeholders without any 
arguments passed in the i18n call. And thus inserts the warning.
(Effect can be seen e.g. in Okteta's File Info tool).

Attached patch fixes that, by delaying the argument substitution as proposed in 
http://api.kde.org/frameworks-5.x-api/frameworks-apidocs/ki18n/html/prg_guide.html#spec_usage


Diffs
-

  src/core/global.cpp 42f453b 

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


Testing
---

Results of KIO::convertSize(...) displays fine in Okteta with the patch.


Thanks,

Friedrich W. H. Kossebau