Make it so when custom themes can have the same name as built-in themes

src/singletons/Theme.cpp

@@ -178,14 +178,31 @@ std::optional<QJsonObject> loadTheme(const ThemeDescriptor &theme)
 
 namespace chatterino {
 
-const std::map<QString, ThemeDescriptor> Theme::builtInThemes{
-    {"White", {":/themes/White.json", false}},
-    {"Light", {":/themes/Light.json", false}},
-    {"Dark", {":/themes/Dark.json", false}},
-    {"Black", {":/themes/Black.json", false}},
+const std::vector<ThemeDescriptor> Theme::builtInThemes{
+    {
+        .key = "White",
+        .path = ":/themes/White.json",
+        .name = "White",
+    },
+    {
+        .key = "Light",
+        .path = ":/themes/Light.json",
+        .name = "Light",
+    },
+    {
+        .key = "Dark",
+        .path = ":/themes/Dark.json",
+        .name = "Dark",
+    },
+    {
+        .key = "Black",
+        .path = ":/themes/Black.json",
+        .name = "Black",
+    },
 };
 
-const ThemeDescriptor Theme::fallbackTheme = Theme::builtInThemes.at("Dark");
+// Dark is our default & fallback theme
+const ThemeDescriptor Theme::fallbackTheme = Theme::builtInThemes.at(2);
 
 bool Theme::isLightTheme() const
 {
@@ -208,11 +225,10 @@ void Theme::initialize(Settings &settings, Paths &paths)
 
 void Theme::update()
 {
-    auto theme = this->availableThemes_.find(this->themeName);
+    auto oTheme = this->findThemeByKey(this->themeName);
 
     std::optional<QJsonObject> themeJSON;
-
-    if (theme == this->availableThemes_.end())
+    if (!oTheme)
     {
         qCWarning(chatterinoTheme)
             << "Theme" << this->themeName
@@ -222,7 +238,9 @@ void Theme::update()
     }
     else
     {
-        themeJSON = loadTheme(theme->second);
+        const auto &theme = *oTheme;
+
+        themeJSON = loadTheme(theme);
 
         if (!themeJSON)
         {
@@ -247,24 +265,28 @@ void Theme::update()
     this->updated.invoke();
 }
 
-QStringList Theme::availableThemeNames() const
+std::vector<std::pair<QString, QVariant>> Theme::availableThemes() const
 {
-    QStringList themeNames;
+    std::vector<std::pair<QString, QVariant>> packagedThemes;
 
-    for (const auto &[name, theme] : this->availableThemes_)
+    for (const auto &theme : this->availableThemes_)
     {
-        // NOTE: This check could also use `Theme::builtInThemes.contains(name)` instead
         if (theme.custom)
         {
-            themeNames.append(QString("Custom: %1").arg(name));
+            auto p = std::make_pair(
+                QStringLiteral("Custom: %1").arg(theme.name), theme.key);
+
+            packagedThemes.emplace_back(p);
         }
         else
         {
-            themeNames.append(name);
+            auto p = std::make_pair(theme.name, theme.key);
+
+            packagedThemes.emplace_back(p);
         }
     }
 
-    return themeNames;
+    return packagedThemes;
 }
 
 void Theme::loadAvailableThemes()
@@ -285,7 +307,10 @@ void Theme::loadAvailableThemes()
             continue;
         }
 
-        auto themeDescriptor = ThemeDescriptor{info.absoluteFilePath(), true};
+        auto themeName = info.baseName();
+
+        auto themeDescriptor = ThemeDescriptor{
+            info.fileName(), info.absoluteFilePath(), themeName, true};
 
         auto theme = loadTheme(themeDescriptor);
         if (!theme)
@@ -294,10 +319,21 @@ void Theme::loadAvailableThemes()
             continue;
         }
 
-        auto themeName = info.baseName();
+        this->availableThemes_.emplace_back(std::move(themeDescriptor));
+    }
+}
 
-        this->availableThemes_.emplace(themeName, themeDescriptor);
+std::optional<ThemeDescriptor> Theme::findThemeByKey(const QString &key)
+{
+    for (const auto &theme : this->availableThemes_)
+    {
+        if (theme.key == key)
+        {
+            return theme;
+        }
     }
+
+    return std::nullopt;
 }
 
 void Theme::parseFrom(const QJsonObject &root)

src/singletons/Theme.hpp

@@ -8,6 +8,7 @@
 #include <QColor>
 #include <QJsonObject>
 #include <QPixmap>
+#include <QVariant>
 
 #include <map>
 #include <optional>
@@ -17,17 +18,22 @@ namespace chatterino {
 class WindowManager;
 
 struct ThemeDescriptor {
+    QString key;
+
     // Path to the theme on disk
     // Can be a Qt resource path
     QString path;
 
-    bool custom;
+    // Name of the theme
+    QString name;
+
+    bool custom{};
 };
 
 class Theme final : public Singleton
 {
 public:
-    static const std::map<QString, ThemeDescriptor> builtInThemes;
+    static const std::vector<ThemeDescriptor> builtInThemes;
 
     // The built in theme that will be used if some theme parsing fails
     static const ThemeDescriptor fallbackTheme;
@@ -133,10 +139,8 @@ class Theme final : public Singleton
 
     /**
      * Return a list of available themes
-     *
-     * Custom themes are prefixed with "Custom: "
      **/
-    QStringList availableThemeNames() const;
+    std::vector<std::pair<QString, QVariant>> availableThemes() const;
 
     pajlada::Signals::NoArgSignal updated;
 
@@ -145,7 +149,7 @@ class Theme final : public Singleton
 private:
     bool isLight_ = false;
 
-    std::map<QString, ThemeDescriptor> availableThemes_;
+    std::vector<ThemeDescriptor> availableThemes_;
 
     /**
      * Figure out which themes are available in the Themes directory
@@ -154,6 +158,8 @@ class Theme final : public Singleton
      **/
     void loadAvailableThemes();
 
+    std::optional<ThemeDescriptor> findThemeByKey(const QString &key);
+
     void parseFrom(const QJsonObject &root);
 
     pajlada::Signals::NoArgSignal repaintVisibleChatWidgets_;

src/widgets/settingspages/GeneralPage.cpp

@@ -115,25 +115,17 @@ void GeneralPage::initLayout(GeneralPageView &layout)
 
     layout.addTitle("Interface");
 
-    // NOTE: Available theme names are only initialized on startup here
-    QStringList availableThemeNames = getApp()->themes->availableThemeNames();
-
     layout.addDropdown<QString>(
-        "Theme", availableThemeNames, getApp()->themes->themeName,
-        [](auto val) {
-            if (!Theme::builtInThemes.contains(val))
-            {
-                return QString("Custom: %1").arg(val);
-            }
-
-            return val;
+        "Theme", getApp()->themes->availableThemes(),
+        getApp()->themes->themeName,
+        [](ComboBox *combo,
+           const QString &themeKey) -> boost::variant<int, QString> {
+            return combo->findData(themeKey, Qt::UserRole);
         },
-        [](const DropdownArgs &args) {
-            // Remove "Custom: " prefix from the dropdown
-            // XXX: This should be a regex or "trim prefix"
-            QString value = args.value;
-            return value.replace("Custom: ", "");
-        });
+        [](const DropdownArgs &args) -> QString {
+            return args.combobox->itemData(args.index, Qt::UserRole).toString();
+        },
+        "Theme", Theme::fallbackTheme.name);
 
     layout.addDropdown<QString>(
         "Font", {"Segoe UI", "Arial", "Choose..."},

src/widgets/settingspages/GeneralPageView.hpp

@@ -14,6 +14,8 @@
 #include <QSpinBox>
 #include <QVBoxLayout>
 
+#include <utility>
+
 class QScrollArea;
 
 namespace chatterino {
@@ -192,6 +194,59 @@ class GeneralPageView : public QWidget
 
         return combo;
     }
+
+    template <typename T>
+    ComboBox *addDropdown(
+        const QString &text,
+        const std::vector<std::pair<QString, QVariant>> &items,
+        pajlada::Settings::Setting<T> &setting,
+        std::function<boost::variant<int, QString>(ComboBox *, T)> getValue,
+        std::function<T(DropdownArgs)> setValue, QString toolTipText = {},
+        const QString &defaultValueText = {})
+    {
+        auto *combo = this->addDropdown(text, {}, std::move(toolTipText));
+
+        for (const auto &[text, userData] : items)
+        {
+            combo->addItem(text, userData);
+        }
+
+        if (!defaultValueText.isEmpty())
+        {
+            combo->setCurrentText(defaultValueText);
+        }
+
+        setting.connect(
+            [getValue = std::move(getValue), combo](const T &value, auto) {
+                auto var = getValue(combo, value);
+                if (var.which() == 0)
+                {
+                    const auto index = boost::get<int>(var);
+                    if (index >= 0)
+                    {
+                        combo->setCurrentIndex(index);
+                    }
+                }
+                else
+                {
+                    combo->setCurrentText(boost::get<QString>(var));
+                    combo->setEditText(boost::get<QString>(var));
+                }
+            },
+            this->managedConnections_);
+
+        QObject::connect(
+            combo, QOverload<const int>::of(&QComboBox::currentIndexChanged),
+            [combo, &setting,
+             setValue = std::move(setValue)](const int newIndex) {
+                setting = setValue(DropdownArgs{combo->itemText(newIndex),
+                                                combo->currentIndex(), combo});
+                getApp()->windows->forceLayoutChannelViews();
+            });
+
+        return combo;
+    }
+
     DescriptionLabel *addDescription(const QString &text);
 
     void addSeperator();