This may be a stupid question but is the TRANSLATION_DOMAIN stuff actually used somewhere? I'm not familiar with how it works. However, I find it strange that its not defined for kwrite and kate but is defined for all plugins.

Modern CMake has a recommendation to avoid variables when working w/ targets (google "Variables are fragile").
With a small modification to ki18n_wrap_ui (yeah, another project), it could be possible to replace this:

set(katebacktracebrowserplugin_PART_SRCS
  katebacktracebrowser.cpp
  btparser.cpp
  btfileindexer.cpp
  btdatabase.cpp
)

set(katebacktracebrowserplugin_PART_UI
  btbrowserwidget.ui
  btconfigwidget.ui
)

ki18n_wrap_ui(katebacktracebrowserplugin_PART_SRCS ${katebacktracebrowserplugin_PART_UI})


add_library(katebacktracebrowserplugin MODULE ${katebacktracebrowserplugin_PART_SRCS})

with this:

add_library(
  katebacktracebrowserplugin MODULE 
  katebacktracebrowser.cpp
  btparser.cpp
  btfileindexer.cpp
  btdatabase.cpp
)

ki18n_wrap_ui(
  katebacktracebrowserplugin # NOTE passing the target name!
  btbrowserwidget.ui
  btconfigwidget.ui
)

the main point is to avoid variables (some of them just one time used) and "make the target declaration the only source of truth" %)

ki18n_wrap_ui should learn to accept a target name as the first argument and call target_sources(<tgt>...) to update target sources directly instead of storing 'em to a variable. For the backward compatibility, it's possible to check the first argument with if(TARGET...) and do the direct update or fallback to the old behavior (assuming that was a variable name to update) on else()...

If you pending some other CMake related patches and have some spare time please add this feature too %)))

In D22721#501682, @daandemeyer wrote:

This may be a stupid question but is the TRANSLATION_DOMAIN stuff actually used somewhere? I'm not familiar with how it works. However, I find it strange that its not defined for kwrite and kate but is defined for all plugins.

Short story: yes, it is used, silently by all the *i18n* calls which take this definition as first argument (hidden behind a macro that any *i18n* calls is). For kate & kwrite you do not see that, as there is a default catalog defined per application, and both kate & kwrite set that using KLocalizedString::setApplicationDomain("fooapp");.And for libs & plugins TRANSLATION_DOMAIN allows to overwrite the default catalog in that macro-magic way in each translation look-up call, to make sure their own catalog as defined by the domain is used.

If you want to get more familiar, please see https://api.kde.org/frameworks/ki18n/html/prg_guide.html#link_cat

@turbov This is exactly what I've already proposed, See https://phabricator.kde.org/D22698. I've also proposed similar patches for ecm_add_app_icon and ecm_qt_declare_logging_category (https://phabricator.kde.org/D22709, https://phabricator.kde.org/D22699)

@kossebau Thanks for the explanation.

I think the overall direction of these changes is good, some cleanup and modernization seems in order.
@turbov provides useful hints to improve, once you reached a fixed-point, just ping me again, I see no reason to not merge this then.

@cullmann I think this revision is good to merge now. I was thinking of making a second revision with the changes to the global, kwrite and kate CMake scripts to keep things a bit more manageable. However, those changes depend on this revision so ideally this revision would be merged before I make the second one. Does that sound good to you or should I add the other changes to this revision as well?

Is there also a tutorial on building plugins somewhere that needs to be updated in tandem with these changes?

Could this merge be the reason for this compile error:

https://build.neon.kde.org/job/bionic_unstable_kde_kate_bin_amd64/243/console

My bad, I forgot to replace a source list variable with the empty string.
This does not cause errors on the latest CMake versions because it is now
allowed to pass no sources to an add_executable call so I didn't have any
problems locally. The issue is solved by replacing ${ProjectPluginSrc} in
addons/project/autotests/CMakeLists.txt by the empty string ("")

Do I need to make a revision for this on phabricator or it is faster to fix
it locally?

Regards,

Daan

The new revision should fix the compile error

Hmm, could you attach just a diff to latest master?
arc is too dumb to handle this ;=)