Fossil: Make settings more directly accessible

I am still not sure how the final pattern may ideally look like
but it looks like some kind of singleton access to a plugin's
settings helps with clarity.

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

src/plugins/fossil/fossilclient.cpp

@@ -55,9 +55,9 @@ public:
         addReloadButton();
         if (features.testFlag(FossilClient::DiffIgnoreWhiteSpaceFeature)) {
             mapSetting(addToggleButton("-w", Tr::tr("Ignore All Whitespace")),
-                       &client->settings().diffIgnoreAllWhiteSpace);
+                       &settings().diffIgnoreAllWhiteSpace);
             mapSetting(addToggleButton("--strip-trailing-cr", Tr::tr("Strip Trailing CR")),
-                       &client->settings().diffStripTrailingCR);
+                       &settings().diffStripTrailingCR);
         }
     }
 };
@@ -73,20 +73,19 @@ public:
     {
         QTC_ASSERT(client, return);
 
-        FossilSettings &settings = client->settings();
         FossilClient::SupportedFeatures features = client->supportedFeatures();
 
         if (features.testFlag(FossilClient::AnnotateBlameFeature)) {
             mapSetting(addToggleButton("|BLAME|", Tr::tr("Show Committers")),
-                       &settings.annotateShowCommitters);
+                       &settings().annotateShowCommitters);
         }
 
         // Force listVersions setting to false by default.
         // This way the annotated line number would not get offset by the version list.
-        settings.annotateListVersions.setValue(false);
+        settings().annotateListVersions.setValue(false);
 
         mapSetting(addToggleButton("--log", Tr::tr("List Versions")),
-                   &settings.annotateListVersions);
+                   &settings().annotateListVersions);
     }
 };
 
@@ -122,8 +121,6 @@ public:
 
     void addLineageComboBox()
     {
-        FossilSettings &settings = m_client->settings();
-
         // ancestors/descendants filter
         // This is a positional argument not an option.
         // Normally it takes the checkin/branch/tag as an additional parameter
@@ -137,23 +134,19 @@ public:
             ChoiceItem(Tr::tr("Unfiltered"), "")
         };
         mapSetting(addChoices(Tr::tr("Lineage"), QStringList("|LINEAGE|%1|current"), lineageFilterChoices),
-                   &settings.timelineLineageFilter);
+                   &settings().timelineLineageFilter);
     }
 
     void addVerboseToggleButton()
     {
-        FossilSettings &settings = m_client->settings();
-
         // show files
         mapSetting(addToggleButton("-showfiles", Tr::tr("Verbose"),
                                    Tr::tr("Show files changed in each revision")),
-                   &settings.timelineVerbose);
+                   &settings().timelineVerbose);
     }
 
     void addItemTypeComboBox()
     {
-        FossilSettings &settings = m_client->settings();
-
         // option: -t <val>
         const QList<ChoiceItem> itemTypeChoices = {
             ChoiceItem(Tr::tr("All Items"), "all"),
@@ -169,7 +162,7 @@ public:
         // Fossil expects separate arguments for option and value ( i.e. "-t" "all")
         // so we need to handle the splitting explicitly in arguments().
         mapSetting(addChoices(Tr::tr("Item Types"), QStringList("-t %1"), itemTypeChoices),
-                   &settings.timelineItemType);
+                   &settings().timelineItemType);
     }
 
     QStringList arguments() const final
@@ -224,19 +217,19 @@ QString FossilClient::makeVersionString(unsigned version)
                     .arg(versionPart(version));
 }
 
-FossilClient::FossilClient(FossilSettings *settings)
-    : VcsBaseClient(settings), m_settings(settings)
+FossilSettings &FossilClient::settings() const
+{
+    return Internal::settings();
+}
+
+FossilClient::FossilClient()
+    : VcsBaseClient(&Internal::settings())
 {
     setDiffConfigCreator([this](QToolBar *toolBar) {
         return new FossilDiffConfig(this, toolBar);
     });
 }
 
