[GTK3] Add module to reload colorscheme in GTK3 apps without restarting them
AcceptedPublic

Authored by cblack on Mon, Mar 16, 2:01 PM.

Details

Summary

The title is somewhat self explanatory.

Test Plan

gtk-app-here --gtk-module colorreload-gtk-module

Diff Detail

Repository
R99 KDE Gtk Configuration Tool
Branch
arcpatch-D28076
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 24424
Build 24442: arc lint + arc unit
cblack requested review of this revision.Mon, Mar 16, 2:01 PM
cblack created this revision.
cblack updated this revision to Diff 77730.Mon, Mar 16, 2:02 PM

Drop extraneous meson.build

cblack updated this revision to Diff 77731.Mon, Mar 16, 2:05 PM

Fix memory leak

cblack updated this revision to Diff 77733.Mon, Mar 16, 2:16 PM

Replace 801 with something more semantically meaningful

gikari requested changes to this revision.Mon, Mar 16, 2:18 PM
gikari added a subscriber: gikari.
gikari added inline comments.
color-reload-module/CMakeLists.txt
2

Would you mind setting this at the parent CMakeLists.txt?

3

Would you mind making this as a subproject?

11

Also, could you remove spaces around parentaces to match the CMake style with the one, that is in kded and kconf_update directories?

15

Isn't the PUBLIC/PRIVATE modifiers a good modern CMake practice? The same for target_include_directories

This revision now requires changes to proceed.Mon, Mar 16, 2:18 PM
cblack updated this revision to Diff 77739.Mon, Mar 16, 2:47 PM
cblack marked 4 inline comments as done.

Address feedback

gikari added inline comments.Mon, Mar 16, 3:01 PM
color-reload-module/CMakeLists.txt
10
add_library(colorreload-gtk-module SHARED
    reloader.c
)
15

Sorry, this is a nitpick, but could you please make this look like this:

target_link_libraries(colorreload-gtk-module
  PRIVATE
    ${GLIB_LIBRARIES}
    ${GTK_LIBRARIES}
gikari added inline comments.Mon, Mar 16, 3:05 PM
color-reload-module/CMakeLists.txt
2

Why is this needed?

4

Move to parent CMakeLists.txt

5

Same

7

Remove space before ( and I guess this could also go to parent CMakeLists.txt?

cblack added inline comments.Mon, Mar 16, 3:09 PM
color-reload-module/CMakeLists.txt
3

That's a Meson thing, not a CMake thing. CMake just has add_subdirectory, afaik.

cblack updated this revision to Diff 77740.Mon, Mar 16, 3:10 PM
cblack marked 7 inline comments as done.

Address feedback

gikari accepted this revision.Mon, Mar 16, 3:28 PM

Nice! Does it need to be manually passed to GTK apps when testing D28072?

This revision is now accepted and ready to land.Mon, Mar 16, 3:28 PM
gikari added inline comments.Mon, Mar 16, 3:30 PM
CMakeLists.txt
2–3

Extra space

Nice! Does it need to be manually passed to GTK apps when testing D28072?

GTK needs to be configured to load the GTK module colorreload-gtk-module.

cblack updated this revision to Diff 77764.Mon, Mar 16, 6:24 PM

Minor formatting improvements

GTK needs to be configured to load the GTK module colorreload-gtk-module.

You mean we need to add gtk-modules=colorreload-gtk-module to settings.ini? And how to add this module to autoload in Wayland?

GTK needs to be configured to load the GTK module colorreload-gtk-module.

You mean we need to add gtk-modules=colorreload-gtk-module to settings.ini? And how to add this module to autoload in Wayland?

Expose modules on the Modules property of org.gtk.Settings via DBUS.

This revision was automatically updated to reflect the committed changes.

Seems like I've made a mistake. This does not build.

/home/gikari/kde/src/kde-gtk-config/color-reload-module/reloader.c:21:10: fatal error: gtk/gtk.h: No such file or directory
   21 | #include <gtk/gtk.h>
      |          ^~

Seems like I've made a mistake. This does not build.

/home/gikari/kde/src/kde-gtk-config/color-reload-module/reloader.c:21:10: fatal error: gtk/gtk.h: No such file or directory
   21 | #include <gtk/gtk.h>
      |          ^~

Builds for me.

I tried to change these ones to GTK3_LIBRARY and GLIB2_LIBRARY, but now it throws this:

/home/gikari/kde/src/kde-gtk-config/color-reload-module/reloader.c:37: undefined reference to `g_type_check_instance_cast'
color-reload-module/CMakeLists.txt
11

It seems like you use other variables, than in other parts of kde-gtk-config.
In other places it uses GTK3_LIBRARY and GLIB2_LIBRARY variables from find module. The versions also appear in target_include_directories command.

I tried to change these ones to GTK3_LIBRARY and GLIB2_LIBRARY, but now it throws this:

/home/gikari/kde/src/kde-gtk-config/color-reload-module/reloader.c:37: undefined reference to `g_type_check_instance_cast'

Looks like my cached version was behaving properly, while a clean build wasn't. Investigating.

This change, and changes subsequent to it have broken the build of kde-gtk-config.

Please see https://build.kde.org/view/Failing/job/Plasma/job/kde-gtk-config/job/kf5-qt5%20SUSEQt5.12/ and https://build.kde.org/view/Failing/job/Plasma/job/kde-gtk-config/job/kf5-qt5%20FreeBSDQt5.13/

Plasma developers - this is one of several breakages which have occurred over the last couple of days which you have failed to address in a timely fashion.
Should this behaviour continue then CI coverage for Plasma will be discontinued and Plasma repositories will be blacklisted from the CI system and be unable to be used by other KDE software projects.

cblack reopened this revision.Tue, Mar 17, 6:35 PM
This revision is now accepted and ready to land.Tue, Mar 17, 6:35 PM
cblack updated this revision to Diff 77857.Tue, Mar 17, 6:35 PM

Rebase on master

cblack updated this revision to Diff 77858.Tue, Mar 17, 6:38 PM

Update CMakeLists.txt

cblack updated this revision to Diff 77862.Tue, Mar 17, 7:22 PM

Fix bad symbol management

gikari added inline comments.Tue, Mar 17, 9:57 PM
color-reload-module/CMakeLists.txt
4

Why is this needed? Aren't the CMake default languages in project (parent CMakeLists) both CXX and C?

color-reload-module/reloader.c
36

This is hardcoded directory. The $XDG_CONFIG_HOME may be set to something other, not only the default.

cblack updated this revision to Diff 77871.Tue, Mar 17, 11:50 PM

Address feedback

gikari added inline comments.Fri, Mar 27, 5:11 PM
color-reload-module/CMakeLists.txt
2

May be this could be moved to parent CMakeLists with other includes?

cblack updated this revision to Diff 78804.Sun, Mar 29, 4:52 PM

Use KDE_INSTALL_LIBDIR