Review Request 112785: Add ki18n_wrap_ui macro to ki18nMacros

[Stephen Kelly]

> On Sept. 23, 2013, 10:37 a.m., Kevin Ottens wrote:
> > I'm surprised it doesn't use qt5_wrap_ui. It seems to reinvent it at least partly.
> 
> Jeremy Whiting wrote:
>     well, qt5_wrap_ui wasn't around when this was created (as kde4_add_ui_files iirc). All I did was copy it and rename it. didn't look into making it use qt5_wrap_ui.
> 
> Kevin Ottens wrote:
>     Could you please look into it?
> 
> Chusslove Illich wrote:
>     This is why I asked Jeremy in the other review, that motivated this one, to
>     commit using the plain qt5_wrap_ui, until I get to doing the proper thing
>     for k18n_wrap_ui.
>     
>     Yes, in spirit k18n_wrap_ui should be a wrapper for qt5_wrap_ui, and thus I
>     would call it k18n_qt5_wrap_ui. If uic would have some additional command
>     line options, k18n_wrap_ui would internally really be a wrapper for
>     qt5_wrap_ui; but without these options, it may end up just copying the code
>     (little as there is) from qt5_wrap_ui and adding its own stuff atop.
>     k18n_qt5_wrap_ui must also have its own macro options to support the new/
>     modified ki18n functionality (namely setting the translation domain and
>     activating the KUIT markup processing).
>
> 
> Jeremy Whiting wrote:
>     Ok, I'll leave this alone for now, and just #include <klocalizedstring.h> where we were depending on that being added to the ui_*.h files before.
> 
> Kevin Ottens wrote:
>     Is this patch abandoned or it'll see another revision soon?
> 
> Chusslove Illich wrote:
>     It should see another revision soon, from me. Or maybe that should be
>     another review (different submitter)? The topic is the same...
> 
> Stephen Kelly wrote:
>     Actually, uic should get a command line option to add an include.
>     
>     Then no macro will be needed at all (with the next CMake release - CMake 3.0)
>     
>      http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=6368bd717e1cee3b947667ea0eaee78f187a2b3b
>     
>     All that would be needed is
>     
>      set(CMAKE_AUTOUIC_OPTIONS -tr i18n -include klocalizedstring.h) 
>     
>     in KI18nConfig.cmake.
> 
> Aleix Pol Gonzalez wrote:
>     Well, we certainly don't want on /all/ calls to uic. When uic is used with projects that don't use ki18n, this shouldn't be applied.

Projects which use KI18nConfig.cmake do though, yesno?

Maybe we can encode it into the KF5::KI18n target instead though. That would be a much better solution.


- Stephen


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112785/#review40518
-----------------------------------------------------------


On Sept. 17, 2013, 7:56 p.m., Jeremy Whiting wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112785/
> -----------------------------------------------------------
> 
> (Updated Sept. 17, 2013, 7:56 p.m.)
> 
> 
> Review request for KDE Frameworks and Alexander Neundorf.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> -------
> 
> It builds and installs, but doesn't seem to be usable from within kdelibs, i.e. ki18n_wrap_ui in knewstuff/src/CMakeLists.txt fails with this. I suspect one more thing is needed to make it work from within kdelibs builds.
> 
> 
> Diffs
> -----
> 
>   tier2/ki18n/CMakeLists.txt d0ed448 
>   tier2/ki18n/KI18nConfig.cmake.in 18b6e2f 
>   tier2/ki18n/cmake/KI18NMacros.cmake PRE-CREATION 
>   tier2/ki18n/cmake/ki18nuic.cmake PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/112785/diff/
> 
> 
> Testing
> -------
> 
> Builds and installs into PREFIX/lib64/cmake/KI18N next to KI18nConfig.cmake
> 
> 
> Thanks,
> 
> Jeremy Whiting
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20131014/53ed3a7d/attachment.html>