D21860: Set a font for gtk applications in the fonts kcm

gikari abandoned this revision. gikari added a comment. Abandoned in favor of D24743 REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D21860 To: gikari, #plasma, apol, davidedmundson, cfeck Cc: cfeck, davidedmundson,

D21860: Set a font for gtk applications in the fonts kcm

gikari added a comment. So, do I need to replace `find_package(GTK3 REQUIRED)` to `find_package(GTK3)` and others or not? REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D21860 To: gikari, #plasma, apol, davidedmundson, cfeck Cc: cfeck, davidedmundson,

D21860: Set a font for gtk applications in the fonts kcm

ngraham added a comment. The idea is to move all the functionality in kde-gtk-config here, then delete it. See the parent Phab task for details. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D21860 To: gikari, #plasma, apol, davidedmundson, cfeck Cc: cfeck,

D21860: Set a font for gtk applications in the fonts kcm

cfeck added a comment. Please make any GTK stuff optional, not required. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D21860 To: gikari, #plasma, apol, davidedmundson Cc: cfeck, davidedmundson, nicolasfella, broulik, GB_2, abetts, ngraham, plasma-devel,

D21860: Set a font for gtk applications in the fonts kcm

broulik added a comment. It wouldn't need to be optional if it was in kde-gtk-config, not plasma-desktop REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D21860 To: gikari, #plasma, apol, davidedmundson, cfeck Cc: cfeck, davidedmundson, nicolasfella, broulik,

D21860: Set a font for gtk applications in the fonts kcm

gikari updated this revision to Diff 67693. gikari added a comment. - Remove unnecessary include REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21860?vs=67679=67693 BRANCH arcpatch-D21860 REVISION DETAIL https://phabricator.kde.org/D21860

D21860: Set a font for gtk applications in the fonts kcm

gikari updated this revision to Diff 67679. gikari added a comment. Remove debug messages and add QStringLiteral around const strings REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21860?vs=67657=67679 BRANCH arcpatch-D21860 REVISION DETAIL

D21860: Set a font for gtk applications in the fonts kcm

gikari updated this revision to Diff 67657. gikari added a comment. Replace brace initialization with brackets, use qGuiApp macros REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21860?vs=67643=67657 BRANCH arcpatch-D21860 REVISION DETAIL

D21860: Set a font for gtk applications in the fonts kcm

gikari added inline comments. INLINE COMMENTS > broulik wrote in gtkconfig.cpp:36 > The cast is pointless Why? `QGuiApplication::instance()` returns `QCoreApplication` pointer, not `QGuiApplication` one. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D21860

D21860: Set a font for gtk applications in the fonts kcm

gikari marked 2 inline comments as done. gikari added inline comments. INLINE COMMENTS > broulik wrote in gtkconfig.cpp:34 > We typically don't use braces for initializations, see > https://techbase.kde.org/Policies/Frameworks_Coding_Style Sure, I will change that and other initializations to

D21860: Set a font for gtk applications in the fonts kcm

gikari updated this revision to Diff 67643. gikari added a comment. - Set an icon theme for gtk applications in the icons kcm REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21860?vs=66597=67643 BRANCH arcpatch-D21860 REVISION DETAIL

D21860: Set a font for gtk applications in the fonts kcm

broulik added inline comments. INLINE COMMENTS > gtkconfig.cpp:34 > +GtkConfig::GtkConfig(QObject *parent, const QVariantList&) : > +KDEDModule(parent), configEditor {new ConfigEditor()} > +{ We typically don't use braces for initializations, see

D21860: Set a font for gtk applications in the fonts kcm

gikari updated this revision to Diff 66597. gikari added a comment. Make the daemon independent Remove explicit call from the fonts kcm and make the daemon update font configuration automatically REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE

D21860: Set a font for gtk applications in the fonts kcm

gikari added inline comments. INLINE COMMENTS > broulik wrote in fonts.cpp:607 > Are you sure `parent` is a `QGuiApplication`? I don't think so. Perhaps you > want to connect to `QGuiApplication::instance()` instead. Just tested. It works. So the plan is to remove explicit call from the font

D21860: Set a font for gtk applications in the fonts kcm

broulik added inline comments. INLINE COMMENTS > gikari wrote in fonts.cpp:607 > So, I wrote this: > > GtkConfig::GtkConfig(QObject *parent, const QVariantList&) : > KDEDModule(parent), configEditor {new ConfigEditor()} > { > qDebug() << "Parent: " << parent; >

D21860: Set a font for gtk applications in the fonts kcm

GB_2 added a comment. Ping. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D21860 To: gikari, #plasma, apol, davidedmundson Cc: davidedmundson, nicolasfella, broulik, GB_2, abetts, ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas,

D21860: Set a font for gtk applications in the fonts kcm

gikari added inline comments. INLINE COMMENTS > davidedmundson wrote in fonts.cpp:607 > There are two designs which are both sensible: > > 1. we have a daemon that runs all the time so that it can listen for changes. > When that happens it syncs things > > 2. something that the fonts module

D21860: Set a font for gtk applications in the fonts kcm

gikari added inline comments. INLINE COMMENTS > broulik wrote in fonts.cpp:607 > kded (the application you're running in) is a `QApplication`, so from the > constructor of your kdedmodule you could do > > #include > connect(qApp, ::fontChanged, this, [this](this QFont ) > { > //

D21860: Set a font for gtk applications in the fonts kcm

broulik added inline comments. INLINE COMMENTS > gikari wrote in fonts.cpp:607 > @broulik I do not understand. `KDEDModule` (which is a parent of `GtkConfig`) > is `QObject`, not a `QGuiApplication`. Maybe I am missing something? You want > me to create a connection with that signal within a

D21860: Set a font for gtk applications in the fonts kcm

gikari added inline comments. INLINE COMMENTS > broulik wrote in fonts.cpp:607 > Given this is a kded module and as such a regular qt application, you should > be able to get all of this automatically. > Is the `QGuiApplication::fontChanged` emitted when you change font settings? @broulik I do

D21860: Set a font for gtk applications in the fonts kcm

davidedmundson added inline comments. INLINE COMMENTS > broulik wrote in fonts.cpp:607 > Given this is a kded module and as such a regular qt application, you should > be able to get all of this automatically. > Is the `QGuiApplication::fontChanged` emitted when you change font settings? There

D21860: Set a font for gtk applications in the fonts kcm

broulik added inline comments. INLINE COMMENTS > fonts.cpp:607 > +}; > +kdedInterface.call("setFont", m_generalFont.toString()); > +} Given this is a kded module and as such a regular qt application, you should be able to get all of this automatically. Is the

D21860: Set a font for gtk applications in the fonts kcm

gikari updated this revision to Diff 64619. gikari edited the test plan for this revision. gikari added a comment. Fix harfbuzz include directories when Pango verison is 1.44 or greater REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE

D21860: Set a font for gtk applications in the fonts kcm

gikari updated this revision to Diff 64517. gikari added a comment. Create a Gtk Config kded module To actually apply settings to gtk applications DBus call should have a reciever. This creates it as kded module. REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE

D21860: Set a font for gtk applications in the fonts kcm

gikari added a comment. Since awesome @nicolasfella committed his changes to Gtk Daemon, I have modified the interface according to it. No more wacky xml stuff, because the daemon became a kded module with the adequate dbus signatures.

D21860: Set a font for gtk applications in the fonts kcm

gikari updated this revision to Diff 64342. gikari added a comment. - Remove QStandardPaths as redundant include REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21860?vs=64339=64342 BRANCH set-gtk-font-in-fonts-kcm (branched from master) REVISION

D21860: Set a font for gtk applications in the fonts kcm

gikari updated this revision to Diff 64339. gikari marked 2 inline comments as done. gikari added a comment. - Remove redundant includes REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21860?vs=64331=64339 BRANCH set-gtk-font-in-fonts-kcm

D21860: Set a font for gtk applications in the fonts kcm

gikari updated this revision to Diff 64331. gikari added a comment. - Remove redundant saveFontToGtkSettings method declaration REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21860?vs=64330=64331 BRANCH set-gtk-font-in-fonts-kcm (branched from

D21860: Set a font for gtk applications in the fonts kcm

gikari updated this revision to Diff 64330. gikari marked an inline comment as done. gikari added a comment. - Remove redundant xml stuff REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21860?vs=64329=64330 BRANCH set-gtk-font-in-fonts-kcm

D21860: Set a font for gtk applications in the fonts kcm

gikari updated this revision to Diff 64329. gikari added a comment. - Delegate gtk fonts configuration to Gtk Daemon REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21860?vs=64002=64329 BRANCH set-gtk-font-in-fonts-kcm (branched from master)

D21860: Set a font for gtk applications in the fonts kcm

ngraham added a comment. Please keep in mind that @davidedmundson is 1000 times more technically skilled than I so if our advice is ever in conflict, you should listen to him rather than me. :) REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D21860 To:

D21860: Set a font for gtk applications in the fonts kcm

gikari added a comment. In D21860#514564 , @davidedmundson wrote: > > But it was recomended to me to make the daemon as the patch > > Discussed where? > > I'm curious of the long term plans and how this fits in with krdb which does most

D21860: Set a font for gtk applications in the fonts kcm

davidedmundson added a comment. > But it was recomended to me to make the daemon as the patch Discussed where? I'm curious of the long term plans and how this fits in with krdb which does most other syncing. REPOSITORY R119 Plasma Desktop REVISION DETAIL

D21860: Set a font for gtk applications in the fonts kcm

davidedmundson added inline comments. INLINE COMMENTS > gikari wrote in CMakeLists.txt:20 > Xml comes from gtk configuration daemon I'm currently working on, whose > source code is currently here . > But it was recomended to me to make the daemon as

D21860: Set a font for gtk applications in the fonts kcm

gikari added inline comments. INLINE COMMENTS > davidedmundson wrote in CMakeLists.txt:20 > We have this exporting of KINIT_DBUS_INTERFACES_DIR from the library rather > than just hardcoding /usr/share/dbus/interfaces/ so that packagers can do > whatever weird stuff packagers do where things

D21860: Set a font for gtk applications in the fonts kcm

davidedmundson requested changes to this revision. davidedmundson added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > CMakeLists.txt:20 > set(klauncher_xml ${KINIT_DBUS_INTERFACES_DIR}/kf5_org.kde.KLauncher.xml) > +set(kdegtkconfigd_xml

D21860: Set a font for gtk applications in the fonts kcm

nicolasfella added inline comments. INLINE COMMENTS > fonts.cpp:613 > +org::kde::gtkConfigd gtkDaemonInterface { > +"org.kde.gtkConfigd", > +"/KdeGtkConfigd", QStringLiteral() around strings > fonts.cpp:618 > +}; > +QString fontAsGtkConfigString =

D21860: Set a font for gtk applications in the fonts kcm

gikari updated this revision to Diff 64002. gikari added a comment. - Delegate gtk fonts configuration to Gtk Daemon REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21860?vs=59956=64002 BRANCH set-gtk-font-in-fonts-kcm (branched from master)

D21860: Set a font for gtk applications in the fonts kcm

GB_2 added a comment. Ping. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D21860 To: gikari, #plasma, apol Cc: nicolasfella, broulik, GB_2, abetts, ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai,

D21860: Set a font for gtk applications in the fonts kcm

gikari added a comment. In D21860#481370 , @broulik wrote: > Actually, for most appearance settings (color scheme, widget style, icon settings) it should already be signalled automatically via DBus through plasma-integration with no config

D21860: Set a font for gtk applications in the fonts kcm

nicolasfella added a comment. > but dependency on gtk API disturbs me. plasma-pa also depends on GSettings, so it's not unprecedented REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D21860 To: gikari, #plasma, apol Cc: nicolasfella, broulik, GB_2, abetts,

D21860: Set a font for gtk applications in the fonts kcm

broulik added a comment. In D21860#481346 , @gikari wrote: > However gsettings does, but dependency on gtk API disturbs me. I think that's only a problem if every KCM ends up depending on GTK. If it's a dedicated proxy service binary,

D21860: Set a font for gtk applications in the fonts kcm

gikari planned changes to this revision. gikari added a comment. In D21860#481258 , @broulik wrote: > Messing with GTK settings INI files from various places is not a good idea imho. Also, this approach doesn't work on Wayland. Ideally, we had a

D21860: Set a font for gtk applications in the fonts kcm

broulik added a comment. Messing with GTK settings INI files from various places is not a good idea imho. Also, this approach doesn't work on Wayland. Ideally, we had a daemon that monitors all settings and applies them in a central place, so the settings knobs could be where they belong.

D21860: Set a font for gtk applications in the fonts kcm

GB_2 added a comment. So when this lands I guess we need to remove the font option from the GTK KCM. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D21860 To: gikari, #plasma, apol Cc: GB_2, abetts, ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas,

D21860: Set a font for gtk applications in the fonts kcm

ngraham added a comment. There is no visual change. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D21860 To: gikari, #vdg, #plasma Cc: abetts, ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai,

D21860: Set a font for gtk applications in the fonts kcm

abetts added a comment. How does this look? REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D21860 To: gikari, #vdg, #plasma Cc: abetts, ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai,

D21860: Set a font for gtk applications in the fonts kcm

ngraham added a comment. Wow, thanks very much! To answer some questions: > Not sure if we should provide option to modify font only for gtk applications: why would anyone need to set different fonts for gnome and kde apps? Agreed. I don't think a separate control is needed. Just

D21860: Set a font for gtk applications in the fonts kcm

gikari created this revision. gikari added a reviewer: VDG. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. gikari requested review of this revision. REVISION SUMMARY To increase usability, fonts for gtk applications is now set in respective fonts kcm, instead of