@daandemeyer
Cool stuff! :))

one more proposal is to turn it into a function to avoid pollution of the outer scope w/ inner used (and still unset) variables %)
Using set(${RESULT_VAR} "blah-blah" PARENT_SCOPE) could help to do that %)

I want to keep the changes in KDE Frameworks focused on what's necessary to clean up Kate's CMake scripts. I'll leave the general cleanup for another revision.

@alexmerry I think I added the wrong reviewer for this. I noticed your name in the git blame of k18n_wrap_ui, perhaps you know who I should add as a reviewer for ki18n revisions?

In D22698#502791, @daandemeyer wrote:

@alexmerry I think I added the wrong reviewer for this. I noticed your name in the git blame of k18n_wrap_ui, perhaps you know who I should add as a reviewer for ki18n revisions?

The maintainer is already marked as reviewer. Please also note that the reviewer is not really important: all revision of Frameworks modules are copied to the kde-frameworks-devel@ list, which is a subscriber, and which is read by people interested in the development of Frameworks.

OK, thanks for the information. I'm still getting familiar with this whole process.

Ping

(As a maintainer) I can just say, that I'm not qualified to judge if the added behavior and implementation fits the modern CMake practices. If someone else confirms this is indeed the case, I'm happy to accept it.

To give an example of how the addition would improve KDE CMake build scripts, let's look at part of Kate's CMake build script.

It currently looks like this:

# collect the needed source files
set (KATE_LIBRARY_SRCS
   kateappadaptor.cpp
   kateapp.cpp
   kateconfigdialog.cpp
   kateconfigplugindialogpage.cpp
   katedocmanager.cpp
   katefileactions.cpp
   katemainwindow.cpp
   katepluginmanager.cpp
   kateviewmanager.cpp
   kateviewspace.cpp
   katesavemodifieddialog.cpp
   katemwmodonhddialog.cpp
   katecolorschemechooser.cpp
   katequickopenmodel.cpp
   katetabbutton.cpp
   katetabbar.cpp

   # session
   session/katesessionsaction.cpp
   session/katesessionmanager.cpp
   session/katesessionmanagedialog.cpp
   session/katesession.cpp

   katemdi.cpp
   katerunninginstanceinfo.cpp
   katequickopen.cpp
   katewaiter.h
)

ki18n_wrap_ui(KATE_LIBRARY_SRCS
    ui/sessionconfigwidget.ui
    session/katesessionmanagedialog.ui
)

add_library(kdeinit_kate STATIC ${KATE_LIBRARY_SRCS})

If the proposed change is accepted, it could be refactored to the following:

add_library(kdeinit_kate STATIC "")

ki18n_wrap_ui(kdeinit_kate
    ui/sessionconfigwidget.ui
    session/katesessionmanagedialog.ui
)

target_sources(kdeinit_kate PRIVATE
   kateappadaptor.cpp
   kateapp.cpp
   kateconfigdialog.cpp
   kateconfigplugindialogpage.cpp
   katedocmanager.cpp
   katefileactions.cpp
   katemainwindow.cpp
   katepluginmanager.cpp
   kateviewmanager.cpp
   kateviewspace.cpp
   katesavemodifieddialog.cpp
   katemwmodonhddialog.cpp
   katecolorschemechooser.cpp
   katequickopenmodel.cpp
   katetabbutton.cpp
   katetabbar.cpp

   # session
   session/katesessionsaction.cpp
   session/katesessionmanager.cpp
   session/katesessionmanagedialog.cpp
   session/katesession.cpp

   katemdi.cpp
   katerunninginstanceinfo.cpp
   katequickopen.cpp
   katewaiter.h
)

• We don't need a SRCS variable anymore. Instead, generated files are added directly to the library target using the target_sources command.
• The order commands are called in becomes more intuitive. We can call add_library first, follow with the ki18n_wrap_ui command and end with specifying the long list of source files instead of having to start the script with specifying all source files.

Since you tagged me in this, I'll say that (as a former maintainer of Extra-CMake-Modules as well as someone who still makes extensive use of CMake professionally despite not doing much KDE stuff these days) the change looks good (and follows modern conventions) from a CMake perspective.

The rest of KI18N_WRAP_UI is less nice (from a modern CMake perspective), but as a minimal change to support a feature that helps write cleaner CMake code downstream, I think this is a good patch.