Checkbox

Merge lp://qastaging/~roadmr/checkbox/checkbox-qt-i18n into lp://qastaging/checkbox

checkbox-qt-i18n
Merge into trunk

Proposed by Daniel Manrique on 2012-03-23

Status:

Merged

Merged at revision:

1345

Proposed branch:

lp://qastaging/~roadmr/checkbox/checkbox-qt-i18n

Merge into:

lp://qastaging/checkbox

Diff against target:

2602 lines (+1083/-535) (has conflicts)

11 files modified

debian/changelog (+8/-0)
po/Makevars (+42/-0)
po/POTFILES.in (+11/-1)
po/checkbox.pot (+881/-497)
qt/frontend/checkboxtr.cpp (+48/-0)
qt/frontend/checkboxtr.h (+46/-0)
qt/frontend/frontend.pro (+6/-2)
qt/frontend/main.cpp (+4/-0)
qt/frontend/qtfront.cpp (+34/-34)
qt/frontend/qtfront.h (+1/-0)
qt/frontend/treemodel.cpp (+2/-1)

Text conflict in debian/changelog

To merge this branch:

bzr merge lp://qastaging/~roadmr/checkbox/checkbox-qt-i18n

High

Fix Released

Link a bug report

Reviewer	Date Requested	Status
TienFu Chen (community)		Approve on 2012-04-02
Daniel Manrique (community)		Needs Resubmitting on 2012-03-28
Sylvain Pineau (community)	2012-03-23	Needs Information on 2012-03-28
Review via email: mp+98965@code.qastaging.launchpad.net

Description of the change

This adds i18n support via gettext to checkbox-qt, to address concerns from the translations team about the new checkbox UI.

What I did basically:

- Adder wrapper checkboxTr functions, inspired from unity-2d's implementation.
- Modified main.cpp and qtfront.h to include headers and initialize gettext.
- Modified frontend.pro to instruct uic-qt4 to use checkboxTr as a translation
  function.
- Added checkbox-qt files to po/POTFILES.in, so intltool gets them. NOTE that
  this depends on uic-qt4 running *before* intltool to generate ui_qtfront.h.
- Added po/Makevars to instruct intltool to extract strings from checkboxTr
  calls. Thanks to Gabor Kelemen for pointers on this.

Note that I didn't wrap all text strings in qt/frontend/*.cpp files, I saw a few that use a "tr" function that I didn't touch, and I'm sure some strings are not wrapped. They need to be wrapped in checkboxTr("string",0).

I'd appreciate review of this in case I did something wrong (maybe run it by the ubuntu-translators crowd), and if it gets merged, it'd be good to go over the .cpp files and checkboxTr-wrap any stray strings in another merge request.

Also note that, due to the criticality of the linked bug, this may be worth a Freeze Exception so it can be merged into the Ubuntu version.

lp://qastaging/~roadmr/checkbox/checkbox-qt-i18n updated on 2012-03-28

1342. By Daniel Manrique on 2012-03-28: wrapped a few more strings in checkboxTr
1343. By Daniel Manrique on 2012-03-28: Updated the checkbox.pot file

Revision history for this message

Sylvain Pineau (sylvain-pineau) wrote on 2012-03-28:

checkboxTr function works well and the resulting checkbox.pot now includes strings from the header ui file and treemodel.cpp.

But why leaving some strings not wrapped (e.g. qtfront.cpp)?
Do you prefer to include the checkboxTr mechanism asap and do the rest of the job later ?
And last question, do we have to update the checkbox.pot in bzr to reflect the new additions ?

review: Needs Information

Revision history for this message

Daniel Manrique (roadmr) wrote on 2012-03-28:

I think it'd be worth wrapping the remaining strings in checkboxTr. I translated the ones I spotted, but I may have missed a few.

I'll work on them (I think the ones in lines 87-106 are the main ones to work on) and update the MR in a moment. If you spot any more, please let me know.

review: Needs Fixing

lp://qastaging/~roadmr/checkbox/checkbox-qt-i18n updated on 2012-03-28

1344. By Daniel Manrique on 2012-03-28: Wrapped last batch of strings in checkboxTr
1345. By Daniel Manrique on 2012-03-28: minor changelog update

Revision history for this message

Daniel Manrique (roadmr) wrote on 2012-03-28:

I wrapped the remaining strings and did a minor update to the changelog.

review: Needs Resubmitting

lp://qastaging/~roadmr/checkbox/checkbox-qt-i18n updated on 2012-03-28

1346. By Daniel Manrique on 2012-03-28: Set localedir to a sane value - otherwise things *don't* work

Revision history for this message

TienFu Chen (ctf) wrote on 2012-04-02:

Hi Daniel,
I set up the destination language in system configuration, the Qt string has been translated to the destination locale. Not sure what's wrong with setting up the environment variables LANG,LANGUAGE,LC_ALL that I tried during the sprint.
Thanks for the works.

review: Approve

Revision history for this message

David Planella (dpm) wrote on 2012-04-03:

Daniel has asked me to have a look at the merge proposal. While I don't think I'll manage to review in detail today, I can say that reading the MP's description all the high level steps make perfect sense to me.

You might have already addressed this, but it would make sense to make the unity 2d translation wrapper function as much generic as possible, as I could well imagine that future Ubuntu projects will make use of it, and it will be much easier to set up those projects if it can be used in a generic way.

As per Sylvain's comment, the .pot file should be updated every time you do a release, or every time you've got a set of new strings that you think translators can work on.

In terms of the Ubuntu packaging, the package will need to generate the .pot file as part of the build so translations for the source package can be shipped in language packs (have a look at dh_translations and have a chat with pitti if you've got questions on this area).

Good work, non-English users will appreciate it!