refactor: Make Args less of a singleton (#5041)

This means it's no longer a singleton, and its lifetime is bound to our application.
This felt like a good small experiment to see how its changes would look
if we did this.
As a shortcut, `getApp` that is already a mega singleton keeps a
reference to Args, this means places that are a bit more difficult to
inject into call `getApp()->getArgs()` just like other things are
accessed.

CHANGELOG.md

@@ -100,6 +100,7 @@
 - Dev: BREAKING: Replace custom `import()` with normal Lua `require()`. (#5014)
 - Dev: Fixed most compiler warnings. (#5028)
 - Dev: Added the ability to show `ChannelView`s without a `Split`. (#4747)
+- Dev: Refactor Args to be less of a singleton. (#5041)
 - Dev: Channels without any animated elements on screen will skip updates from the GIF timer. (#5042, #5043, #5045)
 
 ## 2.4.6

mocks/include/mocks/EmptyApplication.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include "Application.hpp"
+#include "common/Args.hpp"
 
 namespace chatterino::mock {
 
@@ -9,6 +10,11 @@ class EmptyApplication : public IApplication
 public:
     virtual ~EmptyApplication() = default;
 
+    const Args &getArgs() override
+    {
+        return this->args_;
+    }
+
     Theme *getThemes() override
     {
         return nullptr;
@@ -110,6 +116,9 @@ public:
     {
         return nullptr;
     }
+
+private:
+    Args args_;
 };
 
 }  // namespace chatterino::mock

src/Application.cpp

@@ -104,8 +104,9 @@ IApplication::IApplication()
 // It will create the instances of the major classes, and connect their signals
 // to each other
 
-Application::Application(Settings &_settings, Paths &_paths)
+Application::Application(Settings &_settings, Paths &_paths, const Args &_args)
-    : themes(&this->emplace<Theme>())
+    : args_(_args)
+    , themes(&this->emplace<Theme>())
     , fonts(&this->emplace<Fonts>())
     , emotes(&this->emplace<Emotes>())
     , accounts(&this->emplace<AccountController>())
@@ -146,7 +147,7 @@ void Application::initialize(Settings &settings, Paths &paths)
     isAppInitialized = true;
 
     // Show changelog
-    if (!getArgs().isFramelessEmbed &&
+    if (!this->args_.isFramelessEmbed &&
         getSettings()->currentVersion.getValue() != "" &&
         getSettings()->currentVersion.getValue() != CHATTERINO_VERSION)
     {
@@ -161,7 +162,7 @@ void Application::initialize(Settings &settings, Paths &paths)
         }
     }
 
-    if (!getArgs().isFramelessEmbed)
+    if (!this->args_.isFramelessEmbed)
     {
         getSettings()->currentVersion.setValue(CHATTERINO_VERSION);
 
@@ -179,7 +180,7 @@ void Application::initialize(Settings &settings, Paths &paths)
     // Show crash message.
     // On Windows, the crash message was already shown.
 #ifndef Q_OS_WIN
-    if (!getArgs().isFramelessEmbed && getArgs().crashRecovery)
+    if (!this->args_.isFramelessEmbed && this->args_.crashRecovery)
     {
         if (auto selected =
                 this->windows->getMainWindow().getNotebook().getSelectedPage())
@@ -203,7 +204,7 @@ void Application::initialize(Settings &settings, Paths &paths)
 
     this->windows->updateWordTypeMask();
 
-    if (!getArgs().isFramelessEmbed)
+    if (!this->args_.isFramelessEmbed)
     {
         this->initNm(paths);
     }
@@ -219,7 +220,7 @@ int Application::run(QApplication &qtApp)
 
     this->twitch->connect();
 
-    if (!getArgs().isFramelessEmbed)
+    if (!this->args_.isFramelessEmbed)
     {
         this->windows->getMainWindow().show();
     }

src/Application.hpp

@@ -11,6 +11,7 @@
 
 namespace chatterino {
 
+class Args;
 class TwitchIrcServer;
 class ITwitchIrcServer;
 class PubSub;
@@ -54,6 +55,7 @@ public:
 
     static IApplication *instance;
 
+    virtual const Args &getArgs() = 0;
     virtual Theme *getThemes() = 0;
     virtual Fonts *getFonts() = 0;
     virtual IEmotes *getEmotes() = 0;
@@ -78,6 +80,7 @@ public:
 
 class Application : public IApplication
 {
+    const Args &args_;
     std::vector<std::unique_ptr<Singleton>> singletons_;
     int argc_{};
     char **argv_{};
@@ -85,7 +88,7 @@ class Application : public IApplication
 public:
     static Application *instance;
 
-    Application(Settings &settings, Paths &paths);
+    Application(Settings &_settings, Paths &_paths, const Args &_args);
 
     void initialize(Settings &settings, Paths &paths);
     void load();
@@ -126,6 +129,10 @@ public:
 
     /*[[deprecated]]*/ Logging *const logging{};
 
+    const Args &getArgs() override
+    {
+        return this->args_;
+    }
     Theme *getThemes() override
     {
         return this->themes;

src/RunGui.cpp

@@ -98,9 +98,9 @@ namespace {
         installCustomPalette();
     }
 
-    void showLastCrashDialog()
+    void showLastCrashDialog(const Args &args)
     {
-        auto *dialog = new LastRunCrashDialog;
+        auto *dialog = new LastRunCrashDialog(args);
         // Use exec() over open() to block the app from being loaded
         // and to be able to set the safe mode.
         dialog->exec();
@@ -223,16 +223,16 @@ namespace {
     }
 }  // namespace
 
-void runGui(QApplication &a, Paths &paths, Settings &settings)
+void runGui(QApplication &a, Paths &paths, Settings &settings, const Args &args)
 {
     initQt();
     initResources();
     initSignalHandler();
 
 #ifdef Q_OS_WIN
-    if (getArgs().crashRecovery)
+    if (args.crashRecovery)
     {
-        showLastCrashDialog();
+        showLastCrashDialog(args);
     }
 #endif
 
@@ -271,12 +271,12 @@ void runGui(QApplication &a, Paths &paths, Settings &settings)
     chatterino::NetworkManager::init();
     chatterino::Updates::instance().checkForUpdates();
 
-    Application app(settings, paths);
+    Application app(settings, paths, args);
     app.initialize(settings, paths);
     app.run(a);
     app.save();
 
-    if (!getArgs().dontSaveSettings)
+    if (!args.dontSaveSettings)
     {
         pajlada::Settings::SettingManager::gSave();
     }

src/RunGui.hpp

@@ -3,8 +3,12 @@
 class QApplication;
 
 namespace chatterino {
+
+class Args;
 class Paths;
 class Settings;
 
-void runGui(QApplication &a, Paths &paths, Settings &settings);
+void runGui(QApplication &a, Paths &paths, Settings &settings,
+            const Args &args);
+
 }  // namespace chatterino

src/common/Args.cpp

@@ -1,4 +1,4 @@
-#include "Args.hpp"
+#include "common/Args.hpp"
 
 #include "common/QLogging.hpp"
 #include "debug/AssertInGuiThread.hpp"
@@ -248,18 +248,4 @@ void Args::applyCustomChannelLayout(const QString &argValue)
     }
 }
 
-static Args *instance = nullptr;
-
-void initArgs(const QApplication &app)
-{
-    instance = new Args(app);
-}
-
-const Args &getArgs()
-{
-    assert(instance);
-
-    return *instance;
-}
-
 }  // namespace chatterino

src/common/Args.hpp

@@ -30,6 +30,7 @@ namespace chatterino {
 class Args
 {
 public:
+    Args() = default;
     Args(const QApplication &app);
 
     bool printVersion{};
@@ -60,7 +61,4 @@ private:
     QStringList currentArguments_;
 };
 
-void initArgs(const QApplication &app);
-const Args &getArgs();
-
 }  // namespace chatterino

src/controllers/plugins/PluginController.cpp

@@ -233,7 +233,7 @@ void PluginController::load(const QFileInfo &index, const QDir &pluginDir,
     auto *temp = plugin.get();
     this->plugins_.insert({pluginName, std::move(plugin)});
 
-    if (getArgs().safeMode)
+    if (getApp()->getArgs().safeMode)
     {
         // This isn't done earlier to ensure the user can disable a misbehaving plugin
         qCWarning(chatterinoLua) << "Skipping loading plugin " << meta.name

src/main.cpp

@@ -62,18 +62,18 @@ int main(int argc, char **argv)
         return 1;
     }
 
-    initArgs(a);
+    const Args args(a);
 
 #ifdef CHATTERINO_WITH_CRASHPAD
-    const auto crashpadHandler = installCrashHandler();
+    const auto crashpadHandler = installCrashHandler(args);
 #endif
 
     // run in gui mode or browser extension host mode
-    if (getArgs().shouldRunBrowserExtensionHost)
+    if (args.shouldRunBrowserExtensionHost)
     {
         runBrowserExtensionHost();
     }
-    else if (getArgs().printVersion)
+    else if (args.printVersion)
     {
         attachToConsole();
 
@@ -87,7 +87,7 @@ int main(int argc, char **argv)
     }
     else
     {
-        if (getArgs().verbose)
+        if (args.verbose)
         {
             attachToConsole();
         }
@@ -99,7 +99,7 @@ int main(int argc, char **argv)
 
         Settings settings(paths->settingsDirectory);
 
-        runGui(a, *paths, settings);
+        runGui(a, *paths, settings, args);
     }
     return 0;
 }

src/singletons/CrashHandler.cpp

@@ -82,10 +82,9 @@ std::optional<bool> readRecoverySettings(const Paths &paths)
     return shouldRecover.toBool();
 }
 
-bool canRestart(const Paths &paths)
+bool canRestart(const Paths &paths, const Args &args)
 {
 #ifdef NDEBUG
-    const auto &args = chatterino::getArgs();
     if (args.isFramelessEmbed || args.shouldRunBrowserExtensionHost)
     {
         return false;
@@ -109,10 +108,10 @@ bool canRestart(const Paths &paths)
 /// additional plus ('++' -> '+').
 ///
 /// The decoding happens in crash-handler/src/CommandLine.cpp
-std::string encodeArguments()
+std::string encodeArguments(const Args &appArgs)
 {
     std::string args;
-    for (auto arg : getArgs().currentArguments())
+    for (auto arg : appArgs.currentArguments())
     {
         if (!args.empty())
         {
@@ -161,7 +160,7 @@ void CrashHandler::saveShouldRecover(bool value)
 }
 
 #ifdef CHATTERINO_WITH_CRASHPAD
-std::unique_ptr<crashpad::CrashpadClient> installCrashHandler()
+std::unique_ptr<crashpad::CrashpadClient> installCrashHandler(const Args &args)
 {
     // Currently, the following directory layout is assumed:
     // [applicationDirPath]
@@ -197,7 +196,7 @@ std::unique_ptr<crashpad::CrashpadClient> installCrashHandler()
     std::map<std::string, std::string> annotations{
         {
             "canRestart"s,
-            canRestart(*getPaths()) ? "true"s : "false"s,
+            canRestart(*getPaths(), args) ? "true"s : "false"s,
         },
         {
             "exePath"s,
@@ -209,7 +208,7 @@ std::unique_ptr<crashpad::CrashpadClient> installCrashHandler()
         },
         {
             "exeArguments"s,
-            encodeArguments(),
+            encodeArguments(args),
         },
     };
 

src/singletons/CrashHandler.hpp

@@ -12,6 +12,8 @@
 
 namespace chatterino {
 
+class Args;
+
 class CrashHandler : public Singleton
 {
 public:
@@ -30,7 +32,7 @@ private:
 };
 
 #ifdef CHATTERINO_WITH_CRASHPAD
-std::unique_ptr<crashpad::CrashpadClient> installCrashHandler();
+std::unique_ptr<crashpad::CrashpadClient> installCrashHandler(const Args &args);
 #endif
 
 }  // namespace chatterino

src/singletons/WindowManager.cpp

@@ -55,7 +55,7 @@ using SplitDirection = SplitContainer::Direction;
 void WindowManager::showSettingsDialog(QWidget *parent,
                                        SettingsDialogPreference preference)
 {
-    if (getArgs().dontSaveSettings)
+    if (getApp()->getArgs().dontSaveSettings)
     {
         QMessageBox::critical(parent, "Chatterino - Editing Settings Forbidden",
                               "Settings cannot be edited when running with\n"
@@ -356,9 +356,9 @@ void WindowManager::initialize(Settings &settings, Paths &paths)
     {
         WindowLayout windowLayout;
 
-        if (getArgs().customChannelLayout)
+        if (getApp()->getArgs().customChannelLayout)
         {
-            windowLayout = getArgs().customChannelLayout.value();
+            windowLayout = getApp()->getArgs().customChannelLayout.value();
         }
         else
         {
@@ -370,7 +370,7 @@ void WindowManager::initialize(Settings &settings, Paths &paths)
         this->applyWindowLayout(windowLayout);
     }
 
-    if (getArgs().isFramelessEmbed)
+    if (getApp()->getArgs().isFramelessEmbed)
     {
         this->framelessEmbedWindow_.reset(new FramelessEmbedWindow);
         this->framelessEmbedWindow_->show();
@@ -383,7 +383,7 @@ void WindowManager::initialize(Settings &settings, Paths &paths)
         this->mainWindow_->getNotebook().addPage(true);
 
         // TODO: don't create main window if it's a frameless embed
-        if (getArgs().isFramelessEmbed)
+        if (getApp()->getArgs().isFramelessEmbed)
         {
             this->mainWindow_->hide();
         }
@@ -418,7 +418,7 @@ void WindowManager::initialize(Settings &settings, Paths &paths)
 
 void WindowManager::save()
 {
-    if (getArgs().dontSaveSettings)
+    if (getApp()->getArgs().dontSaveSettings)
     {
         return;
     }
@@ -724,7 +724,7 @@ WindowLayout WindowManager::loadWindowLayoutFromFile() const
 
 void WindowManager::applyWindowLayout(const WindowLayout &layout)
 {
-    if (getArgs().dontLoadMainWindow)
+    if (getApp()->getArgs().dontLoadMainWindow)
     {
         return;
     }

src/widgets/FramelessEmbedWindow.cpp

@@ -64,13 +64,13 @@ bool FramelessEmbedWindow::nativeEvent(const QByteArray &eventType,
 
 void FramelessEmbedWindow::showEvent(QShowEvent *)
 {
-    if (!getArgs().parentWindowId)
+    if (!getApp()->getArgs().parentWindowId)
     {
         return;
     }
 
     if (auto parentHwnd =
-            reinterpret_cast<HWND>(getArgs().parentWindowId.value()))
+            reinterpret_cast<HWND>(getApp()->getArgs().parentWindowId.value()))
     {
         auto handle = reinterpret_cast<HWND>(this->winId());
         if (!::SetParent(handle, parentHwnd))

src/widgets/Notebook.cpp

@@ -1347,7 +1347,7 @@ void SplitNotebook::addCustomButtons()
     auto settingsBtn = this->addCustomButton();
 
     // This is to ensure you can't lock yourself out of the settings
-    if (getArgs().safeMode)
+    if (getApp()->getArgs().safeMode)
     {
         settingsBtn->setVisible(true);
     }

src/widgets/Window.cpp

@@ -737,7 +737,7 @@ void Window::onAccountSelected()
     }
 #endif
 
-    if (getArgs().safeMode)
+    if (getApp()->getArgs().safeMode)
     {
         windowTitle += " (safe mode)";
     }

src/widgets/dialogs/LastRunCrashDialog.cpp

@@ -43,7 +43,7 @@ namespace chatterino {
 
 using namespace literals;
 
-LastRunCrashDialog::LastRunCrashDialog()
+LastRunCrashDialog::LastRunCrashDialog(const Args &args)
 {
     this->setWindowFlag(Qt::WindowContextHelpButtonHint, false);
     this->setWindowTitle(u"Chatterino - " % randomMessage());
@@ -62,14 +62,14 @@ LastRunCrashDialog::LastRunCrashDialog()
             "<a href=\"file:///" %
             reportsDir % u"\">" % reportsDir % u"</a>.<br>";
 
-    if (getArgs().exceptionCode)
+    if (args.exceptionCode)
     {
         text += u"The last run crashed with code <code>0x" %
-                QString::number(*getArgs().exceptionCode, 16) % u"</code>";
+                QString::number(*args.exceptionCode, 16) % u"</code>";
 
-        if (getArgs().exceptionMessage)
+        if (args.exceptionMessage)
         {
-            text += u" (" % *getArgs().exceptionMessage % u")";
+            text += u" (" % *args.exceptionMessage % u")";
         }
 
         text += u".<br>"_s;

src/widgets/dialogs/LastRunCrashDialog.hpp

@@ -4,10 +4,12 @@
 
 namespace chatterino {
 
+class Args;
+
 class LastRunCrashDialog : public QDialog
 {
 public:
-    LastRunCrashDialog();
+    explicit LastRunCrashDialog(const Args &args);
 };
 
 }  // namespace chatterino

src/widgets/dialogs/SettingsDialog.cpp

@@ -411,7 +411,7 @@ void SettingsDialog::showEvent(QShowEvent *e)
 ///// Widget creation helpers
 void SettingsDialog::onOkClicked()
 {
-    if (!getArgs().dontSaveSettings)
+    if (!getApp()->getArgs().dontSaveSettings)
     {
         getApp()->commands->save();
         pajlada::Settings::SettingManager::gSave();

src/widgets/settingspages/PluginsPage.cpp

@@ -53,7 +53,7 @@ PluginsPage::PluginsPage()
             this->rebuildContent();
         });
         groupLayout->addRow(box);
-        if (getArgs().safeMode)
+        if (getApp()->getArgs().safeMode)
         {
             box->setEnabled(false);
             auto *disabledLabel = new QLabel(this);
@@ -197,7 +197,7 @@ void PluginsPage::rebuildContent()
                              this->rebuildContent();
                          });
         pluginEntry->addRow(reloadButton);
-        if (getArgs().safeMode)
+        if (getApp()->getArgs().safeMode)
         {
             reloadButton->setEnabled(false);
         }