Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

panel/plugin: Fix settings storing/restoring #1745

Merged
merged 3 commits into from
Mar 14, 2022
Merged

panel/plugin: Fix settings storing/restoring #1745

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b62ed29
panel/plugin: Fix settings storing/restoring
palinek Mar 14, 2022
51b3132
pluginsettings: Fix cache all keys handling
palinek Mar 14, 2022
56efcad
taskbar: Use "static" last resort default values
palinek Mar 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions panel/lxqtpanelpluginconfigdialog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,15 @@ PluginSettings& LXQtPanelPluginConfigDialog::settings() const
}


/************************************************

************************************************/
void LXQtPanelPluginConfigDialog::closeEvent(QCloseEvent *event)
{
mSettings.storeToCache();
return QDialog::closeEvent(event);
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want only to save settings. We also want to reflect the changes that the Rest button causes in the GUI. There's no guarantee that it happens. Each plugin needs its own handling and that makes this change redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also want to reflect the changes that the Rest button causes in the GUI.

The Reset button just restores the values from cache (pre-existing) into the settings object and calls locadSettings() which must each plugin's config dialog implement. And that's exactly, what you described, it should do.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from not being needed, this is also at odd with the on-the-fly applying of settings change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@palinek, please forget about what I wrote at #1741 (comment). As I said at #1741 (comment), I remembered everything later.

Copy link
Contributor Author

@palinek palinek Mar 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from not being needed, this is also at odd with the on-the-fly applying of settings change.

?
Aren't you misreading the code? This does:

  • when the config dialog is closed, the current (and applied) settings are stored into "old values cache"
  • when the config dialog is opened next time, the reset button will be (optionally) resetting to values "from previous close" (or from the plugin creation if no config showed yet)

The close logic does nothing to the currently used configuration values.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK but it isn't needed. We don't need to be worried about the cache; it'll be correct if Reset does its job.


/************************************************

Expand Down
4 changes: 4 additions & 0 deletions panel/lxqtpanelpluginconfigdialog.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include "pluginsettings.h"

class QComboBox;
class QCloseEvent;

class LXQT_PANEL_API LXQtPanelPluginConfigDialog : public QDialog
{
Expand All @@ -46,6 +47,9 @@ class LXQT_PANEL_API LXQtPanelPluginConfigDialog : public QDialog

PluginSettings &settings() const;

protected:
virtual void closeEvent(QCloseEvent *event) override;

protected slots:
/*
Saves settings in conf file.
Expand Down
3 changes: 2 additions & 1 deletion panel/plugin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,8 @@ Plugin::Plugin(const LXQt::PluginInfo &desktopFile, LXQt::Settings *settings, co
************************************************/
Plugin::~Plugin()
{
if (mConfigDialog)
delete mConfigDialog.data();
delete mPlugin;
delete mPluginLoader;
delete mSettings;
Expand Down Expand Up @@ -515,7 +517,6 @@ void Plugin::showConfigureDialog()
if (!mConfigDialog)
return;

connect(this, &Plugin::destroyed, mConfigDialog.data(), &QWidget::close);
mPanel->willShowWindow(mConfigDialog);
mConfigDialog->show();
mConfigDialog->raise();
Expand Down
23 changes: 17 additions & 6 deletions panel/pluginsettings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,18 @@
#include "pluginsettings.h"
#include "pluginsettings_p.h"
#include <LXQt/Settings>
#include <memory>

class PluginSettingsPrivate
{
public:
PluginSettingsPrivate(LXQt::Settings* settings, const QString &group)
: mSettings(settings)
, mOldSettings(settings)
, mGroup(group)
{
mSettings->beginGroup(mGroup);
mOldSettings = std::make_unique<LXQt::SettingsCache>(mSettings);
mSettings->endGroup();
}

QString prefix() const;
Expand All @@ -46,7 +49,7 @@ class PluginSettingsPrivate
}

LXQt::Settings *mSettings;
LXQt::SettingsCache mOldSettings;
std::unique_ptr<LXQt::SettingsCache> mOldSettings;
QString mGroup;
QStringList mSubGroups;
};
Expand Down Expand Up @@ -163,10 +166,8 @@ void PluginSettings::clear()
void PluginSettings::sync()
{
Q_D(PluginSettings);
d->mSettings->beginGroup(d->mGroup);
d->mSettings->sync();
d->mOldSettings.loadFromSettings();
d->mSettings->endGroup();
storeToCache();
emit settingsChanged();
}

Expand Down Expand Up @@ -205,7 +206,17 @@ void PluginSettings::loadFromCache()
{
Q_D(PluginSettings);
d->mSettings->beginGroup(d->mGroup);
d->mOldSettings.loadToSettings();
d->mSettings->remove(QString{});
d->mOldSettings->loadToSettings();
d->mSettings->endGroup();
emit settingsChanged();
}

void PluginSettings::storeToCache()
{
Q_D(PluginSettings);
d->mSettings->beginGroup(d->mGroup);
d->mOldSettings = std::make_unique<LXQt::SettingsCache>(d->mSettings);
d->mSettings->endGroup();
}

Expand Down
1 change: 1 addition & 0 deletions panel/pluginsettings.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ class LXQT_PANEL_API PluginSettings : public QObject
void endGroup();

void loadFromCache();
void storeToCache();

signals:
void settingsChanged();
Expand Down
8 changes: 4 additions & 4 deletions plugin-taskbar/lxqttaskbar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,10 +485,10 @@ void LXQtTaskBar::settingsChanged()
else
setButtonStyle(Qt::ToolButtonTextBesideIcon);

mShowOnlyOneDesktopTasks = mPlugin->settings()->value(QStringLiteral("showOnlyOneDesktopTasks"), mShowOnlyOneDesktopTasks).toBool();
mShowDesktopNum = mPlugin->settings()->value(QStringLiteral("showDesktopNum"), mShowDesktopNum).toInt();
mShowOnlyCurrentScreenTasks = mPlugin->settings()->value(QStringLiteral("showOnlyCurrentScreenTasks"), mShowOnlyCurrentScreenTasks).toBool();
mShowOnlyMinimizedTasks = mPlugin->settings()->value(QStringLiteral("showOnlyMinimizedTasks"), mShowOnlyMinimizedTasks).toBool();
mShowOnlyOneDesktopTasks = mPlugin->settings()->value(QStringLiteral("showOnlyOneDesktopTasks"), false).toBool();
mShowDesktopNum = mPlugin->settings()->value(QStringLiteral("showDesktopNum"), 0).toInt();
mShowOnlyCurrentScreenTasks = mPlugin->settings()->value(QStringLiteral("showOnlyCurrentScreenTasks"), false).toBool();
mShowOnlyMinimizedTasks = mPlugin->settings()->value(QStringLiteral("showOnlyMinimizedTasks"), false).toBool();
mAutoRotate = mPlugin->settings()->value(QStringLiteral("autoRotate"), true).toBool();
mCloseOnMiddleClick = mPlugin->settings()->value(QStringLiteral("closeOnMiddleClick"), true).toBool();
mRaiseOnCurrentDesktop = mPlugin->settings()->value(QStringLiteral("raiseOnCurrentDesktop"), false).toBool();
Expand Down