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

[Mikhail Zolotukhin]

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, nicolasfella, broulik, GB_2, abetts, ngraham, 
plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, 
apol, ahiemstra, mart

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

[Mikhail Zolotukhin]

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, nicolasfella, broulik, GB_2, abetts, ngraham, 
plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, 
apol, ahiemstra, mart

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

[Nathaniel Graham]

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, davidedmundson, nicolasfella, broulik, GB_2, abetts, ngraham, 
plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, 
apol, ahiemstra, mart

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

[Christoph Feck]

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, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, 
apol, ahiemstra, mart

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

[Kai Uwe Broulik]

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, GB_2, abetts, ngraham, 
plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, 
apol, ahiemstra, mart

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

[Mikhail Zolotukhin]

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

AFFECTED FILES
  CMakeLists.txt
  cmake/modules/FindGSettingSchemas.cmake
  cmake/modules/FindGTK2.cmake
  cmake/modules/FindGTK3.cmake
  gtkconfig/CMakeLists.txt
  gtkconfig/gtk2apps_reloader/CMakeLists.txt
  gtkconfig/gtk2apps_reloader/gtk2reloader.c
  gtkconfig/kded/CMakeLists.txt
  gtkconfig/kded/configeditor.cpp
  gtkconfig/kded/configeditor.h
  gtkconfig/kded/gtkconfig.cpp
  gtkconfig/kded/gtkconfig.h
  gtkconfig/kded/gtkconfig.json

To: gikari, #plasma, apol, davidedmundson
Cc: davidedmundson, nicolasfella, broulik, GB_2, abetts, ngraham, plasma-devel, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, 
ahiemstra, mart

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

[Mikhail Zolotukhin]

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
  https://phabricator.kde.org/D21860

AFFECTED FILES
  CMakeLists.txt
  cmake/modules/FindGSettingSchemas.cmake
  cmake/modules/FindGTK2.cmake
  cmake/modules/FindGTK3.cmake
  gtkconfig/CMakeLists.txt
  gtkconfig/gtk2apps_reloader/CMakeLists.txt
  gtkconfig/gtk2apps_reloader/gtk2reloader.c
  gtkconfig/kded/CMakeLists.txt
  gtkconfig/kded/configeditor.cpp
  gtkconfig/kded/configeditor.h
  gtkconfig/kded/gtkconfig.cpp
  gtkconfig/kded/gtkconfig.h
  gtkconfig/kded/gtkconfig.json
  kcms/fonts/fonts.cpp

To: gikari, #plasma, apol, davidedmundson
Cc: davidedmundson, nicolasfella, broulik, GB_2, abetts, ngraham, plasma-devel, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, 
ahiemstra, mart

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

[Mikhail Zolotukhin]

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
  https://phabricator.kde.org/D21860

AFFECTED FILES
  CMakeLists.txt
  cmake/modules/FindGSettingSchemas.cmake
  cmake/modules/FindGTK2.cmake
  cmake/modules/FindGTK3.cmake
  gtkconfig/CMakeLists.txt
  gtkconfig/gtk2apps_reloader/CMakeLists.txt
  gtkconfig/gtk2apps_reloader/gtk2reloader.c
  gtkconfig/kded/CMakeLists.txt
  gtkconfig/kded/configeditor.cpp
  gtkconfig/kded/configeditor.h
  gtkconfig/kded/gtkconfig.cpp
  gtkconfig/kded/gtkconfig.h
  gtkconfig/kded/gtkconfig.json
  kcms/fonts/fonts.cpp

To: gikari, #plasma, apol, davidedmundson
Cc: davidedmundson, nicolasfella, broulik, GB_2, abetts, ngraham, plasma-devel, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, 
ahiemstra, mart

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

[Mikhail Zolotukhin]

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

To: gikari, #plasma, apol, davidedmundson
Cc: davidedmundson, nicolasfella, broulik, GB_2, abetts, ngraham, plasma-devel, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, 
ahiemstra, mart

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

[Mikhail Zolotukhin]

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 curly brackets and equal 
signs, but I don't see any mention that brace initialization should be avoided 
in the coding style. May be I'm inconsiderate.

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, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, 
ahiemstra, mart

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

[Mikhail Zolotukhin]

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
  https://phabricator.kde.org/D21860