-FossilSettings &FossilClient::settings() const
-{
-    return *m_settings;
-}
-
 unsigned int FossilClient::synchronousBinaryVersion() const
 {
     if (settings().binaryPath.value().isEmpty())

src/plugins/fossil/fossilclient.h

@@ -41,7 +41,7 @@ public:
     static unsigned makeVersionNumber(int major, int minor, int patch);
     static QString makeVersionString(unsigned version);
 
-    explicit FossilClient(FossilSettings *settings);
+    FossilClient();
     FossilSettings &settings() const;
 
     unsigned int synchronousBinaryVersion() const;
@@ -107,7 +107,6 @@ private:
     VcsBase::VcsBaseEditorConfig *createLogEditor(VcsBase::VcsBaseEditorWidget *editor);
 
     friend class FossilPluginPrivate;
-    FossilSettings *m_settings = nullptr;
 };
 
 Q_DECLARE_OPERATORS_FOR_FLAGS(FossilClient::SupportedFeatures)

src/plugins/fossil/fossilplugin.cpp

@@ -187,10 +187,9 @@ public:
     bool pullOrPush(SyncMode mode);
 
     // Variables
-    FossilSettings m_fossilSettings;
-    FossilClient m_client{&m_fossilSettings};
+    FossilClient m_client;
 
-    OptionsPage optionPage{&m_fossilSettings};
+    OptionsPage optionPage;
 
     VcsSubmitEditorFactory submitEditorFactory {
         submitEditorParameters,
@@ -274,11 +273,6 @@ void FossilPlugin::extensionsInitialized()
     dd->extensionsInitialized();
 }
 
-const FossilSettings &FossilPlugin::settings()
-{
-    return dd->m_fossilSettings;
-}
-
 FossilClient *FossilPlugin::client()
 {
     return &dd->m_client;
@@ -296,11 +290,9 @@ FossilPluginPrivate::FossilPluginPrivate()
     m_commandLocator->setDescription(Tr::tr("Triggers a Fossil version control operation."));
 
     ProjectExplorer::JsonWizardFactory::addWizardPath(Utils::FilePath::fromString(Constants::WIZARD_PATH));
-    Core::JsExpander::registerGlobalObject("Fossil", [this] {
-        return new FossilJsExtension(&m_fossilSettings);
-    });
+    Core::JsExpander::registerGlobalObject("Fossil", [] { return new FossilJsExtension; });
 
-    connect(&m_fossilSettings, &AspectContainer::changed,
+    connect(&settings(), &AspectContainer::changed,
             this, &IVersionControl::configurationChanged);
 
     createMenu(context);

src/plugins/fossil/fossilplugin.h

@@ -3,8 +3,6 @@
 
 #pragma once
 
-#include "fossilsettings.h"
-
 #include <vcsbase/vcsbaseclient.h>
 #include <vcsbase/vcsbaseplugin.h>
 #include <coreplugin/icontext.h>
@@ -25,7 +23,6 @@ class FossilPlugin final : public ExtensionSystem::IPlugin
     void extensionsInitialized() final;
 
 public:
-    static const FossilSettings &settings();
     static FossilClient *client();
 
 #ifdef WITH_TESTS

src/plugins/fossil/fossilsettings.cpp

@@ -101,9 +101,9 @@ FossilSettings::FossilSettings()
 class OptionsPageWidget final : public Core::IOptionsPageWidget
 {
 public:
-    OptionsPageWidget(FossilSettings *settings)
+    OptionsPageWidget()
     {
-        FossilSettings &s = *settings;
+        FossilSettings &s = settings();
 
         using namespace Layouting;
 
@@ -141,16 +141,22 @@ public:
 
         }.attachTo(this);
 
-        setOnApply([settings] { settings->apply(); });
+        setOnApply([] { settings().apply(); });
     }
 };
 
-OptionsPage::OptionsPage(FossilSettings *settings)
+OptionsPage::OptionsPage()
 {
     setId(Constants::VCS_ID_FOSSIL);
     setDisplayName(Tr::tr("Fossil"));
-    setWidgetCreator([settings] { return new OptionsPageWidget(settings); });
+    setWidgetCreator([] { return new OptionsPageWidget; });
     setCategory(VcsBase::Constants::VCS_SETTINGS_CATEGORY);
 }
 
+FossilSettings &settings()
+{
+    static FossilSettings theSettings;
+    return theSettings;
+}
+
 } // Fossil::Internal

src/plugins/fossil/fossilsettings.h

@@ -26,6 +26,8 @@ public:
     Utils::BoolAspect disableAutosync;
 };
 
