SettingsDialog: Call Aspect::cancel on cancel

Previously the SettingsDialog did not call Aspect::cancel().

Without this, the volatile value is not reset if the user did some
changes before pressing cancel.

This "should" mean that re-opening the settings dialog should present
the changed values again. But since not all aspects are implemented
correctly, they reinitialize from their value instead of their
volatileValue.

When the Settings Dialog is reopened and applied, a warning is triggered
as the container thinks its dirty, but the Aspect::apply returns false.

Change-Id: I8fac66fc95f9118d69c789ced22481d121769f0b
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
Reviewed-by: Christian Stenger <christian.stenger@qt.io>
Reviewed-by: hjk <hjk@qt.io>

src/plugins/autotest/testsettingspage.cpp

@@ -153,6 +153,8 @@ TestSettingsWidget::TestSettingsWidget()
         if (!changedIds.isEmpty())
             TestTreeModel::instance()->rebuild(changedIds);
     });
+
+    setOnCancel([] { Internal::testSettings().cancel(); });
 }
 
 enum TestBaseInfo

src/plugins/beautifier/artisticstyle/artisticstyle.cpp

@@ -221,9 +221,10 @@ public:
 
         setOnApply([&s, configurations] {
             s.customStyle.setValue(configurations->currentConfiguration());
-            settings().apply();
+            s.apply();
             s.save();
         });
+        setOnCancel([&s] { s.cancel(); });
 
         s.read();
 

src/plugins/beautifier/clangformat/clangformat.cpp

@@ -300,6 +300,7 @@ public:
             settings().apply();
             settings().save();
         });
+        setOnCancel([] { settings().cancel(); });
 
         s.read();
 

src/plugins/beautifier/uncrustify/uncrustify.cpp

@@ -225,6 +225,8 @@ public:
             settings().apply();
             s.save();
         });
+
+        setOnCancel([] { settings().cancel(); });
     }
 };
 

src/plugins/coreplugin/dialogs/ioptionspage.cpp

@@ -29,6 +29,7 @@ class IOptionsPageWidgetPrivate
 {
 public:
     std::function<void()> m_onApply;
+    std::function<void()> m_onCancel;
     std::function<void()> m_onFinish;
 };
 
@@ -104,6 +105,11 @@ void IOptionsPageWidget::setOnApply(const std::function<void()> &func)
     d->m_onApply = func;
 }
 
+void IOptionsPageWidget::setOnCancel(const std::function<void()> &func)
+{
+    d->m_onCancel = func;
+}
+
 /*!
     Sets the function that is called by default on finish to \a func.
 */
@@ -122,6 +128,12 @@ void IOptionsPageWidget::apply()
         d->m_onApply();
 }
 
+void IOptionsPageWidget::cancel()
+{
+    if (d->m_onCancel)
+        d->m_onCancel();
+}
+
 /*!
     Calls the finish function, if set.
     \sa setOnFinish
@@ -260,6 +272,19 @@ void IOptionsPage::apply()
     }
 }
 
+void IOptionsPage::cancel()
+{
+    if (auto widget = qobject_cast<IOptionsPageWidget *>(d->m_widget))
+         widget->cancel();
+
+    if (d->m_settingsProvider) {
+         AspectContainer *container = d->m_settingsProvider();
+         QTC_ASSERT(container, return);
+         if (container->isDirty())
+            container->cancel();
+    }
+}
+
 /*!
     Called directly before the \uicontrol Options dialog closes. Here you should
     delete the widget that was created in widget() to free resources.

src/plugins/coreplugin/dialogs/ioptionspage.h

@@ -27,11 +27,13 @@ public:
     IOptionsPageWidget();
     ~IOptionsPageWidget();
     void setOnApply(const std::function<void()> &func);
+    void setOnCancel(const std::function<void()> &func);
     void setOnFinish(const std::function<void()> &func);
 
 protected:
     friend class IOptionsPage;
     virtual void apply();
+    virtual void cancel();
     virtual void finish();
 
 private:
@@ -59,6 +61,7 @@ public:
 
     virtual QWidget *widget();
     virtual void apply();
+    virtual void cancel();
     virtual void finish();
 
     virtual bool matches(const QRegularExpression &regexp) const;

src/plugins/coreplugin/dialogs/settingsdialog.cpp

@@ -594,11 +594,13 @@ void SettingsDialog::createGui()
     QWidget *emptyWidget = new QWidget(this);
     m_stackedLayout->addWidget(emptyWidget); // no category selected, for example when filtering
 
-    QDialogButtonBox *buttonBox = new QDialogButtonBox(QDialogButtonBox::Ok |
+    QDialogButtonBox *buttonBox = new QDialogButtonBox(
-                                                       QDialogButtonBox::Apply |
+        QDialogButtonBox::Ok | QDialogButtonBox::Apply | QDialogButtonBox::Cancel);
-                                                       QDialogButtonBox::Cancel);
+    connect(
-    connect(buttonBox->button(QDialogButtonBox::Apply), &QAbstractButton::clicked,
+        buttonBox->button(QDialogButtonBox::Apply),
-            this, &SettingsDialog::apply);
+        &QAbstractButton::clicked,
+        this,
+        &SettingsDialog::apply);
 
     connect(buttonBox, &QDialogButtonBox::accepted, this, &SettingsDialog::accept);
     connect(buttonBox, &QDialogButtonBox::rejected, this, &SettingsDialog::reject);
@@ -744,8 +746,10 @@ void SettingsDialog::reject()
         return;
     m_finished = true;
     disconnectTabWidgets();
-    for (IOptionsPage *page : std::as_const(m_pages))
+    for (IOptionsPage *page : std::as_const(m_pages)) {
+        page->cancel();
         page->finish();
+    }
     done(QDialog::Rejected);
 }
 

src/plugins/git/gerrit/gerritoptionspage.cpp

@@ -63,8 +63,14 @@ public:
             Git::Tr::tr("P&rotocol:"), httpsCheckBox
         }.attachTo(this);
 
-        setOnApply([this, hostLineEdit, userLineEdit, sshChooser,
+        setOnApply([this,
-                    curlChooser, portSpinBox, httpsCheckBox, onChanged] {
+                    hostLineEdit,
+                    userLineEdit,
+                    sshChooser,
+                    curlChooser,
+                    portSpinBox,
+                    httpsCheckBox,
+                    onChanged] {
             GerritParameters newParameters;
             newParameters.server = GerritServer(hostLineEdit->text().trimmed(),
                                          static_cast<unsigned short>(portSpinBox->value()),