AFFECTED FILES
  CMakeLists.txt
  cmake/modules/FindGSettingSchemas.cmake
  cmake/modules/FindGTK2.cmake
  cmake/modules/FindGTK3.cmake
  gtkconfig/CMakeLists.txt
  gtkconfig/gtk2apps_reloader/CMakeLists.txt
  gtkconfig/gtk2apps_reloader/gtk2reloader.c
  gtkconfig/kded/CMakeLists.txt
  gtkconfig/kded/configeditor.cpp
  gtkconfig/kded/configeditor.h
  gtkconfig/kded/gtkconfig.cpp
  gtkconfig/kded/gtkconfig.h
  gtkconfig/kded/gtkconfig.json
  kcms/fonts/fonts.cpp

To: gikari, #plasma, apol, davidedmundson
Cc: davidedmundson, nicolasfella, broulik, GB_2, abetts, ngraham, plasma-devel, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, 
ahiemstra, mart

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

[Kai Uwe Broulik]

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 
https://techbase.kde.org/Policies/Frameworks_Coding_Style

> gtkconfig.cpp:36
> +{
> +QGuiApplication *qapp = static_cast *>(QGuiApplication::instance());
> +connect(qapp, ::fontChanged, this, ::setFont);

The cast is pointless

> gtkconfig.cpp:38
> +connect(qapp, ::fontChanged, this, ::setFont);
> +qDebug() << "GTK configuration module loaded";
> +}

Remove, or use categorized logging

> gtkconfig.cpp:43
> +{
> +QString configFontName {getConfigFontName(font)};
> +configEditor->setGtk2ConfigValue("gtk-font-name", configFontName);

const

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, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, 
ahiemstra, mart

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

[Mikhail Zolotukhin]

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
  https://phabricator.kde.org/D21860?vs=64619=66597

BRANCH
  arcpatch-D21860

REVISION DETAIL
  https://phabricator.kde.org/D21860

AFFECTED FILES
  CMakeLists.txt
  cmake/modules/FindGSettingSchemas.cmake
  cmake/modules/FindGTK2.cmake
  cmake/modules/FindGTK3.cmake
  gtkconfig/CMakeLists.txt
  gtkconfig/gtk2apps_reloader/CMakeLists.txt
  gtkconfig/gtk2apps_reloader/gtk2reloader.c
  gtkconfig/kded/CMakeLists.txt
  gtkconfig/kded/configeditor.cpp
  gtkconfig/kded/configeditor.h
  gtkconfig/kded/gtkconfig.cpp
  gtkconfig/kded/gtkconfig.h
  gtkconfig/kded/gtkconfig.json
  kcms/fonts/fonts.cpp

To: gikari, #plasma, apol, davidedmundson
Cc: davidedmundson, nicolasfella, broulik, GB_2, abetts, ngraham, plasma-devel, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Mikhail Zolotukhin]

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 kcm 
and replace it with that signal binding? For fonts it sure will work, but what 
about other settings, that will be in the future, like color schemes, icons 
etc? Whose signals we will receive?

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, ragreen, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Kai Uwe Broulik]

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;
>   QGuiApplication *qapp = static_cast(parent);
>   qDebug() << "Qapp: " << qapp;
>   connect(qapp, ::fontChanged, this, [this](const QFont 
> ) {
>   qDebug() << "Font recieved";
>   qDebug() << font;
>   });
>   qDebug() << "GTK configuration module loaded";
>   }
> 
> And when I save fonts via KCM nothing (Except the "parent", "qapp" and "GTK 
> ..." lines) was printed in konsole, where I run Kded5.  So, it means signal 
> was not send?

Are you sure `parent` is a `QGuiApplication`? I don't think so. Perhaps you 
want to connect to `QGuiApplication::instance()` instead.

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, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Björn Feber]

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, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Mikhail Zolotukhin]

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 explicitly calls (through whatever means) 
> which then syncs things.
> 
> Both have an advantage, and a disadvantage.
> 
> This is both  running all the time *and* is being explicitly called,  which 
> is the worst of both worlds.

@davidedmundson So, what are advantages and disadvantages of each of those 
options and what way do you recommend me go?

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, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Mikhail Zolotukhin]

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 ) 
> {
>   // now check if you have the new fonts
>   });
> 
> I can't guarantee this will work (accessing `font` from there will actually 
> deadlock :D but I am curious whether that signal is emitted properly, so you 
> dont have to listen to or ask the font kcm to tell you.

So, I wrote this:

  GtkConfig::GtkConfig(QObject *parent, const QVariantList&) :
  KDEDModule(parent), configEditor {new ConfigEditor()}
  {
  qDebug() << "Parent: " << parent;
  QGuiApplication *qapp = static_cast(parent);
  qDebug() << "Qapp: " << qapp;
  connect(qapp, ::fontChanged, this, [this](const QFont 
) {
  qDebug() << "Font recieved";
  qDebug() << font;
  });
  qDebug() << "GTK configuration module loaded";
  }