+FossilSettings &settings();
+
 struct RepositorySettings
 {
     enum AutosyncMode {AutosyncOff, AutosyncOn, AutosyncPullOnly};
@@ -45,7 +47,7 @@ struct RepositorySettings
 class OptionsPage : public Core::IOptionsPage
 {
 public:
-    explicit OptionsPage(FossilSettings *settings);
+    OptionsPage();
 };
 
 } // Fossil::Internal

src/plugins/fossil/wizard/fossiljsextension.cpp

@@ -17,19 +17,6 @@ using namespace Core;
 namespace Fossil {
 namespace Internal {
 
-class FossilJsExtensionPrivate {
-public:
-    FossilJsExtensionPrivate(FossilSettings *settings) :
-        m_vscId(Constants::VCS_ID_FOSSIL),
-        m_settings(settings)
-    {
-    }
-
-    Utils::Id m_vscId;
-    FossilSettings *m_settings;
-};
-
-
 QMap<QString, QString> FossilJsExtension::parseArgOptions(const QStringList &args)
 {
     QMap<QString, QString> options;
@@ -42,24 +29,19 @@ QMap<QString, QString> FossilJsExtension::parseArgOptions(const QStringList &arg
     return options;
 }
 
-FossilJsExtension::FossilJsExtension(FossilSettings *settings) :
-    d(new FossilJsExtensionPrivate(settings))
-{ }
+FossilJsExtension::FossilJsExtension() = default;
 
-FossilJsExtension::~FossilJsExtension()
-{
-    delete d;
-}
+FossilJsExtension::~FossilJsExtension() = default;
 
 bool FossilJsExtension::isConfigured() const
 {
-    IVersionControl *vc = VcsManager::versionControl(d->m_vscId);
+    IVersionControl *vc = VcsManager::versionControl(Constants::VCS_ID_FOSSIL);
     return vc && vc->isConfigured();
 }
 
 QString FossilJsExtension::displayName() const
 {
-    IVersionControl *vc = VcsManager::versionControl(d->m_vscId);
+    IVersionControl *vc = VcsManager::versionControl(Constants::VCS_ID_FOSSIL);
     return vc ? vc->displayName() : QString();
 }
 
@@ -68,7 +50,7 @@ QString FossilJsExtension::defaultAdminUser() const
     if (!isConfigured())
         return QString();
 
-    return d->m_settings->userName.value();
+    return settings().userName.value();
 }
 
 QString FossilJsExtension::defaultSslIdentityFile() const
@@ -76,7 +58,7 @@ QString FossilJsExtension::defaultSslIdentityFile() const
     if (!isConfigured())
         return QString();
 
-    return d->m_settings->sslIdentityFile.value();
+    return settings().sslIdentityFile.value();
 }
 
 QString FossilJsExtension::defaultLocalRepoPath() const
@@ -84,7 +66,7 @@ QString FossilJsExtension::defaultLocalRepoPath() const
     if (!isConfigured())
         return QString();
 
-    return d->m_settings->defaultRepoPath.value();
+    return settings().defaultRepoPath.value();
 }
 
 bool FossilJsExtension::defaultDisableAutosync() const
@@ -92,7 +74,7 @@ bool FossilJsExtension::defaultDisableAutosync() const
     if (!isConfigured())
         return false;
 
-    return d->m_settings->disableAutosync.value();
+    return settings().disableAutosync.value();
 }
 
 } // namespace Internal

src/plugins/fossil/wizard/fossiljsextension.h

@@ -12,9 +12,6 @@
 namespace Fossil {
 namespace Internal {
 
-class FossilJsExtensionPrivate;
-class FossilSettings;
-
 class FossilJsExtension : public QObject
 {
     Q_OBJECT
@@ -22,7 +19,7 @@ class FossilJsExtension : public QObject
 public:
     static QMap<QString, QString> parseArgOptions(const QStringList &args);
 
-    FossilJsExtension(FossilSettings *settings);
+    FossilJsExtension();
     ~FossilJsExtension();
 
     Q_INVOKABLE bool isConfigured() const;
@@ -31,9 +28,6 @@ public:
     Q_INVOKABLE QString defaultSslIdentityFile() const;
     Q_INVOKABLE QString defaultLocalRepoPath() const;
     Q_INVOKABLE bool defaultDisableAutosync() const;
-
-private:
-    FossilJsExtensionPrivate *d = nullptr;
 };
 
 } // namespace Internal