search

lxqt / lxqt-config

Tools to configure LXQt and the underlying operating system
https://lxqt.github.io
GNU Lesser General Public License v2.1
86 stars 60 forks source link

Start `xsettingsd` on X11 only when GTK themes should be set #1051

Closed tsujan closed 2 months ago

tsujan commented 2 months ago

Closes https://github.com/lxqt/lxqt-config/issues/1049

M4he commented 2 months ago

I have not tested this code (currently have no means to) and just looked at the changed code but I wondered:

If the dialog is opened while the ControlGTKThemeEnabled option is disabled, then xsettingsd is not started (check does not trigger in showAdvancedOptions). This is desired.

But if the user now clicks the checkbox for "Set GTK themes" for the first time and then "Apply", the GTK files will be written but is xsettingsd started as a result? To me it seems it is not (only when ControlGTKThemeEnabled was true before the dialog opened).

I'd expect that if the checkbox is enabled and "Apply" is clicked in the same dialog instance, xsettingsd gets started (if not already running) to apply the changes to running applications immediately.

tsujan commented 2 months ago

But if the user now clicks the checkbox for "Set GTK themes" for the first time

xsettingsd is started as soon as the box is checked. That can happen when the user checks it or when the dialog is shown with ControlGTKThemeEnabled=true in ~/.config/lxqt/lxqt-config-appearance.conf (the code checks it).

M4he commented 2 months ago

But if the user now clicks the checkbox for "Set GTK themes" for the first time

xsettingsd is started as soon as the box is checked. That can happen when the user checks it or when the dialog is shown with ControlGTKThemeEnabled=true in ~/.config/lxqt/lxqt-config-appearance.conf (the code checks it).

Okay sorry then I might have misinterpreted the code. I only saw the starting happen in showAdvancedOptions via initControls which I thought would only be called once when the dialog is initially opened.

tsujan commented 2 months ago

→ GTKConfig::showAdvancedOptions in the patch.

tsujan commented 2 months ago

@stefonarch, thanks for the test! I think the code is simple enough to be merged.