fix: Fixed account switch not being saved if no other settings were changed (#5558)

CHANGELOG.md

@@ -42,6 +42,7 @@
 - Bugfix: Links with invalid characters in the domain are no longer detected. (#5509)
 - Bugfix: Fixed janky selection for messages with RTL segments (selection is still wrong, but consistently wrong). (#5525)
 - Bugfix: Fixed tab visibility being controllable in the emote popup. (#5530)
+- Bugfix: Fixed account switch not being saved if no other settings were changed. (#5558)
 - Dev: Update Windows build from Qt 6.5.0 to Qt 6.7.1. (#5420)
 - Dev: Update vcpkg build Qt from 6.5.0 to 6.7.0, boost from 1.83.0 to 1.85.0, openssl from 3.1.3 to 3.3.0. (#5422)
 - Dev: Unsingletonize `ISoundController`. (#5462)

benchmarks/src/Highlights.cpp

@@ -6,7 +6,6 @@
 #include "messages/Message.hpp"
 #include "messages/MessageBuilder.hpp"
 #include "mocks/BaseApplication.hpp"
-#include "singletons/Settings.hpp"
 #include "util/Helpers.hpp"
 
 #include <benchmark/benchmark.h>
@@ -72,8 +71,6 @@ public:
 static void BM_HighlightTest(benchmark::State &state)
 {
     MockApplication mockApplication;
-    QTemporaryDir settingsDir;
-    Settings settings(settingsDir.path());
 
     std::string message =
         R"(@badge-info=subscriber/34;badges=moderator/1,subscriber/24;color=#FF0000;display-name=테스트계정420;emotes=41:6-13,15-22;flags=;id=a3196c7e-be4c-4b49-9c5a-8b8302b50c2a;mod=1;room-id=11148817;subscriber=1;tmi-sent-ts=1590922213730;turbo=0;user-id=117166826;user-type=mod :testaccount_420!testaccount_420@testaccount_420.tmi.twitch.tv PRIVMSG #pajlada :-tags Kreygasm,Kreygasm (no space))";

benchmarks/src/main.cpp

@@ -1,3 +1,4 @@
+#include "common/Args.hpp"
 #include "singletons/Resources.hpp"
 #include "singletons/Settings.hpp"
 
@@ -16,10 +17,12 @@ int main(int argc, char **argv)
 
     ::benchmark::Initialize(&argc, argv);
 
+    Args args;
+
     // Ensure settings are initialized before any benchmarks are run
     QTemporaryDir settingsDir;
     settingsDir.setAutoRemove(false);  // we'll remove it manually
-    chatterino::Settings settings(settingsDir.path());
+    chatterino::Settings settings(args, settingsDir.path());
 
     QTimer::singleShot(0, [&]() {
         ::benchmark::RunSpecifiedBenchmarks();

mocks/include/mocks/BaseApplication.hpp

@@ -1,5 +1,6 @@
 #pragma once
 
+#include "common/Args.hpp"
 #include "mocks/DisabledStreamerMode.hpp"
 #include "mocks/EmptyApplication.hpp"
 #include "providers/bttv/BttvLiveUpdates.hpp"
@@ -16,13 +17,13 @@ class BaseApplication : public EmptyApplication
 {
 public:
     BaseApplication()
-        : settings(this->settingsDir.filePath("settings.json"))
+        : settings(this->args, this->settingsDir.path())
     {
     }
 
     explicit BaseApplication(const QString &settingsData)
         : EmptyApplication(settingsData)
-        , settings(this->settingsDir.filePath("settings.json"))
+        , settings(this->args, this->settingsDir.path())
     {
     }
 
@@ -41,6 +42,7 @@ public:
         return nullptr;
     }
 
+    Args args;
     Settings settings;
     DisabledStreamerMode streamerMode;
 };

src/RunGui.cpp

@@ -266,10 +266,7 @@ void runGui(QApplication &a, const Paths &paths, Settings &settings,
     app.run(a);
     app.save();
 
-    if (!args.dontSaveSettings)
+    settings.requestSave();
-    {
-        pajlada::Settings::SettingManager::gSave();
-    }
 
     chatterino::NetworkManager::deinit();
 

src/main.cpp

@@ -117,7 +117,7 @@ int main(int argc, char **argv)
         IvrApi::initialize();
         Helix::initialize();
 
-        Settings settings(paths->settingsDirectory);
+        Settings settings(args, paths->settingsDirectory);
 
         runGui(a, *paths, settings, args, updates);
     }

src/singletons/Settings.cpp

@@ -1,6 +1,7 @@
 #include "singletons/Settings.hpp"
 
 #include "Application.hpp"
+#include "common/Args.hpp"
 #include "controllers/filters/FilterRecord.hpp"
 #include "controllers/highlights/HighlightBadge.hpp"
 #include "controllers/highlights/HighlightBlacklistUser.hpp"
@@ -140,8 +141,9 @@ bool Settings::toggleMutedChannel(const QString &channelName)
 
 Settings *Settings::instance_ = nullptr;
 
-Settings::Settings(const QString &settingsDirectory)
+Settings::Settings(const Args &args, const QString &settingsDirectory)
     : prevInstance_(Settings::instance_)
+    , disableSaving(args.dontSaveSettings)
 {
     QString settingsPath = settingsDirectory + "/settings.json";
 
@@ -153,7 +155,7 @@ Settings::Settings(const QString &settingsDirectory)
     settingsInstance->setBackupEnabled(true);
     settingsInstance->setBackupSlots(9);
     settingsInstance->saveMethod =
-        pajlada::Settings::SettingManager::SaveMethod::SaveOnExit;
+        pajlada::Settings::SettingManager::SaveMethod::SaveManually;
 
     initializeSignalVector(this->signalHolder, this->highlightedMessagesSetting,
                            this->highlightedMessages);
@@ -193,6 +195,16 @@ Settings::~Settings()
     Settings::instance_ = this->prevInstance_;
 }
 
+void Settings::requestSave() const
+{
+    if (this->disableSaving)
+    {
+        return;
+    }
+
+    pajlada::Settings::SettingManager::gSave();
+}
+
 void Settings::saveSnapshot()
 {
     BenchmarkGuard benchmark("Settings::saveSnapshot");

src/singletons/Settings.hpp

@@ -25,6 +25,8 @@ using TimeoutButton = std::pair<QString, int>;
 
 namespace chatterino {
 
+class Args;
+
 #ifdef Q_OS_WIN32
 #    define DEFAULT_FONT_FAMILY "Segoe UI"
 #    define DEFAULT_FONT_SIZE 10
@@ -80,12 +82,19 @@ class Settings
     static Settings *instance_;
     Settings *prevInstance_ = nullptr;
 
+    const bool disableSaving;
+
 public:
-    Settings(const QString &settingsDirectory);
+    Settings(const Args &args, const QString &settingsDirectory);
     ~Settings();
 
     static Settings &instance();
 
+    /// Request the settings to be saved to file
+    ///
+    /// Depending on the launch options, a save might end up not happening
+    void requestSave() const;
+
     void saveSnapshot();
     void restoreSnapshot();
 

src/widgets/AccountSwitchWidget.cpp

@@ -5,6 +5,7 @@
 #include "controllers/accounts/AccountController.hpp"
 #include "providers/twitch/TwitchAccount.hpp"
 #include "providers/twitch/TwitchCommon.hpp"
+#include "singletons/Settings.hpp"
 
 namespace chatterino {
 
@@ -54,6 +55,8 @@ AccountSwitchWidget::AccountSwitchWidget(QWidget *parent)
             {
                 app->getAccounts()->twitch.currentUsername = newUsername;
             }
+
+            getSettings()->requestSave();
         }
     });
 }

src/widgets/dialogs/SettingsDialog.cpp

@@ -435,8 +435,10 @@ void SettingsDialog::onOkClicked()
     if (!getApp()->getArgs().dontSaveSettings)
     {
         getApp()->getCommands()->save();
-        pajlada::Settings::SettingManager::gSave();
     }
+
+    getSettings()->requestSave();
+
     this->close();
 }
 

tests/src/Filters.cpp

@@ -4,6 +4,7 @@
 #include "controllers/filters/lang/Types.hpp"
 #include "controllers/highlights/HighlightController.hpp"
 #include "messages/MessageBuilder.hpp"
+#include "mocks/BaseApplication.hpp"
 #include "mocks/Channel.hpp"
 #include "mocks/ChatterinoBadges.hpp"
 #include "mocks/EmptyApplication.hpp"
@@ -26,12 +27,11 @@ TypingContext typingContext = MESSAGE_TYPING_CONTEXT;
 
 namespace {
 
-class MockApplication : mock::EmptyApplication
+class MockApplication : public mock::BaseApplication
 {
 public:
     MockApplication()
-        : settings(this->settingsDir.filePath("settings.json"))
+        : highlights(this->settings, &this->accounts)
-        , highlights(this->settings, &this->accounts)
     {
     }
 
@@ -75,7 +75,6 @@ public:
         return &this->highlights;
     }
 
-    Settings settings;
     AccountController accounts;
     Emotes emotes;
     mock::UserDataController userData;

tests/src/HighlightController.cpp

@@ -3,12 +3,11 @@
 #include "controllers/accounts/AccountController.hpp"
 #include "controllers/highlights/HighlightPhrase.hpp"
 #include "messages/MessageBuilder.hpp"  // for MessageParseArgs
-#include "mocks/EmptyApplication.hpp"
+#include "mocks/BaseApplication.hpp"
 #include "mocks/Helix.hpp"
 #include "mocks/UserData.hpp"
 #include "providers/twitch/api/Helix.hpp"
 #include "providers/twitch/TwitchBadge.hpp"  // for Badge
-#include "singletons/Settings.hpp"
 #include "Test.hpp"
 
 #include <QDebug>
@@ -22,12 +21,11 @@ using ::testing::Exactly;
 
 namespace {
 
-class MockApplication : public mock::EmptyApplication
+class MockApplication : public mock::BaseApplication
 {
 public:
     MockApplication(const QString &settingsBody)
-        : mock::EmptyApplication(settingsBody)
+        : mock::BaseApplication(settingsBody)
-        , settings(this->settingsDir.path())
         , highlights(this->settings, &this->accounts)
     {
     }
@@ -47,7 +45,6 @@ public:
         return &this->userData;
     }
 
-    Settings settings;
     AccountController accounts;
     HighlightController highlights;
     mock::UserDataController userData;

tests/src/InputCompletion.cpp

@@ -5,8 +5,8 @@
 #include "controllers/completion/strategies/ClassicUserStrategy.hpp"
 #include "controllers/completion/strategies/Strategy.hpp"
 #include "messages/Emote.hpp"
+#include "mocks/BaseApplication.hpp"
 #include "mocks/Channel.hpp"
-#include "mocks/EmptyApplication.hpp"
 #include "mocks/Helix.hpp"
 #include "mocks/TwitchIrcServer.hpp"
 #include "singletons/Emotes.hpp"
@@ -31,9 +31,14 @@ namespace {
 using namespace chatterino::completion;
 using ::testing::Exactly;
 
-class MockApplication : mock::EmptyApplication
+class MockApplication : public mock::BaseApplication
 {
 public:
+    explicit MockApplication(const QString &settingsData)
+        : BaseApplication(settingsData)
+    {
+    }
+
     AccountController *getAccounts() override
     {
         return &this->accounts;
@@ -110,24 +115,14 @@ class InputCompletionTest : public ::testing::Test
 protected:
     void SetUp() override
     {
-        // Write default settings to the mock settings json file
-        this->settingsDir_ = std::make_unique<QTemporaryDir>();
-
-        QFile settingsFile(this->settingsDir_->filePath("settings.json"));
-        ASSERT_TRUE(settingsFile.open(QIODevice::WriteOnly | QIODevice::Text));
-        ASSERT_GT(settingsFile.write(DEFAULT_SETTINGS.toUtf8()), 0);
-        ASSERT_TRUE(settingsFile.flush());
-        settingsFile.close();
-
         // Initialize helix client
         this->mockHelix = std::make_unique<mock::Helix>();
         initializeHelix(this->mockHelix.get());
         EXPECT_CALL(*this->mockHelix, loadBlocks).Times(Exactly(1));
         EXPECT_CALL(*this->mockHelix, update).Times(Exactly(1));
 
-        this->mockApplication = std::make_unique<MockApplication>();
+        this->mockApplication =
-        this->settings = std::make_unique<Settings>(this->settingsDir_->path());
+            std::make_unique<MockApplication>(DEFAULT_SETTINGS);
-        this->paths = std::make_unique<Paths>();
 
         this->mockApplication->accounts.load();
 
@@ -139,19 +134,11 @@ protected:
     void TearDown() override
     {
         this->mockApplication.reset();
-        this->settings.reset();
-        this->paths.reset();
         this->mockHelix.reset();
         this->channelPtr.reset();
-
-        this->settingsDir_.reset();
     }
 
-    std::unique_ptr<QTemporaryDir> settingsDir_;
-
     std::unique_ptr<MockApplication> mockApplication;
-    std::unique_ptr<Settings> settings;
-    std::unique_ptr<Paths> paths;
     std::unique_ptr<mock::Helix> mockHelix;
 
     ChannelPtr channelPtr;

tests/src/MessageBuilder.cpp

@@ -3,10 +3,10 @@
 #include "controllers/accounts/AccountController.hpp"
 #include "controllers/highlights/HighlightController.hpp"
 #include "controllers/ignores/IgnorePhrase.hpp"
+#include "mocks/BaseApplication.hpp"
 #include "mocks/Channel.hpp"
 #include "mocks/ChatterinoBadges.hpp"
 #include "mocks/DisabledStreamerMode.hpp"
-#include "mocks/EmptyApplication.hpp"
 #include "mocks/TwitchIrcServer.hpp"
 #include "mocks/UserData.hpp"
 #include "providers/ffz/FfzBadges.hpp"
@@ -28,12 +28,11 @@ using chatterino::mock::MockChannel;
 
 namespace {
 
-class MockApplication : mock::EmptyApplication
+class MockApplication : public mock::BaseApplication
 {
 public:
     MockApplication()
-        : settings(this->settingsDir.filePath("settings.json"))
+        : highlights(this->settings, &this->accounts)
-        , highlights(this->settings, &this->accounts)
     {
     }
 
@@ -92,12 +91,6 @@ public:
         return &this->seventvEmotes;
     }
 
-    IStreamerMode *getStreamerMode() override
-    {
-        return &this->streamerMode;
-    }
-
-    Settings settings;
     AccountController accounts;
     Emotes emotes;
     mock::UserDataController userData;
@@ -109,7 +102,6 @@ public:
     BttvEmotes bttvEmotes;
     FfzEmotes ffzEmotes;
     SeventvEmotes seventvEmotes;
-    DisabledStreamerMode streamerMode;
 };
 
 }  // namespace

tests/src/ModerationAction.cpp

@@ -1,10 +1,9 @@
 #include "controllers/moderationactions/ModerationAction.hpp"
 
 #include "messages/Image.hpp"
-#include "mocks/EmptyApplication.hpp"
+#include "mocks/BaseApplication.hpp"
 #include "singletons/Emotes.hpp"
 #include "singletons/Resources.hpp"
-#include "singletons/Settings.hpp"
 #include "Test.hpp"
 
 #include <QString>
@@ -15,20 +14,16 @@ using namespace std::chrono_literals;
 
 namespace {
 
-class MockApplication : mock::EmptyApplication
+class MockApplication : public mock::BaseApplication
 {
 public:
-    MockApplication()
+    MockApplication() = default;
-        : settings(this->settingsDir.filePath("settings.json"))
-    {
-    }
 
     IEmotes *getEmotes() override
     {
         return &this->emotes;
     }
 
-    Settings settings;
     Emotes emotes;
 };
 

tests/src/NotebookTab.cpp

@@ -3,9 +3,8 @@
 #include "common/Literals.hpp"
 #include "controllers/hotkeys/HotkeyController.hpp"
 #include "gmock/gmock.h"
-#include "mocks/EmptyApplication.hpp"
+#include "mocks/BaseApplication.hpp"
 #include "singletons/Fonts.hpp"
-#include "singletons/Settings.hpp"
 #include "singletons/Theme.hpp"
 #include "Test.hpp"
 #include "widgets/Notebook.hpp"
@@ -18,12 +17,11 @@ using ::testing::Exactly;
 
 namespace {
 
-class MockApplication : mock::EmptyApplication
+class MockApplication : public mock::BaseApplication
 {
 public:
     MockApplication()
-        : settings(this->settingsDir.filePath("settings.json"))
+        : theme(this->paths_)
-        , theme(this->paths_)
         , fonts(this->settings)
     {
     }
@@ -42,7 +40,6 @@ public:
         return &this->fonts;
     }
 
-    Settings settings;
     Theme theme;
     HotkeyController hotkeys;
     Fonts fonts;

tests/src/main.cpp

@@ -1,3 +1,4 @@
+#include "common/Args.hpp"
 #include "common/network/NetworkManager.hpp"
 #include "singletons/Resources.hpp"
 #include "singletons/Settings.hpp"
@@ -29,10 +30,12 @@ int main(int argc, char **argv)
 
     chatterino::NetworkManager::init();
 
+    Args args;
+
     // Ensure settings are initialized before any tests are run
     QTemporaryDir settingsDir;
     settingsDir.setAutoRemove(false);  // we'll remove it manually
-    chatterino::Settings settings(settingsDir.path());
+    chatterino::Settings settings(args, settingsDir.path());
 
     QTimer::singleShot(0, [&]() {
         auto res = RUN_ALL_TESTS();