D24743: Update GTK settings according to Plasma settings

eszlari added a comment. In D24743#562745 , @gikari wrote: > I'm not very proficient in CMake, so how can I do that? https://cmake.org/cmake/help/latest/command/find_program.html REPOSITORY R99 KDE Gtk Configuration Tool REVISION

D24743: Update GTK settings according to Plasma settings

gikari added a comment. In D24743#562656 , @zzag wrote: > Sweet! I wonder whether this KDED module can be used to synchronize the `gtk-decoration-layout` property with kwin's decoration button layout. Yes it probably will be used for

D24743: Update GTK settings according to Plasma settings

eszlari added a comment. Maybe a warning about the runtime dependency on xsettingsd should be added to cmake, so distro packagers know about it. REPOSITORY R99 KDE Gtk Configuration Tool REVISION DETAIL https://phabricator.kde.org/D24743 To: gikari, #plasma, #vdg, broulik, cblack Cc:

D24743: Update GTK settings according to Plasma settings

zzag added a comment. Sweet! I wonder whether this KDED module can be used to synchronize the `gtk-decoration-layout` property with kwin's decoration button layout. REPOSITORY R99 KDE Gtk Configuration Tool REVISION DETAIL https://phabricator.kde.org/D24743 To: gikari, #plasma, #vdg,

D24743: Update GTK settings according to Plasma settings

This revision was automatically updated to reflect the committed changes. Closed by commit R99:94732eb8d6ee: Update GTK settings according to Plasma settings (authored by gikari, committed by ngraham). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D24743?vs=69704=69766#toc REPOSITORY

D24743: Update GTK settings according to Plasma settings

broulik accepted this revision. broulik added a comment. This revision is now accepted and ready to land. Good job! REPOSITORY R99 KDE Gtk Configuration Tool BRANCH gtk-settings-in-respective-kcms REVISION DETAIL https://phabricator.kde.org/D24743 To: gikari, #plasma, #vdg, broulik,

D24743: Update GTK settings according to Plasma settings

gikari updated this revision to Diff 69704. gikari marked 5 inline comments as done. gikari added a comment. Remove unnecessary GTK lib usages REPOSITORY R99 KDE Gtk Configuration Tool CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24743?vs=69702=69704 BRANCH

D24743: Update GTK settings according to Plasma settings

cblack added inline comments. INLINE COMMENTS > gikari wrote in configeditor.cpp:51 > You mean remove `gtk_init`? Yes, my bad. REPOSITORY R99 KDE Gtk Configuration Tool REVISION DETAIL https://phabricator.kde.org/D24743 To: gikari, #plasma, #vdg, broulik, cblack Cc: ervin, ngraham,

D24743: Update GTK settings according to Plasma settings

gikari added inline comments. INLINE COMMENTS > cblack wrote in configeditor.cpp:51 > You should probably remove this call here, as you aren't using any GTK > functions. You mean remove `gtk_init`? REPOSITORY R99 KDE Gtk Configuration Tool REVISION DETAIL

D24743: Update GTK settings according to Plasma settings

cblack added inline comments. INLINE COMMENTS > CMakeLists.txt:29 > +${GLIB2_LIBRARY} > +${GTK3_LIBRARY} > +${GOBJECT2_LIBRARY} See below comments about GTK. > configeditor.cpp:35 > +#include > +#include > + You shouldn't need to import/use gtk for configuration, only gio which

D24743: Update GTK settings according to Plasma settings

gikari updated this revision to Diff 69702. gikari added a comment. Add DBusAddons component REPOSITORY R99 KDE Gtk Configuration Tool CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24743?vs=69691=69702 BRANCH gtk-settings-in-respective-kcms REVISION DETAIL

D24743: Update GTK settings according to Plasma settings

gikari updated this revision to Diff 69691. gikari added a comment. Remember to save all files before blindly commiting REPOSITORY R99 KDE Gtk Configuration Tool CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24743?vs=69690=69691 BRANCH gtk-settings-in-respective-kcms

D24743: Update GTK settings according to Plasma settings

gikari updated this revision to Diff 69690. gikari added a comment. Remove the empty line in CMakeLists.txt REPOSITORY R99 KDE Gtk Configuration Tool CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24743?vs=69689=69690 BRANCH gtk-settings-in-respective-kcms REVISION DETAIL

D24743: Update GTK settings according to Plasma settings

gikari updated this revision to Diff 69689. gikari marked 6 inline comments as done. gikari edited the summary of this revision. gikari added a comment. Fix the mistakes REPOSITORY R99 KDE Gtk Configuration Tool CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24743?vs=69259=69689

D24743: Update GTK settings according to Plasma settings