And when I save fonts via KCM nothing (Except the "parent", "qapp" and "GTK 
..." lines) was printed in konsole, where I run Kded5.  So, it means signal was 
not send?

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, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Kai Uwe Broulik]

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 module, so I don't have 
> to ask from KCM to change a font via DBus?

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 ) {
  // now check if you have the new fonts
  });

I can't guarantee this will work (accessing `font` from there will actually 
deadlock :D but I am curious whether that signal is emitted properly, so you 
dont have to listen to or ask the font kcm to tell you.

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, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Mikhail Zolotukhin]

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 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 module, so I don't have to 
ask from KCM to change a font via DBus?

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, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[David Edmundson]

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 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 explicitly calls (through whatever means) 
which then syncs things.

Both have an advantage, and a disadvantage.

This is both  running all the time *and* is being explicitly called,  which is 
the worst of both worlds.

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, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Kai Uwe Broulik]

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 `QGuiApplication::fontChanged` emitted when you change font settings?

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, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Mikhail Zolotukhin]

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
  https://phabricator.kde.org/D21860?vs=64517=64619

BRANCH
  set-gtk-font-in-fonts-kcm (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D21860

AFFECTED FILES
  CMakeLists.txt
  cmake/modules/FindGSettingSchemas.cmake
  cmake/modules/FindGTK2.cmake
  cmake/modules/FindGTK3.cmake
  gtkconfig/CMakeLists.txt
  gtkconfig/gtk2apps_reloader/CMakeLists.txt
  gtkconfig/gtk2apps_reloader/gtk2reloader.c
  gtkconfig/kded/CMakeLists.txt
  gtkconfig/kded/configeditor.cpp
  gtkconfig/kded/configeditor.h
  gtkconfig/kded/gtkconfig.cpp
  gtkconfig/kded/gtkconfig.h
  gtkconfig/kded/gtkconfig.json
  kcms/fonts/fonts.cpp
  kcms/fonts/fonts.h

To: gikari, #plasma, apol, davidedmundson
Cc: davidedmundson, nicolasfella, broulik, GB_2, abetts, ngraham, plasma-devel, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Mikhail Zolotukhin]

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
  https://phabricator.kde.org/D21860?vs=64342=64517

BRANCH
  set-gtk-font-in-fonts-kcm (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D21860

AFFECTED FILES
  CMakeLists.txt
  cmake/modules/FindGSettingSchemas.cmake
  cmake/modules/FindGTK2.cmake
  cmake/modules/FindGTK3.cmake
  gtkconfig/CMakeLists.txt
  gtkconfig/gtk2apps_reloader/CMakeLists.txt
  gtkconfig/gtk2apps_reloader/gtk2reloader.c
  gtkconfig/kded/CMakeLists.txt
  gtkconfig/kded/configeditor.cpp
  gtkconfig/kded/configeditor.h
  gtkconfig/kded/gtkconfig.cpp
  gtkconfig/kded/gtkconfig.h
  gtkconfig/kded/gtkconfig.json
  kcms/fonts/fonts.cpp
  kcms/fonts/fonts.h

To: gikari, #plasma, apol, davidedmundson
Cc: davidedmundson, nicolasfella, broulik, GB_2, abetts, ngraham, plasma-devel, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Mikhail Zolotukhin]

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. 

  
  Now I'm interested where to put already written daemon: to //plasma-desktop// 
or make it part of //kde-gtk-config// (aka Gtk KCM). If the first, should I 
create a separate patch or make it as a part of this one?
  
  P.S. I'm terribly inconsiderate at merge conflicts. Sorry for that wall of 
changes from above.

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, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Mikhail Zolotukhin]

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 DETAIL
  https://phabricator.kde.org/D21860

AFFECTED FILES
  kcms/fonts/fonts.cpp
  kcms/fonts/fonts.h

To: gikari, #plasma, apol, davidedmundson
Cc: davidedmundson, nicolasfella, broulik, GB_2, abetts, ngraham, plasma-devel, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Mikhail Zolotukhin]

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 (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D21860

AFFECTED FILES
  kcms/fonts/fonts.cpp
  kcms/fonts/fonts.h

To: gikari, #plasma, apol, davidedmundson
Cc: davidedmundson, nicolasfella, broulik, GB_2, abetts, ngraham, plasma-devel, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Mikhail Zolotukhin]

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 master)

REVISION DETAIL
  https://phabricator.kde.org/D21860

