D14147: Port from GConf to GSettings

nicolasfella closed this revision. REPOSITORY R115 Plasma Audio Volume Applet REVISION DETAIL https://phabricator.kde.org/D14147 To: nicolasfella, drosca, davidedmundson Cc: fvogt, kossebau, ngraham, pino, lbeltrame, evpokp, rikmills, broulik, asturmlechner, plasma-devel, jraleigh, GB_2, ra

D14147: Port from GConf to GSettings

kossebau added a comment. In D14147#443705 , @kossebau wrote: > Seems this patch sadly broke the build on FreeBSD: https://build.kde.org/job/Plasma/view/Everything/job/plasma-pa/job/kf5-qt5%20FreeBSDQt5.12/18/ Solved for now with fba8f732

D14147: Port from GConf to GSettings

kossebau added inline comments. INLINE COMMENTS > FindGIO.cmake:20 > +include(UsePkgConfig) > +pkgconfig(gio-2.0 _LibGIOIncDir _LibGIOLinkDir _LibGIOLinkFlags > _LibGIOCflags) > +endif(NOT WIN32) It seems `_LibGIOLinkFlags`, `_LibGIOLinkDir` & `_LibGIOCflags` are not propagated into t

D14147: Port from GConf to GSettings

kossebau added a comment. Seems this patch sadly broke the build on FreeBSD: https://build.kde.org/job/Plasma/view/Everything/job/plasma-pa/job/kf5-qt5%20FreeBSDQt5.12/18/ REPOSITORY R115 Plasma Audio Volume Applet REVISION DETAIL https://phabricator.kde.org/D14147 To: nicolasfella, dro

D14147: Port from GConf to GSettings

broulik reopened this revision. broulik added a comment. This revision is now accepted and ready to land. With this it now aborts here (process:114351): GLib-GIO-ERROR **: 18:24:26.823: Settings schema 'org.freedesktop.pulseaudio.module-group' is not installed REPOSITORY R115 Plasma

D14147: Port from GConf to GSettings

ngraham added a comment. Now that this is in, can we now finally fix https://gitlab.freedesktop.org/pulseaudio/pulseaudio/issues/509? Or is there yet more to do before that can happen? REPOSITORY R115 Plasma Audio Volume Applet REVISION DETAIL https://phabricator.kde.org/D14147 To: nic

D14147: Port from GConf to GSettings

This revision was automatically updated to reflect the committed changes. Closed by commit R115:e04e034c24b8: Port from GConf to GSettings (authored by nicolasfella). REPOSITORY R115 Plasma Audio Volume Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14147?vs=55133&id=55136 RE

D14147: Port from GConf to GSettings

nicolasfella updated this revision to Diff 55133. nicolasfella added a comment. - address comments REPOSITORY R115 Plasma Audio Volume Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14147?vs=55090&id=55133 BRANCH arcpatch-D14147 REVISION DETAIL https://phabricator.kde

D14147: Port from GConf to GSettings

drosca added inline comments. INLINE COMMENTS > pino wrote in gsettingsitem.cpp:42 > As I said, to better not silently miss new types of keys, as logging to > terminal will not help with a GUI application. Sure, but qFatal here will take down the whole plasmashell and effectively break the log

D14147: Port from GConf to GSettings

pino added inline comments. INLINE COMMENTS > drosca wrote in gsettingsitem.cpp:42 > qCritical is not hard-fail, that would be qFatal, but don't really see a > reason to abort here in any case. > > It should be at least qWarning though, as qDebug is usually disabled by > default. As I said, t

D14147: Port from GConf to GSettings

drosca added inline comments. INLINE COMMENTS > pino wrote in gsettingsitem.cpp:42 > should this be qCCritical, to hard-fail in case other types of keys are used > (instead of silently doing nothing)? qCritical is not hard-fail, that would be qFatal, but don't really see a reason to abort here

D14147: Port from GConf to GSettings

pino added inline comments. INLINE COMMENTS > gsettingsitem.cpp:39 > +case G_VARIANT_CLASS_STRING: > +toReturn = QVariant(QString::fromUtf8(g_variant_get_string(gvalue, > NULL))); > +break; nullptr here > gsettingsitem.cpp:42 > +default: > +qCDebug(PLASMAPA()) <

D14147: Port from GConf to GSettings

drosca added inline comments. INLINE COMMENTS > gsettingsitem.cpp:58 > +GVariant *oldValue = g_settings_get_value(m_settings, > key.toLatin1().data()); > +GVariant *newValue; > + `= nullptr`, otherwise you have a crash if the following switch falls to default case. REPOSITORY R115 P