nicolasfella added a comment. Works really well, good job! Some minor nitpicks INLINE COMMENTS > CMakeLists.txt:69 > add_subdirectory(tests) > +add_subdirectory(kded-module) > Nitpick, feel free to ignore since it was probably done by me: usually we call the folder just 'kded' >

D24743: Update GTK settings according to Plasma settings

gikari updated this revision to Diff 69259. gikari marked an inline comment as done. gikari added a comment. Fix the typo REPOSITORY R99 KDE Gtk Configuration Tool CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24743?vs=69141=69259 BRANCH gtk-settings-in-respective-kcms

D24743: Update GTK settings according to Plasma settings

ervin added a comment. Just a minor thing, otherwise LGTM, I'll let Kai give the last stamp of approval. INLINE COMMENTS > gtkconfig.cpp:113 > +setFont(); > +setFont(); > +setIconTheme(KIconLoader::Group::Desktop); Are you sure you want to call setFont() twice here? That looks

D24743: Update GTK settings according to Plasma settings

gikari updated this revision to Diff 69141. gikari added a comment. - Transform ConfigEditor to namespace - Make `KSharedConfigPtr`s of kdeglobals and input configs the `ConfigValueProvider` members REPOSITORY R99 KDE Gtk Configuration Tool CHANGES SINCE LAST UPDATE

D24743: Update GTK settings according to Plasma settings

gikari updated this revision to Diff 69133. gikari marked 8 inline comments as done. gikari edited the summary of this revision. gikari added a comment. - Replace `()` initialization with `=` one - Embrace QStringLiteral madness even more - Rename some variables and method names

D24743: Update GTK settings according to Plasma settings

ngraham added inline comments. INLINE COMMENTS > gikari wrote in configeditor.cpp:49 > Where generally should I use equals sign (`=`), where parenthesis (`()`) and > where braces (`{}`) for initialization? I didn't find any note about that in > coding style

D24743: Update GTK settings according to Plasma settings

gikari added inline comments. INLINE COMMENTS > ervin wrote in configeditor.cpp:49 > Better wrap the const char* in a QStringLiteral. > > Also, but more of a nitpick this time: style wise I'd favor using = for those > initialization (and others in the file) than parenthesis. It gets too close

D24743: Update GTK settings according to Plasma settings

ervin added inline comments. INLINE COMMENTS > configeditor.cpp:49 > +QString > configLocation(QStandardPaths::writableLocation(QStandardPaths::GenericConfigLocation)); > +QString gtk3ConfigPath(configLocation + "/gtk-3.0/settings.ini"); > + Better wrap the const char* in a

D24743: Update GTK settings according to Plasma settings

gikari updated this revision to Diff 68260. gikari added a comment. Fix daemon hangup on font change REPOSITORY R99 KDE Gtk Configuration Tool CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24743?vs=68258=68260 BRANCH gtk-settings-in-respective-kcms REVISION DETAIL

D24743: Update GTK settings according to Plasma settings

gikari added inline comments. INLINE COMMENTS > broulik wrote in gtkconfig.cpp:116 > In what way? There's a comment in the style KCM saying > > // # FIXME - Doesn't apply all settings correctly due to bugs in > KApplication/KToolbar >

D24743: Update GTK settings according to Plasma settings

gikari updated this revision to Diff 68258. gikari added a comment. Fix pidof launching and remove static from the regexp's REPOSITORY R99 KDE Gtk Configuration Tool CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24743?vs=68256=68258 BRANCH gtk-settings-in-respective-kcms

D24743: Update GTK settings according to Plasma settings

gikari updated this revision to Diff 68256. gikari marked 16 inline comments as done. gikari added a comment. Fix most of the issues brought by @broulik REPOSITORY R99 KDE Gtk Configuration Tool CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24743?vs=68187=68256 BRANCH

D24743: Update GTK settings according to Plasma settings

gikari added a comment. In D24743#549712 , @ngraham wrote: > Nice work! So what's our update story here? If a user has set something from the GTK KCM that's not the same as the data set by the applicable other KCM, who wins, and what happens

D24743: Update GTK settings according to Plasma settings

ngraham added a comment. Nice work! So what's our update story here? If a user has set something from the GTK KCM that's not the same as the data set by the applicable other KCM, who wins, and what happens when said user upgrades to 5.18 (which is presumably the release that will have

D24743: Update GTK settings according to Plasma settings

broulik added a comment. Pretty cool! Some minor nitpicks. Once all the KCMs have been ported to KConfigXT (XML config description, currently ongoing) we'll have a compile-time checked way to read KDE settings and easier way to get the default values. INLINE COMMENTS > CMakeLists.txt:9

D24743: Update GTK settings according to Plasma settings

gikari created this revision. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. gikari requested review of this revision. REVISION SUMMARY To increase usability, - fonts - icon theme - cursor theme - toolbar style (icons-only, text below buttons etc) - icons