> 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. > > Stephen Kelly wrote: > 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. > > > > Chusslove Illich wrote: > Another needed option to uic is to define a macro, for setting > TRANSLATION_DOMAIN in library code. Then, it must be possible to set > whether > ordinary or KUIT markup calls are used, i.e. whether -tr tr2i18n or > -tr tr2xi18n. So I intended that k18n_qt5_wrap_ui looks something like: > > ki18n_qt5_wrap_ui(outfilevar [DOMAIN <domain>] [KUIT] uifiles...) > > and internally call uic with proper options. For example: > > ki18n_qt5_wrap_ui(foolib_SRCS DOMAIN "foolib" KUIT > fooconfig.ui > foo.ui > ... > ) > > Before uic gets these options, ki18n_qt5_wrap_ui would internally do what > qt5_wrap_ui does plus the makeshift stuff to do the rest (as in KDE 4). > > > Stephen Kelly wrote: > > > Another needed option to uic is to define a macro, for setting > > TRANSLATION_DOMAIN in library code. > > Assuming you mean a preprocessor macro, that can be set from CMake as a > -D, right? > > I think it would be easier to get the -include in than the -domain, so > that should be aimed for separately and first. > > Chusslove Illich wrote: > Right, a preprocessor macro. And I meant setting it by implementing the > -define option in uic, rather than the more specific -domain. > > But how to set all this information is just a matter of convenience. If > add_definitions plus qt5_wrap_ui with explicit -tr option (and -include > unless CMAKE_AUTOUIC_OPTIONS is used) is deemed good enough, then > ki18n_qt5_wrap_ui is not needed indeed. > > > Stephen Kelly wrote: > > > But how to set all this information is just a matter of convenience. If > > add_definitions plus qt5_wrap_ui > > That would also not be needed. The required options to uic would be used > simply by linking to KI18n. > > > Chusslove Illich wrote: > So, one must be able to set two things. Add the TRANSLATION_DOMAIN macro; > this can be done by add_definitions. Choose whether -tr tr2i18n or > -tr tr2xi18n is issued to uic; how can this be done? > > > Stephen Kelly wrote: > I think there's no need for the add_definitions. > > We would add something like this to ki18n: > > set_property(TARGET KI18n PROPERTY INTERFACE_AUTOUIC_OPTIONS -include > klocalizedstring -tr tr2i18n -define > TRANSLATION_DOMAIN=$<TARGET_PROPERTY:NAME>) > > Additionally, if the TRANSLATION_DOMAIN is needed in c++ code that I > write as a developer, we should this to ki18n: > > set_property(TARGET KI18n PROPERTY INTERFACE_COMPILE_DEFINITIONS > TRANSLATION_DOMAIN=$<TARGET_PROPERTY:NAME>) > > That assumes that -DTRANSLATION_DOMAIN=foo should be used when compiling > the foo target. Is that the case? > > Because are INTERFACE_ properties, when I use > > add_library(user mywidget.cpp) > target_link_libraries(user KF5::KI18n) > > CMake will use those options when running uic on mywidget.ui. > > > Chusslove Illich wrote: > I didn't look up yet how this set_property mechanism works, but in your > example I still don't see where the translation domain name is given, and > how it can be selected whether -tr tr2i18n or -tr tr2xi18n is issued to > uic. > It appears to me you want to link the translation domain name to the build > target name; while sometimes these would be same, sometimes they wouldn't > be. The translation domain name should be explicitly given. > > Right, we would normally want -DTRANSLATION_DOMAIN= for all the code in a > target, including *.cpp files. >
Any chance to see an update or it is abandonned? - Kevin ----------------------------------------------------------- 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 > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
