Vcs: Unify options pages for VcsBaseClientImpl based VCSes

Change-Id: Ia5dd7746f5857e7254251e3510c0987737cfad6d
Reviewed-by: Orgad Shaneh <orgads@gmail.com>

src/plugins/bazaar/bazaarplugin.cpp

@@ -168,14 +168,10 @@ bool BazaarPlugin::initialize(const QStringList &arguments, QString *errorMessag
     m_client = new BazaarClient;
     auto vcsCtrl = new BazaarControl(m_client);
     initializeVcs(vcsCtrl, context);
-
-    auto options = new OptionsPage;
-    connect(options, &OptionsPage::settingsChanged,
-            vcsCtrl, &Core::IVersionControl::configurationChanged);
-    addAutoReleasedObject(options);
-
     connect(m_client, &VcsBaseClient::changed, vcsCtrl, &BazaarControl::changed);
 
+    addAutoReleasedObject(new OptionsPage(vcsCtrl));
+
     static const char *describeSlot = SLOT(view(QString,QString));
     const int editorCount = sizeof(editorParameters) / sizeof(VcsBaseEditorParameters);
     const auto widgetCreator = []() { return new BazaarEditorWidget; };

src/plugins/bazaar/optionspage.cpp

@@ -41,8 +41,7 @@ using namespace Bazaar::Internal;
 using namespace Bazaar;
 using namespace VcsBase;
 
-OptionsPageWidget::OptionsPageWidget(QWidget *parent)
+OptionsPageWidget::OptionsPageWidget(QWidget *parent) : VcsClientOptionsPageWidget(parent)
-    : QWidget(parent)
 {
     m_ui.setupUi(this);
     m_ui.commandChooser->setExpectedKind(Utils::PathChooser::ExistingCommand);
@@ -70,29 +69,10 @@ void OptionsPageWidget::setSettings(const VcsBaseClientSettings &s)
     m_ui.timeout->setValue(s.intValue(BazaarSettings::timeoutKey));
 }
 
-OptionsPage::OptionsPage()
+OptionsPage::OptionsPage(Core::IVersionControl *control) :
+    VcsClientOptionsPage(control, BazaarPlugin::instance()->client())
 {
     setId(VcsBase::Constants::VCS_ID_BAZAAR);
     setDisplayName(tr("Bazaar"));
-}
+    setWidgetFactory([]() { return new OptionsPageWidget; });
-
-QWidget *OptionsPage::widget()
-{
-    if (!m_optionsPageWidget)
-        m_optionsPageWidget = new OptionsPageWidget;
-    m_optionsPageWidget->setSettings(BazaarPlugin::instance()->client()->settings());
-    return m_optionsPageWidget;
-}
-
-void OptionsPage::apply()
-{
-    if (!m_optionsPageWidget)
-        return;
-    BazaarPlugin *plugin = BazaarPlugin::instance();
-    const VcsBaseClientSettings newSettings = m_optionsPageWidget->settings();
-    VcsBaseClientSettings &s = plugin->client()->settings();
-    if (newSettings != s) {
-        s = newSettings;
-        emit settingsChanged();
-    }
 }

src/plugins/bazaar/optionspage.h

@@ -44,7 +44,7 @@ class VcsBaseClientSettings;
 namespace Bazaar {
 namespace Internal {
 
-class OptionsPageWidget : public QWidget
+class OptionsPageWidget : public VcsBase::VcsClientOptionsPageWidget
 {
     Q_OBJECT
 
@@ -59,22 +59,12 @@ private:
 };
 
 
-class OptionsPage : public VcsBase::VcsBaseOptionsPage
+class OptionsPage : public VcsBase::VcsClientOptionsPage
 {
     Q_OBJECT
 
 public:
-    OptionsPage();
+    OptionsPage(Core::IVersionControl *control);
-
-    QWidget *widget();
-    void apply();
-    void finish() { }
-
-signals:
-    void settingsChanged();
-
-private:
-    QPointer<OptionsPageWidget> m_optionsPageWidget;
 };
 
 } // namespace Internal

src/plugins/cvs/cvsplugin.cpp

@@ -254,10 +254,7 @@ bool CvsPlugin::initialize(const QStringList &arguments, QString *errorMessage)
 
     m_client = new CvsClient;
 
-    auto options = new SettingsPage;
+    addAutoReleasedObject(new SettingsPage(versionControl()));
-    connect(options, &SettingsPage::settingsChanged,
-            versionControl(), &IVersionControl::configurationChanged);
-    addAutoReleasedObject(new SettingsPage);
 
     addAutoReleasedObject(new VcsSubmitEditorFactory(&submitParameters,
         []() { return new CvsSubmitEditor(&submitParameters); }));

src/plugins/cvs/settingspage.cpp

@@ -47,8 +47,7 @@ using namespace Cvs::Internal;
 using namespace Utils;
 using namespace VcsBase;
 
-SettingsPageWidget::SettingsPageWidget(QWidget *parent) :
+SettingsPageWidget::SettingsPageWidget(QWidget *parent) : VcsClientOptionsPageWidget(parent)
-    QWidget(parent)
 {
     m_ui.setupUi(this);
     m_ui.commandPathChooser->setExpectedKind(PathChooser::ExistingCommand);
@@ -78,33 +77,10 @@ void SettingsPageWidget::setSettings(const VcsBaseClientSettings &s)
     m_ui.describeByCommitIdCheckBox->setChecked(s.boolValue(CvsSettings::describeByCommitIdKey));
 }
 
-SettingsPage::SettingsPage()
+SettingsPage::SettingsPage(Core::IVersionControl *control) :
+    VcsClientOptionsPage(control, CvsPlugin::instance()->client())
 {
     setId(VcsBase::Constants::VCS_ID_CVS);
     setDisplayName(tr("CVS"));
-}
+    setWidgetFactory([]() { return new SettingsPageWidget; });
-
-QWidget *SettingsPage::widget()
-{
-    if (!m_widget) {
-        m_widget = new SettingsPageWidget;
-        m_widget->setSettings(CvsPlugin::instance()->client()->settings());
-    }
-    return m_widget;
-}
-
-void SettingsPage::apply()
-{
-    VcsBaseClientSettings &s = CvsPlugin::instance()->client()->settings();
-    const VcsBaseClientSettings newSettings = m_widget->settings();
-    if (s != newSettings) {
-        s = newSettings;
-        s.writeSettings(Core::ICore::settings());
-        emit settingsChanged();
-    }
-}
-
-void SettingsPage::finish()
-{
-    delete m_widget;
 }

src/plugins/cvs/settingspage.h

@@ -50,7 +50,7 @@ class VcsBaseClientSettings;
 namespace Cvs {
 namespace Internal {
 
-class SettingsPageWidget : public QWidget
+class SettingsPageWidget : public VcsBase::VcsClientOptionsPageWidget
 {
     Q_OBJECT
 
@@ -65,22 +65,12 @@ private:
 };
 
 
-class SettingsPage : public VcsBase::VcsBaseOptionsPage
+class SettingsPage : public VcsBase::VcsClientOptionsPage
 {
     Q_OBJECT
 
 public:
-    SettingsPage();
+    SettingsPage(Core::IVersionControl *control);
-
-    QWidget *widget();
-    void apply();
-    void finish();
-
-signals:
-    void settingsChanged();
-
-private:
-    QPointer<SettingsPageWidget> m_widget;
 };
 
 } // namespace Cvs

src/plugins/git/gitplugin.cpp

@@ -279,10 +279,7 @@ bool GitPlugin::initialize(const QStringList &arguments, QString *errorMessage)
     initializeVcs(new GitVersionControl(m_gitClient), context);
 
     // Create the settings Page
-    SettingsPage *options = new SettingsPage;
+    addAutoReleasedObject(new SettingsPage(versionControl()));
-    connect(options, &SettingsPage::settingsChanged,
-            versionControl(), &IVersionControl::configurationChanged);
-    addAutoReleasedObject(options);
 
     static const char *describeSlot = SLOT(show(QString,QString));
     const int editorCount = sizeof(editorParameters) / sizeof(editorParameters[0]);

src/plugins/git/settingspage.cpp

@@ -47,8 +47,7 @@ using namespace VcsBase;
 namespace Git {
 namespace Internal {
 
-SettingsPageWidget::SettingsPageWidget(QWidget *parent) :
+SettingsPageWidget::SettingsPageWidget(QWidget *parent) : VcsClientOptionsPageWidget(parent)
-    QWidget(parent)
 {
     m_ui.setupUi(this);
     if (Utils::HostOsInfo::isWindowsHost()) {
@@ -80,6 +79,7 @@ VcsBaseClientSettings SettingsPageWidget::settings() const
     rc.setValue(GitSettings::winSetHomeEnvironmentKey, m_ui.winHomeCheckBox->isChecked());
     rc.setValue(GitSettings::gitkOptionsKey, m_ui.gitkOptionsLineEdit->text().trimmed());
     rc.setValue(GitSettings::repositoryBrowserCmd, m_ui.repBrowserCommandPathChooser->path().trimmed());
+
     return rc;
 }
 
@@ -96,46 +96,28 @@ void SettingsPageWidget::setSettings(const VcsBaseClientSettings &s)
 }
 
 // -------- SettingsPage
-SettingsPage::SettingsPage() :
+SettingsPage::SettingsPage(Core::IVersionControl *control) :
-    m_widget(0)
+    VcsClientOptionsPage(control, GitPlugin::instance()->client())
 {
     setId(VcsBase::Constants::VCS_ID_GIT);
     setDisplayName(tr("Git"));
-}
+    setWidgetFactory([]() { return new SettingsPageWidget; });
-
-QWidget *SettingsPage::widget()
-{
-    if (!m_widget) {
-        m_widget = new SettingsPageWidget;
-        m_widget->setSettings(GitPlugin::instance()->client()->settings());
-    }
-    return m_widget;
 }
 
 void SettingsPage::apply()
 {
-    // Warn if git cannot be found in path if the widget is on top
+    VcsClientOptionsPage::apply();
-    const VcsBaseClientSettings newSettings = m_widget->settings();
+
-    if (m_widget->isVisible()) {
+    if (widget()->isVisible()) {
+        const VcsBaseClientSettings settings = widget()->settings();
+        const GitSettings *rc = static_cast<const GitSettings *>(&settings);
         bool gitFoundOk;
         QString errorMessage;
-        static_cast<const GitSettings &>(newSettings).gitExecutable(&gitFoundOk, &errorMessage);
+        rc->gitExecutable(&gitFoundOk, &errorMessage);
         if (!gitFoundOk)
             Core::AsynchronousMessageBox::warning(tr("Git Settings"), errorMessage);
     }
-
-    VcsBaseClientSettings &s = GitPlugin::instance()->client()->settings();
-    if (s != newSettings) {
-        s = newSettings;
-        s.writeSettings(Core::ICore::settings());
-        emit settingsChanged();
-    }
 }
 
-void SettingsPage::finish()
+} // namespace Internal
-{
+} // namespace Git
-    delete m_widget;
-}
-
-}
-}

src/plugins/git/settingspage.h

@@ -49,34 +49,25 @@ class VcsBaseClientSettings;
 namespace Git {
 namespace Internal {
 
-class SettingsPageWidget : public QWidget {
+class SettingsPageWidget : public VcsBase::VcsClientOptionsPageWidget {
     Q_OBJECT
 public:
     explicit SettingsPageWidget(QWidget *parent = 0);
 
-    VcsBase::VcsBaseClientSettings settings() const;
+    VcsBase::VcsBaseClientSettings settings() const override;
-    void setSettings(const VcsBase::VcsBaseClientSettings &s);
+    void setSettings(const VcsBase::VcsBaseClientSettings &s) override;
 
 private:
     Ui::SettingsPage m_ui;
 };
 
-class SettingsPage : public VcsBase::VcsBaseOptionsPage
+class SettingsPage : public VcsBase::VcsClientOptionsPage
 {
     Q_OBJECT
 
 public:
-    SettingsPage();
+    SettingsPage(Core::IVersionControl *control);
-
+    void apply() override;
-    QWidget *widget();
-    void apply();
-    void finish();
-
-signals:
-    void settingsChanged();
-
-private:
-    QPointer<SettingsPageWidget> m_widget;
 };
 
 } // namespace Internal

src/plugins/mercurial/mercurialplugin.cpp

@@ -139,8 +139,7 @@ bool MercurialPlugin::initialize(const QStringList & /* arguments */, QString *
     m_client = new MercurialClient;
     initializeVcs(new MercurialControl(m_client), context);
 
-    optionsPage = new OptionsPage();
+    addAutoReleasedObject(new OptionsPage(versionControl()));
-    addAutoReleasedObject(optionsPage);
 
     connect(m_client, SIGNAL(changed(QVariant)), versionControl(), SLOT(changed(QVariant)));
     connect(m_client, SIGNAL(needUpdate()), this, SLOT(update()));

src/plugins/mercurial/optionspage.cpp

@@ -45,8 +45,7 @@ using namespace VcsBase;
 namespace Mercurial {
 namespace Internal  {
 
-OptionsPageWidget::OptionsPageWidget(QWidget *parent) :
+OptionsPageWidget::OptionsPageWidget(QWidget *parent) : VcsClientOptionsPageWidget(parent)
-        QWidget(parent)
 {
     m_ui.setupUi(this);
     m_ui.commandChooser->setExpectedKind(Utils::PathChooser::ExistingCommand);
@@ -74,38 +73,12 @@ void OptionsPageWidget::setSettings(const VcsBaseClientSettings &s)
     m_ui.timeout->setValue(s.intValue(MercurialSettings::timeoutKey));
 }
 
-OptionsPage::OptionsPage()
+OptionsPage::OptionsPage(Core::IVersionControl *control) :
+    VcsClientOptionsPage(control, MercurialPlugin::client())
 {
     setId(VcsBase::Constants::VCS_ID_MERCURIAL);
     setDisplayName(tr("Mercurial"));
-}
+    setWidgetFactory([]() { return new OptionsPageWidget; });
-
-QWidget *OptionsPage::widget()
-{
-    if (!optionsPageWidget)
-        optionsPageWidget = new OptionsPageWidget;
-    optionsPageWidget->setSettings(MercurialPlugin::client()->settings());
-    return optionsPageWidget;
-}
-
-void OptionsPage::apply()
-{
-    if (!optionsPageWidget)
-        return;
-
-    const VcsBaseClientSettings newSettings = optionsPageWidget->settings();
-    VcsBaseClientSettings &s = MercurialPlugin::instance()->client()->settings();
-    if (s != newSettings) {
-        s = newSettings;
-        s.writeSettings(Core::ICore::settings());
-        emit settingsChanged();
-    }
-
-}
-
-void OptionsPage::finish()
-{
-    delete optionsPageWidget;
 }
 
 } // namespace Internal

src/plugins/mercurial/optionspage.h

@@ -45,7 +45,7 @@ class VcsBaseClientSettings;
 namespace Mercurial {
 namespace Internal {
 
-class OptionsPageWidget : public QWidget
+class OptionsPageWidget : public VcsBase::VcsClientOptionsPageWidget
 {
     Q_OBJECT
 
@@ -60,22 +60,12 @@ private:
 };
 
 
-class OptionsPage : public VcsBase::VcsBaseOptionsPage
+class OptionsPage : public VcsBase::VcsClientOptionsPage
 {
     Q_OBJECT
 
 public:
-    OptionsPage();
+    OptionsPage(Core::IVersionControl *control);
-
-    QWidget *widget();
-    void apply();
-    void finish();
-
-signals:
-    void settingsChanged();
-
-private:
-    QPointer<OptionsPageWidget> optionsPageWidget;
 };
 
 } // namespace Internal

src/plugins/subversion/settingspage.cpp

@@ -47,8 +47,7 @@ using namespace Subversion::Internal;
 using namespace Utils;
 using namespace VcsBase;
 
-SettingsPageWidget::SettingsPageWidget(QWidget *parent) :
+SettingsPageWidget::SettingsPageWidget(QWidget *parent) : VcsClientOptionsPageWidget(parent)
-    QWidget(parent)
 {
     m_ui.setupUi(this);
     m_ui.pathChooser->setExpectedKind(PathChooser::ExistingCommand);
@@ -86,34 +85,10 @@ void SettingsPageWidget::setSettings(const VcsBaseClientSettings &s)
     m_ui.logCountSpinBox->setValue(s.intValue(SubversionSettings::logCountKey));
 }
 
-SettingsPage::SettingsPage() :
+SettingsPage::SettingsPage(Core::IVersionControl *control) :
-    m_widget(0)
+    VcsClientOptionsPage(control, SubversionPlugin::instance()->client())
 {
     setId(VcsBase::Constants::VCS_ID_SUBVERSION);
     setDisplayName(tr("Subversion"));
-}
+    setWidgetFactory([]() { return new SettingsPageWidget; });
-
-QWidget *SettingsPage::widget()
-{
-    if (!m_widget) {
-        m_widget = new SettingsPageWidget;
-        m_widget->setSettings(SubversionPlugin::instance()->client()->settings());
-    }
-    return m_widget;
-}
-
-void SettingsPage::apply()
-{
-    const VcsBaseClientSettings newSettings = m_widget->settings();
-    VcsBaseClientSettings &s = SubversionPlugin::instance()->client()->settings();
-    if (s != newSettings) {
-        s = newSettings;
-        s.writeSettings(Core::ICore::settings());
-        emit settingsChanged();
-    }
-}
-
-void SettingsPage::finish()
-{
-    delete m_widget;
 }

src/plugins/subversion/settingspage.h

@@ -43,14 +43,10 @@ QT_BEGIN_NAMESPACE
 class QSettings;
 QT_END_NAMESPACE
 
-namespace VcsBase {
-class VcsBaseClientSettings;
-} // namespace VcsBa
-
 namespace Subversion {
 namespace Internal {
 
-class SettingsPageWidget : public QWidget
+class SettingsPageWidget : public VcsBase::VcsClientOptionsPageWidget
 {
     Q_OBJECT
 
@@ -65,22 +61,12 @@ private:
 };
 
 
-class SettingsPage : public VcsBase::VcsBaseOptionsPage
+class SettingsPage : public VcsBase::VcsClientOptionsPage
 {
     Q_OBJECT
 
 public:
-    SettingsPage();
+    SettingsPage(Core::IVersionControl *control);
-
-    QWidget *widget();
-    void apply();
-    void finish();
-
-signals:
-    void settingsChanged();
-
-private:
-    QPointer<SettingsPageWidget> m_widget;
 };
 
 } // namespace Subversion

src/plugins/subversion/subversionplugin.cpp

@@ -249,7 +249,7 @@ bool SubversionPlugin::initialize(const QStringList & /*arguments */, QString *e
 
     m_client = new SubversionClient;
 
-    addAutoReleasedObject(new SettingsPage);
+    addAutoReleasedObject(new SettingsPage(versionControl()));
 
     addAutoReleasedObject(new VcsSubmitEditorFactory(&submitParameters,
         []() { return new SubversionSubmitEditor(&submitParameters); }));

src/plugins/vcsbase/vcsbaseoptionspage.cpp

@@ -30,8 +30,14 @@
 
 #include "vcsbaseoptionspage.h"
 
+#include "vcsbaseclient.h"
 #include "vcsbaseconstants.h"
 
+#include <coreplugin/icore.h>
+#include <coreplugin/iversioncontrol.h>
+
+#include <utils/qtcassert.h>
+
 #include <QCoreApplication>
 
 /*!
@@ -43,12 +49,58 @@
 
 namespace VcsBase {
 
-VcsBaseOptionsPage::VcsBaseOptionsPage(QObject *parent) :
+VcsBaseOptionsPage::VcsBaseOptionsPage(QObject *parent) : Core::IOptionsPage(parent)
-    Core::IOptionsPage(parent)
 {
     setCategory(Constants::VCS_SETTINGS_CATEGORY);
     setDisplayCategory(QCoreApplication::translate("VcsBase", Constants::VCS_SETTINGS_TR_CATEGORY));
     setCategoryIcon(QLatin1String(Constants::SETTINGS_CATEGORY_VCS_ICON));
 }
 
+VcsClientOptionsPageWidget::VcsClientOptionsPageWidget(QWidget *parent) : QWidget(parent)
+{ }
+
+VcsClientOptionsPage::VcsClientOptionsPage(Core::IVersionControl *control, VcsBaseClientImpl *client,
+                                           QObject *parent) :
+    VcsBaseOptionsPage(parent),
+    m_widget(0),
+    m_client(client)
+{
+    QTC_CHECK(m_client);
+    connect(this, &VcsClientOptionsPage::settingsChanged,
+            control, &Core::IVersionControl::configurationChanged);
+}
+
+void VcsClientOptionsPage::setWidgetFactory(VcsClientOptionsPage::WidgetFactory factory)
+{
+    QTC_ASSERT(!m_factory, return);
+    m_factory = factory;
+}
+
+VcsClientOptionsPageWidget *VcsClientOptionsPage::widget()
+{
+    QTC_ASSERT(m_factory, return 0);
+    if (!m_widget)
+        m_widget = m_factory();
+    QTC_ASSERT(m_widget, return 0);
+    m_widget->setSettings(m_client->settings());
+    return m_widget;
+}
+
+void VcsClientOptionsPage::apply()
+{
+    QTC_ASSERT(m_widget, return);
+    const VcsBaseClientSettings newSettings = m_widget->settings();
+    VcsBaseClientSettings &s = m_client->settings();
+    if (s != newSettings) {
+        s = newSettings;
+        emit settingsChanged();
+    }
+}
+
+void VcsClientOptionsPage::finish()
+{
+    delete m_widget;
+    m_widget = 0;
+}
+
 } // namespace VcsBase

src/plugins/vcsbase/vcsbaseoptionspage.h

@@ -33,8 +33,16 @@
 
 #include "vcsbase_global.h"
 
+#include "vcsbaseclientsettings.h"
+
 #include <coreplugin/dialogs/ioptionspage.h>
 
+#include <QWidget>
+
+#include <functional>
+
+namespace Core { class IVersionControl; }
+
 namespace VcsBase {
 
 class VCSBASE_EXPORT VcsBaseOptionsPage : public Core::IOptionsPage
@@ -43,6 +51,44 @@ public:
     explicit VcsBaseOptionsPage(QObject *parent = 0);
 };
 
+class VcsBaseClientImpl;
+
+class VCSBASE_EXPORT VcsClientOptionsPageWidget : public QWidget
+{
+    Q_OBJECT
+
+public:
+    VcsClientOptionsPageWidget(QWidget *parent = 0);
+
+    virtual void setSettings(const VcsBaseClientSettings &s) = 0;
+    virtual VcsBaseClientSettings settings() const = 0;
+};
+
+class VCSBASE_EXPORT VcsClientOptionsPage : public VcsBaseOptionsPage
+{
+    Q_OBJECT
+
+public:
+    using WidgetFactory = std::function<VcsClientOptionsPageWidget *()>;
+
+    explicit VcsClientOptionsPage(Core::IVersionControl *control, VcsBaseClientImpl *client, QObject *parent = 0);
+
+    VcsClientOptionsPageWidget *widget();
+    virtual void apply();
+    virtual void finish();
+
+signals:
+    void settingsChanged();
+
+protected:
+    void setWidgetFactory(WidgetFactory factory);
+
+private:
+    WidgetFactory m_factory;
+    VcsClientOptionsPageWidget *m_widget;
+    VcsBaseClientImpl *const m_client;
+};
+
 } // namespace VcsBase
 
 #endif // VCSBASEOPTIONSPAGE_H