Commit c441909c authored by Igor Kushnir's avatar Igor Kushnir
Browse files

Don't call ConfigDialog::applyChanges() if nothing has changed

The OK button is always enabled, so it can be clicked when nothing is
modified on the current page. Calling page->apply() and emitting
configSaved() is useless and wasteful in this case.

One downside of this optimization is that if a change of some widget on
some ConfigPage is not connected to that page's changed() signal,
clicking OK button no longer saves that setting, unless another
"connected" widget on that page is also changed and the Apply button is
enabled. However such missing connection is problematic in other ways:
the Apply button does not become enabled; switching to a different
ConfigPage silently discards a lone "unconnected" change. Therefore such
a bug can be fixed properly only by adding the missing connection.
parent e86fb930
Pipeline #224159 passed with stage
in 15 minutes and 44 seconds
......@@ -39,7 +39,11 @@ ConfigDialog::ConfigDialog(QWidget* parent)
connect(button(QDialogButtonBox::Apply), &QPushButton::clicked, onApplyClicked);
connect(button(QDialogButtonBox::Ok), &QPushButton::clicked, onApplyClicked);
connect(button(QDialogButtonBox::Ok), &QPushButton::clicked, [this, onApplyClicked] {
if (m_currentPageHasChanges) {
connect(button(QDialogButtonBox::RestoreDefaults), &QPushButton::clicked, this, [this]() {
auto page = qobject_cast<ConfigPage*>(currentPage()->widget());
......@@ -187,6 +191,7 @@ void ConfigDialog::onPageChanged()
void ConfigDialog::applyChanges(ConfigPage* page)
// must set this to false before calling apply, otherwise we get the confirmation dialog
// whenever we enable/disable plugins.
// This is because KPageWidget then emits currentPageChanged("Plugins", nullptr), which seems like a bug to me,
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment