CCBUG: 386665
As discussed in bug 386665 GConf is deprecated and needs to be replaced by GSettings to keep features enabled.
Question: Do we need GConf as a fallback for PA versions without module-gsettings?
drosca | |
davidedmundson |
CCBUG: 386665
As discussed in bug 386665 GConf is deprecated and needs to be replaced by GSettings to keep features enabled.
Question: Do we need GConf as a fallback for PA versions without module-gsettings?
Checkboxes in Advanced tab are enabled again.
Changed settings are shown in dconf-editor and vice versa.
Combine output checkbox shows/hides combined sink in applet
Automatic diff as part of commit; lint not applicable. |
Automatic diff as part of commit; unit tests not applicable. |
It still needs to keep GConf code for systems without gsettings pulseaudio module (eg. pulseaudio < 12.0).
It should be configurable which one to build, with gsettings being default if found.
Also make the Qt keywords changes in separate commit or include gsettings headers as last with #undef signal.
It now defaults to GSettings if found and one can pass -DUSE_GCONF=TRUE to build it using GConf
CMakeLists.txt | ||
---|---|---|
52 | Those two options (gconf, gsettings) should be mutually exclusive. | |
src/kcm/package/contents/ui/Advanced.qml | ||
82 | This should still check for "module-gconf" or "module-gsettings" being loaded in pulseaudio. The way you changed it is that it will just completely hide those options if appropriate module is not loaded in pulseaudio without any indication to user what is wrong. |
CMakeLists.txt | ||
---|---|---|
44 | This should probably use cache variables: set(USE_GCONF FALSE CACHE STRING "Build with GConf") set(USE_GSETTINGS TRUE CACHE STRING "Build with GSettings") | |
src/gsettingsitem.cpp | ||
23 | not needed | |
27 | local includes should be the first in file | |
41 | nullptr | |
54 | newline | |
64 | indentation | |
74 | braces | |
83 | newline | |
92 | braces | |
src/gsettingsitem.h | ||
35 | newline | |
42 | newline | |
49 | move to .cpp | |
src/modulemanager.cpp | ||
91–92 | newline |
Ping! Is this still being looked at? As distributions may phase out pulseaudio-gconf, this is important.
CMakeLists.txt | ||
---|---|---|
44 | this still needs to be addressed, anything else though? |
src/gsettingsitem.cpp | ||
---|---|---|
59 | = nullptr, otherwise you have a crash if the following switch falls to default case. |
src/gsettingsitem.cpp | ||
---|---|---|
43 | 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. |
src/gsettingsitem.cpp | ||
---|---|---|
43 | As I said, to better not silently miss new types of keys, as logging to terminal will not help with a GUI application. |
src/gsettingsitem.cpp | ||
---|---|---|
43 | Sure, but qFatal here will take down the whole plasmashell and effectively break the login to desktop session. Also, this should only be reached by developer when trying to read new key as type of existing keys should not change (and it already supports all types that are currently used in code). |
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?
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
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/
cmake/FindGIO.cmake | ||
---|---|---|
20 | It seems _LibGIOLinkFlags, _LibGIOLinkDir & _LibGIOCflags are not propagated into the build. So if e.g. the libraries are not in default lib dir, linking will fail, cmp. https://build.kde.org/view/Failing/job/Plasma/job/plasma-pa/job/kf5-qt5%20FreeBSDQt5.12/18/ Please give this another look & also compare with other pkgconfig() using Find' macros. |