D14147: Port from GConf to GSettings

nicolasfella updated this revision to Diff 55090. nicolasfella marked 11 inline comments as done. nicolasfella added a comment. - Use and reqire GSettings by default and add CMake option to use GConf - Coding style REPOSITORY R115 Plasma Audio Volume Applet CHANGES SINCE LAST UPDATE htt

D14147: Port from GConf to GSettings

asturmlechner added inline comments. INLINE COMMENTS > drosca wrote in CMakeLists.txt:46 > This should probably use cache variables: > > set(USE_GCONF FALSE CACHE STRING "Build with GConf") > set(USE_GSETTINGS TRUE CACHE STRING "Build with GSettings") this still needs to be addressed, anyth

D14147: Port from GConf to GSettings

lbeltrame added a comment. Ping! Is this still being looked at? As distributions may phase out pulseaudio-gconf, this is important. REPOSITORY R115 Plasma Audio Volume Applet REVISION DETAIL https://phabricator.kde.org/D14147 To: nicolasfella, drosca, davidedmundson Cc: lbeltrame, evpok

D14147: Port from GConf to GSettings

drosca added inline comments. INLINE COMMENTS > CMakeLists.txt:46 > + > +if(USE_GCONF AND USE_GSETTINGS) > +message(FATAL_ERROR "USE_GCONF and USE_GSETTINGS cannot be used at the > same time") This should probably use cache variables: set(USE_GCONF FALSE CACHE STRING "Build with GConf")

D14147: Port from GConf to GSettings

nicolasfella updated this revision to Diff 37913. nicolasfella added a comment. - Remove unneeded class declaration REPOSITORY R115 Plasma Audio Volume Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14147?vs=37911&id=37913 BRANCH gsettings REVISION DETAIL https://phab

D14147: Port from GConf to GSettings

nicolasfella updated this revision to Diff 37911. nicolasfella added a comment. - Fail if user sets both USE_GCONF and USE_GSETTINGS - Restore warning when module is unloaded REPOSITORY R115 Plasma Audio Volume Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14147?vs=37888

D14147: Port from GConf to GSettings

drosca requested changes to this revision. drosca added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > CMakeLists.txt:54 > +set(USE_GCONF FALSE) > +endif() > Those two options (gconf, gsettings) should be mutually exclusive. > Advanced.qml:76 >

D14147: Port from GConf to GSettings

nicolasfella added a comment. It now defaults to GSettings of found and one can pass -DUSE_GCONF to build it using GConf REPOSITORY R115 Plasma Audio Volume Applet REVISION DETAIL https://phabricator.kde.org/D14147 To: nicolasfella, drosca, davidedmundson Cc: rikmills, broulik, asturmle

D14147: Port from GConf to GSettings

nicolasfella updated this revision to Diff 37888. nicolasfella added a comment. - Don't require GIO - Fix copyright REPOSITORY R115 Plasma Audio Volume Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14147?vs=37887&id=37888 BRANCH gsettings REVISION DETAIL https://ph

D14147: Port from GConf to GSettings

nicolasfella updated this revision to Diff 37887. nicolasfella added a comment. - Support both GConf and GSettings REPOSITORY R115 Plasma Audio Volume Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14147?vs=37836&id=37887 BRANCH gsettings REVISION DETAIL https://phabr

D14147: Port from GConf to GSettings

nicolasfella added a comment. Keyword changes were made in D14107 REPOSITORY R115 Plasma Audio Volume Applet REVISION DETAIL https://phabricator.kde.org/D14147 To: nicolasfella, drosca, davidedmundson Cc: rikmills, broulik, asturmlechner, plasma-devel

D14147: Port from GConf to GSettings

broulik added a comment. I would prefer those keyword changes to be done separately REPOSITORY R115 Plasma Audio Volume Applet REVISION DETAIL https://phabricator.kde.org/D14147 To: nicolasfella, drosca, davidedmundson Cc: broulik, asturmlechner, plasma-devel, ragreen, Pitel, ZrenBot, le

D14147: Port from GConf to GSettings

drosca requested changes to this revision. drosca added a comment. This revision now requires changes to proceed. It still needs to keep GConf code for systems without gsettings pulseaudio module (eg. pulseaudio < 12.0). Also make the Qt keywords changes in separate commit or include gset

D14147: Port from GConf to GSettings

nicolasfella created this revision. nicolasfella added reviewers: drosca, davidedmundson. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. nicolasfella requested review of this revision. REVISION SUMMARY CCBUG: 386665 As discussed in b