refactor: Make Args less of a singleton (Chatterino#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 @@ class EmptyApplication : public IApplication
     {
         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)
-    : themes(&this->emplace<Theme>())
+Application::Application(Settings &_settings, Paths &_paths, const Args &_args)
+    : 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 @@ class IApplication
 
     static IApplication *instance;
 
+    virtual const Args &getArgs() = 0;
     virtual Theme *getThemes() = 0;
     virtual Fonts *getFonts() = 0;
     virtual IEmotes *getEmotes() = 0;
@@ -78,14 +80,15 @@ class IApplication
 
 class Application : public IApplication
 {
+    const Args &args_;
     std::vector<std::unique_ptr<Singleton>> singletons_;
     int argc_{};
     char **argv_{};
 
 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 @@ class Application : public IApplication
 
     /*[[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 @@ class Args
     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 @@ class CrashHandler : public Singleton
 };
 
 #ifdef CHATTERINO_WITH_CRASHPAD
-std::unique_ptr<crashpad::CrashpadClient> installCrashHandler();
+std::unique_ptr<crashpad::CrashpadClient> installCrashHandler(const Args &args);
 #endif
 
 }  // namespace chatterino