Review Request 117052: Add ECMSetupQtTranslations

[Alex Merry]

> On March 25, 2014, 2:33 p.m., Alex Merry wrote:
> > The docs need cleaning up, but I'd like to concentrate on the API first.
> > 
> > I'd rather this followed the convention of other file-generating macros in getting the user to provide a variable name to store the file(s) in, rather than always using ECM_QT_TRANSLATION_LOADER.
> > 
> > I also think that the installation should be handled by the caller.  Provide a variable to store the qmfiles in, and require the caller to install them in a subdir <podir> of something in QStandardPaths::GenericDataLocation.  That way, the project can make use of the user-configurable DATA_INSTALL_DIR from KDEInstallDirs, for example, or their own installation path code.
> > 
> > I could imagine that an option to specify the name of the loader file could potentially be useful to avoid clashes, but let's leave that until someone says they want it.
> 
> Aurélien Gâteau wrote:
>     I made the macro install files itself to avoid developers getting it wrong: it is very easy to forget to install translation files or to install them in the wrong place. Most developers won't notice it because they don't test translations.
>     
>     Same with the variable name: the function could be modified to accept the name of the variable as argument, but that adds complexity and more chances of getting it wrong. I assume we don't want to support the possibility to call ecm_setup_qt_translations() twice within the same framework. If you think this is a valid use-case, though, then the macro needs to be made more customizable.
>     
>     Nevertheless, you are the maintainer, so I will make any changes you feel are needed. Can you give me the prototype of the macro you would like to see and how one should use it?

Hmm, OK, I see your point.  The macros in FindGettext do much the same thing with regards to installation.

With regards to the variable, I'd say that making the variable name explicit is likely to make it *less* error prone.  If it's there in the function call, it's a little more obvious that you're suppose to make use of it, rather than having to know about a magic variable name (it has to be added to the target either way).

However, I think being able to specify the data dir could be important; if the user changes ${DATA_INSTALL_DIR} (and sets XDG_DATA_DIRS appropriately), the translations installation location should be changed accordingly.

So let's go with:

ecm_setup_qt_translations(<source_file_var>
PO_DIR <po_dir>
POT_NAME <pot_name>
[INSTALL_DATA_DIR <install_data_dir>]
[INSTALL_SUB_DIR <install_sub_dir>]
[CREATE_LOADER])

where <install_data_dir> defaults to share and must be set to something that will be found by QStandardPaths::GenericDataLocation.


- Alex


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


On March 25, 2014, 10:48 a.m., Aurélien Gâteau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117052/
> -----------------------------------------------------------
> 
> (Updated March 25, 2014, 10:48 a.m.)
> 
> 
> Review request for Build System, Extra Cmake Modules and KDE Frameworks.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> -------
> 
> This simplifies translation handling for frameworks using Qt translation system.
> 
> 
> Diffs
> -----
> 
>   modules/ECMTrLoader.cpp.in PRE-CREATION 
>   modules/ECMSetupQtTranslations.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/117052/diff/
> 
> 
> Testing
> -------
> 
> Here is a patch which make kbookmarks use it: http://agateau.com/tmp/kbookmarks-tr.diff . The necessary changes for the Messages.sh part of the patch are being reviewed for inclusion in the l10n-kde4 repository.
> 
> 
> Thanks,
> 
> Aurélien Gâteau
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-buildsystem/attachments/20140326/11a4c4cc/attachment.html>