AFFECTED FILES
  kcms/fonts/fonts.cpp
  kcms/fonts/fonts.h

To: gikari, #plasma, apol, davidedmundson
Cc: davidedmundson, nicolasfella, broulik, GB_2, abetts, ngraham, plasma-devel, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Mikhail Zolotukhin]

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 (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D21860

AFFECTED FILES
  kcms/fonts/fonts.cpp
  kcms/fonts/fonts.h

To: gikari, #plasma, apol, davidedmundson
Cc: davidedmundson, nicolasfella, broulik, GB_2, abetts, ngraham, plasma-devel, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Mikhail Zolotukhin]

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)

REVISION DETAIL
  https://phabricator.kde.org/D21860

AFFECTED FILES
  kcms/fonts/CMakeLists.txt
  kcms/fonts/fonts.cpp
  kcms/fonts/fonts.h

To: gikari, #plasma, apol, davidedmundson
Cc: davidedmundson, nicolasfella, broulik, GB_2, abetts, ngraham, plasma-devel, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Nathaniel Graham]

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: gikari, #plasma, apol, davidedmundson
Cc: davidedmundson, nicolasfella, broulik, GB_2, abetts, ngraham, plasma-devel, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Mikhail Zolotukhin]

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 other syncing.
  
  
  @ngraham suggested that in VDG room (Matrix Message Link 
)

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, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[David Edmundson]

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
  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, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[David Edmundson]

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 the patch to either 
> plasma-desktop, plasma-workspace or kde-gtk-config repos.
> 
> If I understand correctly, this is not a correct way to import dbus 
> interface, but unfortunately I don't know how to do it properly. I tried what 
> I learned from the wiki tutorial about dbus (which is rather old by the way). 
> This is CMake code from the repo above to generate xml for development.
> 
>   set(DBUS_PREFIX "~/kde/usr/share/dbus-1/interfaces")
>   
>   set(SOURCES "kdegtkconfigd.cpp" 
> ${CMAKE_CURRENT_BINARY_DIR}/org.kde.gtkConfigd.xml)
>   set(HEADERS "kdegtkconfigd.h")
>   
>   qt5_generate_dbus_interface(kdegtkconfigd.h org.kde.gtkConfigd.xml OPTIONS 
> -A)
>   
>   qt5_add_dbus_adaptor(SOURCES org.kde.gtkConfigd.xml
>   kdegtkconfigd.h KdeGtkConfigd)
>   
>   install(
>   FILES ${CMAKE_CURRENT_BINARY_DIR}/org.kde.gtkConfigd.xml
>   DESTINATION ${DBUS_PREFIX}
>   )

If you put that daemon in plasma-desktop, then you can just do
qt5_add_dbus_interface("../somePath.xml")  here and my comment becomes a 
non-issue.

If it ends up in gtk-config for separation, then we should continue that 
separation and just include the .xml file in two places.

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, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Mikhail Zolotukhin]

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 are in different prefixes.
> 
> Where does org.kde.gtkConfigd.xml come from?

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 the patch to either plasma-desktop, 
plasma-workspace or kde-gtk-config repos.

If I understand correctly, this is not a correct way to import dbus interface, 
but unfortunately I don't know how to do it properly. I tried what I learned 
from the wiki tutorial about dbus (which is rather old by the way). This is 
CMake code from the repo above to generate xml for development.

  set(DBUS_PREFIX "~/kde/usr/share/dbus-1/interfaces")
  
  set(SOURCES "kdegtkconfigd.cpp" 
${CMAKE_CURRENT_BINARY_DIR}/org.kde.gtkConfigd.xml)
  set(HEADERS "kdegtkconfigd.h")
  
  qt5_generate_dbus_interface(kdegtkconfigd.h org.kde.gtkConfigd.xml OPTIONS -A)
  
  qt5_add_dbus_adaptor(SOURCES org.kde.gtkConfigd.xml
  kdegtkconfigd.h KdeGtkConfigd)
  
  install(
  FILES ${CMAKE_CURRENT_BINARY_DIR}/org.kde.gtkConfigd.xml
  DESTINATION ${DBUS_PREFIX}
  )

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, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[David Edmundson]

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 ${KINIT_DBUS_INTERFACES_DIR}/org.kde.gtkConfigd.xml)
>  qt5_add_dbus_interface(kcm_fonts_PART_SRCS ${klauncher_xml} klauncher_iface)

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 are in different prefixes.

Where does org.kde.gtkConfigd.xml come from?

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, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Nicolas Fella]

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 = m_generalFont.family() + " "
> ++ m_generalFont.styleName() + " "

I think the specific string should be determined by the gtkconfig daemon

> fonts.cpp:621
> ++ QString::number(m_generalFont.pointSize());
> +gtkDaemonInterface.changeGtkFont(fontAsGtkConfigString);
> +}

We know it will be about GTK, so changeFont should be enough.

Also, setFont would be more consistent with other API naming

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, 
The-Feren-OS-Dev, jraleigh, fbampaloukas, ragreen, Pitel, ZrenBot, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Mikhail Zolotukhin]

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)

REVISION DETAIL
  https://phabricator.kde.org/D21860

AFFECTED FILES
  kcms/fonts/CMakeLists.txt
  kcms/fonts/fonts.cpp
  kcms/fonts/fonts.h

To: gikari, #plasma, apol
Cc: nicolasfella, broulik, GB_2, abetts, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, fbampaloukas, ragreen, Pitel, ZrenBot, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Björn Feber]

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, 
ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Mikhail Zolotukhin]

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 reading on its own.
  
  
  I have found those lines in colorscheme config (and the similar in icon 
settings, but nothing in widget style):
  
QDBusMessage message = 
QDBusMessage::createSignal(QStringLiteral("/KGlobalSettings"),
  
QStringLiteral("org.kde.KGlobalSettings"),
  
QStringLiteral("notifyChange"));
message.setArguments({
0, //previous KGlobalSettings::PaletteChanged. This is now private API 
in khintsettings
0  //unused in palette changed but needed for the DBus signature
});
QDBusConnection::sessionBus().send(message);
  
  Is this it? Should my daemon use the similar DBus signatures for fonts?
  
  If I understand correctly, what I need to do is:
  
  1. Write similar to above lines in fonts kcm code (No sure about arguments 
actually - what they should be?)
  2. Write a daemon that listens DBus with those signatures and when specific 
setting changes (for now it's only fonts, then it will be cursors or something 
else) it changes corresponding setting in gtk via GSettings

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, 
ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Nicolas Fella]

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, ngraham, plasma-devel, LeGast00n, 
jraleigh, fbampaloukas, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Kai Uwe Broulik]

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, should be fine.
  
  > Some sort of a gtk-settings-only file-watcher, that monitors Plasma config 
files with relevant info like fonts/cursors/etc and that changes gtk configs 
accordingly, whenever Plasma files changes?
  
  Pretty much. 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 reading on its own.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D21860

To: gikari, #plasma, apol
Cc: broulik, GB_2, abetts, ngraham, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

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

[Mikhail Zolotukhin]

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 daemon 
that monitors all settings and applies them in a central place, so the settings 
knobs could be where they belong.
  
  
  You're right, for some reason settings.ini does not work in Wayland. However 
gsettings does, but dependency on gtk API disturbs me.
  As for the daemon: what should it look like? Some sort of a gtk-settings-only 
file-watcher, that monitors Plasma config files with relevant info like 
fonts/cursors/etc and that changes gtk configs accordingly, whenever Plasma 
files changes?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D21860

To: gikari, #plasma, apol
Cc: broulik, GB_2, abetts, ngraham, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

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

[Kai Uwe Broulik]

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.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D21860

To: gikari, #plasma, apol
Cc: broulik, GB_2, abetts, ngraham, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

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

[Björn Feber]

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, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

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

[Nathaniel Graham]

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, ali-mohamed, jensreuterberg, 
sebas, apol, mart

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

[Andres Betts]

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, ali-mohamed, jensreuterberg, 
sebas, apol, mart

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

[Nathaniel Graham]

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 use the "General" 
font size to GTK apps.
  
  > Note: if fonts configuration option will be removed from kde-gtk-config, 
this patch will be needed (D21524 ), so 
gtk2rc file won't be overridden without font string.
  
  It will be, so yes, it looks like we'll need that. I'll see if I can rustle 
up some reviewers!

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D21860

To: gikari, #vdg, #plasma
Cc: ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

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

[Mikhail Zolotukhin]

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 separate gtk kcm.
  
  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?
  
  Note: if fonts configuration option will be removed from kde-gtk-config, this 
patch will be needed (D21524 ), so gtk2rc 
file won't be overridden without font string.
  
  Depends on D21524 
  
  BUG: 138866

TEST PLAN
  Check if fonts in gtk2 and gtk3 apps are set correctly, after changing 
General Font.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  set-gtk-font-in-fonts-kcm (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D21860

AFFECTED FILES
  kcms/fonts/fonts.cpp
  kcms/fonts/fonts.h

To: gikari, #vdg
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart