Themes: Fix issues with restoring themes.

Themes from the user config where not restored correctly.
Improve error handling when no themes are found
in case of broken installations.
Cleanly differentiate between theme "id" (currently complete basename of
theme file) and theme "displayName" (as specified as a property in the
theme file).
Remove convoluted broken code that tried to allow using an absolute file
path for a theme on the command line and require themes to be installed
either in Qt Creator globally or in the user settings path.
In general stream line the code.

Task-number: QTCREATORBUG-15113
Task-number: QTCREATORBUG-15233
Change-Id: I014a4314e8bea27422ed4c42462cf16f4220698b
Reviewed-by: Alessandro Portale <alessandro.portale@theqtcompany.com>

src/libs/utils/theme/theme.cpp

@@ -66,11 +66,11 @@ void setCreatorTheme(Theme *theme)
     m_creatorTheme = theme;
 }
 
-Theme::Theme(const QString &name, QObject *parent)
+Theme::Theme(const QString &id, QObject *parent)
   : QObject(parent)
   , d(new ThemePrivate)
 {
-    d->name = name;
+    d->id = id;
 }
 
 Theme::~Theme()
@@ -88,6 +88,11 @@ QStringList Theme::preferredStyles() const
     return d->preferredStyles;
 }
 
+QString Theme::id() const
+{
+    return d->id;
+}
+
 bool Theme::flag(Theme::Flag f) const
 {
     return d->flags[f];
@@ -130,14 +135,14 @@ QString Theme::filePath() const
     return d->fileName;
 }
 
-QString Theme::name() const
+QString Theme::displayName() const
 {
-    return d->name;
+    return d->displayName;
 }
 
-void Theme::setName(const QString &name)
+void Theme::setDisplayName(const QString &name)
 {
-    d->name = name;
+    d->displayName = name;
 }
 
 QVariantHash Theme::values() const
@@ -185,7 +190,7 @@ void Theme::writeSettings(const QString &filename) const
 
     const QMetaObject &m = *metaObject();
     {
-        settings.setValue(QLatin1String("ThemeName"), d->name);
+        settings.setValue(QLatin1String("ThemeName"), d->displayName);
         settings.setValue(QLatin1String("PreferredStyles"), d->preferredStyles);
     }
     {
@@ -264,7 +269,7 @@ void Theme::readSettings(QSettings &settings)
     const QMetaObject &m = *metaObject();
 
     {
-        d->name = settings.value(QLatin1String("ThemeName"), QLatin1String("unnamed")).toString();
+        d->displayName = settings.value(QLatin1String("ThemeName"), QLatin1String("unnamed")).toString();
         d->preferredStyles = settings.value(QLatin1String("PreferredStyles")).toStringList();
         d->preferredStyles.removeAll(QLatin1String(""));
     }

src/libs/utils/theme/theme.h

@@ -54,7 +54,7 @@ class QTCREATOR_UTILS_EXPORT Theme : public QObject
     Q_ENUMS(WidgetStyle)
 
 public:
-    Theme(const QString &name, QObject *parent = 0);
+    Theme(const QString &id, QObject *parent = 0);
     ~Theme();
 
     enum Color {
@@ -253,9 +253,10 @@ public:
     QPalette palette() const;
     QStringList preferredStyles() const;
 
+    QString id() const;
     QString filePath() const;
-    QString name() const;
+    QString displayName() const;
-    void setName(const QString &name);
+    void setDisplayName(const QString &displayName);
 
     QVariantHash values() const;
 

src/libs/utils/theme/theme_p.h

@@ -44,8 +44,9 @@ class QTCREATOR_UTILS_EXPORT ThemePrivate
 public:
     ThemePrivate();
 
+    QString id;
     QString fileName;
-    QString name;
+    QString displayName;
     QStringList preferredStyles;
     QVector<QPair<QColor, QString> > colors;
     QVector<QString> imageFiles;

src/plugins/coreplugin/coreplugin.cpp

@@ -37,6 +37,7 @@
 #include "modemanager.h"
 #include "infobar.h"
 #include "iwizardfactory.h"
+#include "themesettings.h"
 
 #include <coreplugin/actionmanager/actionmanager.h>
 #include <coreplugin/documentmanager.h>
@@ -48,6 +49,7 @@
 
 #include <extensionsystem/pluginerroroverview.h>
 #include <extensionsystem/pluginmanager.h>
+#include <utils/algorithm.h>
 #include <utils/pathchooser.h>
 #include <utils/macroexpander.h>
 #include <utils/savefile.h>
@@ -58,7 +60,6 @@
 #include <QtPlugin>
 #include <QDebug>
 #include <QDateTime>
-#include <QDir>
 #include <QMenu>
 
 using namespace Core;
@@ -97,36 +98,11 @@ CorePlugin::~CorePlugin()
     setCreatorTheme(0);
 }
 
-static QString absoluteThemePath(const QString &themeName, bool userProvidedTheme)
-{
-    if (themeName.isEmpty())
-        return themeName;
-    QString res = QDir::fromNativeSeparators(themeName);
-    QFileInfo fi(res);
-    bool tryRawName = userProvidedTheme || fi.isAbsolute();
-    // Try the given name
-    if (tryRawName && fi.exists())
-        return fi.absoluteFilePath();
-    const QString suffix = QLatin1String("creatortheme");
-    // Try name.creatortheme
-    if (fi.suffix() != suffix) {
-        res = themeName + QLatin1Char('.') + suffix;
-        fi.setFile(res);
-        if (tryRawName && fi.exists())
-            return fi.absoluteFilePath();
-    }
-    if (fi.path().isEmpty())
-        return QString(); // absolute/relative path, but not found
-    // If only name was given, look it up in qtcreator/themes
-    res.prepend(ICore::resourcePath() + QLatin1String("/themes/"));
-    return QFileInfo::exists(res) ? res : QString();
-}
-
 void CorePlugin::parseArguments(const QStringList &arguments)
 {
-    const QString defaultTheme = QLatin1String("default");
+    const Id settingsThemeId = Id::fromSetting(ICore::settings()->value(
-    QString themeName = ICore::settings()->value(
+                QLatin1String(Constants::SETTINGS_THEME), QLatin1String("default")));
-                QLatin1String(Constants::SETTINGS_THEME), defaultTheme).toString();
+    Id themeId = settingsThemeId;
     QColor overrideColor;
     bool presentationMode = false;
     bool userProvidedTheme = false;
@@ -140,28 +116,28 @@ void CorePlugin::parseArguments(const QStringList &arguments)
         if (arguments.at(i) == QLatin1String("-presentationMode"))
             presentationMode = true;
         if (arguments.at(i) == QLatin1String("-theme")) {
-            themeName = arguments.at(i + 1);
+            themeId = Id::fromString(arguments.at(i + 1));
             userProvidedTheme = true;
             i++;
         }
     }
-
+    const QList<ThemeEntry> availableThemes = ThemeSettings::availableThemes();
-    QString themeURI = absoluteThemePath(themeName, userProvidedTheme);
+    int themeIndex = Utils::indexOf(availableThemes, Utils::equal(&ThemeEntry::id, themeId));
-    if (themeURI.isEmpty()) {
+    if (themeIndex < 0) {
-        themeName = defaultTheme;
+        themeIndex = Utils::indexOf(availableThemes,
-        themeURI = QStringLiteral("%1/themes/%2.creatortheme").arg(ICore::resourcePath()).arg(themeName);
+                                    Utils::equal(&ThemeEntry::id, settingsThemeId));
-        if (themeURI.isEmpty()) {
-            qCritical("%s", qPrintable(QCoreApplication::translate("Application", "No valid theme \"%1\"")
-                                       .arg(themeName)));
     }
-    }
+    if (themeIndex < 0)
-
+        themeIndex = 0;
-    QSettings themeSettings(themeURI, QSettings::IniFormat);
+    if (themeIndex < availableThemes.size()) {
-    Theme *theme = new Theme(themeName, qApp);
+        const ThemeEntry themeEntry = availableThemes.at(themeIndex);
+        QSettings themeSettings(themeEntry.filePath(), QSettings::IniFormat);
+        Theme *theme = new Theme(themeEntry.id().toString(), qApp);
         theme->readSettings(themeSettings);
         if (theme->flag(Theme::ApplyThemePaletteGlobally))
             QApplication::setPalette(theme->palette());
         setCreatorTheme(theme);
+    }
 
     // defer creation of these widgets until here,
     // because they need a valid theme set
@@ -176,6 +152,10 @@ void CorePlugin::parseArguments(const QStringList &arguments)
 
 bool CorePlugin::initialize(const QStringList &arguments, QString *errorMessage)
 {
+    if (ThemeSettings::availableThemes().isEmpty()) {
+        *errorMessage = tr("No themes found in installation.");
+        return false;
+    }
     new ActionManager(this);
     Theme::initialPalette(); // Initialize palette before setting it
     qsrand(QDateTime::currentDateTime().toTime_t());

src/plugins/coreplugin/themeeditor/themesettingstablemodel.cpp

@@ -222,7 +222,7 @@ void ThemeSettingsTableModel::initFrom(Theme *theme)
     }
 
     m_widgetStyle = theme->widgetStyle();
-    m_name = theme->d->name;
+    m_displayName = theme->d->displayName;
     m_preferredStyles = theme->d->preferredStyles;
 }
 
@@ -252,7 +252,7 @@ void ThemeSettingsTableModel::toTheme(Theme *t) const
     }
 
     theme->widgetStyle = m_widgetStyle;
-    theme->name = m_name;
+    theme->displayName = m_displayName;
     theme->preferredStyles = m_preferredStyles;
 }
 

src/plugins/coreplugin/themeeditor/themesettingstablemodel.h

@@ -76,7 +76,7 @@ public:
     void initFrom(Utils::Theme *theme);
     void toTheme(Utils::Theme *theme) const;
 
-    QString m_name;
+    QString m_displayName;
     QStringList m_preferredStyles;
 
 public:

src/plugins/coreplugin/themesettings.cpp

@@ -31,14 +31,55 @@
 #include "themesettings.h"
 #include "themesettingswidget.h"
 #include "coreconstants.h"
+#include "icore.h"
+
+#include <utils/algorithm.h>
 
 #include <QCoreApplication>
+#include <QDebug>
+#include <QDir>
+#include <QSettings>
+
+static const char themeNameKey[] = "ThemeName";
 
 namespace Core {
 namespace Internal {
 
-ThemeSettings::ThemeSettings() :
+ThemeEntry::ThemeEntry(Id id, const QString &filePath, bool readOnly)
-    m_widget(0)
+    : m_id(id),
+      m_filePath(filePath),
+      m_readOnly(readOnly)
+{
+}
+
+Id ThemeEntry::id() const
+{
+    return m_id;
+}
+
+QString ThemeEntry::displayName() const
+{
+    if (m_displayName.isEmpty() && !m_filePath.isEmpty()) {
+        QSettings settings(m_filePath, QSettings::IniFormat);
+        m_displayName = settings.value(QLatin1String(themeNameKey),
+                                       QCoreApplication::tr("unnamed")).toString();
+        if (false) // TODO: Revert to m_readOnly
+            m_displayName = QCoreApplication::tr("%1 (built-in)").arg(m_displayName);
+    }
+    return m_displayName;
+}
+
+QString ThemeEntry::filePath() const
+{
+    return m_filePath;
+}
+
+bool ThemeEntry::readOnly() const
+{
+    return m_readOnly;
+}
+
+ThemeSettings::ThemeSettings()
 {
     setId(Constants::SETTINGS_ID_INTERFACE);
     setDisplayName(tr("Theme"));
@@ -71,5 +112,38 @@ void ThemeSettings::finish()
     m_widget = 0;
 }
 
+static void addThemesFromPath(const QString &path, bool readOnly, QList<ThemeEntry> *themes)
+{
+    static const QLatin1String extension(".creatortheme");
+    QDir themeDir(path);
+    themeDir.setNameFilters(QStringList() << QLatin1String("*.creatortheme"));
+    themeDir.setFilter(QDir::Files);
+    const QStringList themeList = themeDir.entryList();
+    foreach (const QString &fileName, themeList) {
+        QString id = QFileInfo(fileName).completeBaseName();
+        themes->append(ThemeEntry(Id::fromString(id), themeDir.absoluteFilePath(fileName), readOnly));
+    }
+}
+
+QList<ThemeEntry> ThemeSettings::availableThemes()
+{
+    QList<ThemeEntry> themes;
+
+    static const QString installThemeDir = ICore::resourcePath() + QLatin1String("/themes");
+    static const QString userThemeDir = ICore::userResourcePath() + QLatin1String("/themes");
+    addThemesFromPath(installThemeDir, /*readOnly=*/true, &themes);
+    if (themes.isEmpty())
+        qWarning() << "Warning: No themes found in installation: "
+                   << QDir::toNativeSeparators(installThemeDir);
+    // move default theme to front
+    int defaultIndex = Utils::indexOf(themes, Utils::equal(&ThemeEntry::id, Id("default")));
+    if (defaultIndex > 0) { // == exists and not at front
+        ThemeEntry defaultEntry = themes.takeAt(defaultIndex);
+        themes.prepend(defaultEntry);
+    }
+    addThemesFromPath(userThemeDir, /*readOnly=*/false, &themes);
+    return themes;
+}
+
 } // namespace Internal
 } // namespace Core

src/plugins/coreplugin/themesettings.h

@@ -31,6 +31,8 @@
 #ifndef THEMESETTINGS_H
 #define THEMESETTINGS_H
 
+#include "id.h"
+
 #include <coreplugin/dialogs/ioptionspage.h>
 
 namespace Core {
@@ -38,6 +40,24 @@ namespace Internal {
 
 class ThemeSettingsWidget;
 
+class ThemeEntry
+{
+public:
+    ThemeEntry() = default;
+    ThemeEntry(Id id, const QString &filePath, bool readOnly);
+
+    Id id() const;
+    QString displayName() const;
+    QString filePath() const;
+    bool readOnly() const;
+
+private:
+    Id m_id;
+    QString m_filePath;
+    mutable QString m_displayName;
+    bool m_readOnly = true;
+};
+
 class ThemeSettings : public IOptionsPage
 {
     Q_OBJECT
@@ -50,7 +70,9 @@ public:
     void apply();
     void finish();
 
-    ThemeSettingsWidget *m_widget;
+    static QList<ThemeEntry> availableThemes();
+private:
+    ThemeSettingsWidget *m_widget = 0;
 };
 
 } // namespace Internal

src/plugins/coreplugin/themesettingswidget.cpp

@@ -33,7 +33,9 @@
 #include "icore.h"
 #include "manhattanstyle.h"
 #include "themeeditor/themesettingstablemodel.h"
+#include "themesettings.h"
 
+#include <utils/algorithm.h>
 #include <utils/theme/theme.h>
 #include <utils/theme/theme_p.h>
 #include <utils/qtcassert.h>
@@ -52,8 +54,6 @@ using namespace Utils;
 namespace Core {
 namespace Internal {
 
-const char themeNameKey[] = "ThemeName";
-
 static QString customThemesPath()
 {
     return ICore::userResourcePath() + QLatin1String("/themes/");
@@ -81,42 +81,6 @@ static QString createThemeFileName(const QString &pattern)
     return fileName;
 }
 
-
-struct ThemeEntry
-{
-    ThemeEntry() : m_readOnly(true) {}
-    ThemeEntry(const QString &name, const QString &filePath, bool readOnly):
-        m_name(name),
-        m_filePath(filePath),
-        m_readOnly(readOnly)
-    {
-    }
-
-    QString name() const { return m_name; }
-    QString displayName() const;
-    QString filePath() const { return m_filePath; }
-    bool readOnly() const { return m_readOnly; }
-
-private:
-    QString m_name;
-    QString m_filePath;
-    mutable QString m_displayName;
-    bool m_readOnly;
-};
-
-QString ThemeEntry::displayName() const
-{
-    if (m_displayName.isEmpty()) {
-        QSettings settings(filePath(), QSettings::IniFormat);
-        m_displayName = settings.value(QLatin1String(themeNameKey),
-                                       QCoreApplication::tr("unnamed")).toString();
-        if (false) // TODO: Revert to m_readOnly
-            m_displayName = QCoreApplication::tr("%1 (built-in)").arg(m_displayName);
-    }
-    return m_displayName;
-}
-
-
 class ThemeListModel : public QAbstractListModel
 {
 public:
@@ -179,7 +143,7 @@ ThemeSettingsPrivate::ThemeSettingsPrivate(QWidget *widget)
     , m_refreshingThemeList(false)
     , m_ui(new Ui::ThemeSettings)
 {
-    m_currentTheme = ThemeEntry(creatorTheme()->name(), creatorTheme()->filePath(), true);
+    m_currentTheme = ThemeEntry(Id::fromString(creatorTheme()->id()), creatorTheme()->filePath(), true);
     m_ui->setupUi(widget);
     // TODO: Restore the editor and the buttons after improving the editor
     m_ui->editor->hide();
@@ -216,44 +180,15 @@ ThemeSettingsWidget::~ThemeSettingsWidget()
 
 void ThemeSettingsWidget::refreshThemeList()
 {
-    QList<ThemeEntry> themes;
+    const QList<ThemeEntry> themes = ThemeSettings::availableThemes();
-
+    const int selected = Utils::indexOf(themes, Utils::equal(&ThemeEntry::id, d->m_currentTheme.id()));
-    QDir themeDir(ICore::resourcePath() + QLatin1String("/themes"));
-    themeDir.setNameFilters(QStringList() << QLatin1String("*.creatortheme"));
-    themeDir.setFilter(QDir::Files);
-
-    int selected = 0;
-
-    QStringList themeList = themeDir.entryList();
-    const QString defaultTheme = QLatin1String("default.creatortheme");
-    if (themeList.removeOne(defaultTheme))
-        themeList.prepend(defaultTheme);
-    const QLatin1String extension(".creatortheme");
-    for (int i = 0, total = themeList.count(); i < total; ++i) {
-        const QString fileName = themeList.at(i);
-        if (d->m_currentTheme.name() + extension == fileName)
-            selected = i;
-        QString name = fileName;
-        name.remove(extension);
-        themes.append(ThemeEntry(name, themeDir.absoluteFilePath(fileName), true));
-    }
-
-    if (themes.isEmpty())
-        qWarning() << "Warning: no themes found in path:" << themeDir.path();
-
-    themeDir.setPath(customThemesPath());
-    foreach (const QString &file, themeDir.entryList()) {
-        const QString fileName = themeDir.absoluteFilePath(file);
-        if (d->m_currentTheme.name() == fileName)
-            selected = themes.size();
-        themes.append(ThemeEntry(fileName, fileName, false));
-    }
-
-    d->m_currentTheme = themes[selected];
 
     d->m_refreshingThemeList = true;
     d->m_themeListModel->setThemes(themes);
+    if (selected >= 0) {
+        d->m_currentTheme = themes[selected];
         d->m_ui->themeComboBox->setCurrentIndex(selected);
+    }
     d->m_refreshingThemeList = false;
 }
 
@@ -270,7 +205,7 @@ void ThemeSettingsWidget::themeSelected(int index)
         d->m_currentTheme = entry;
 
         QSettings settings(entry.filePath(), QSettings::IniFormat);
-        Theme theme(entry.name());
+        Theme theme(entry.id().toString());
         theme.readSettings(settings);
         d->m_ui->editor->initFrom(&theme);
     }
@@ -354,7 +289,7 @@ void ThemeSettingsWidget::maybeSaveTheme()
     messageBox->setDefaultButton(QMessageBox::Save);
 
     if (messageBox->exec() == QMessageBox::Save) {
-        Theme newTheme(d->m_currentTheme.name());
+        Theme newTheme(d->m_currentTheme.id().toString());
         d->m_ui->editor->model()->toTheme(&newTheme);
         newTheme.writeSettings(d->m_currentTheme.filePath());
     }
@@ -373,7 +308,7 @@ void ThemeSettingsWidget::renameTheme()
     dialog->setInputMode(QInputDialog::TextInput);
     dialog->setWindowTitle(tr("Rename Theme"));
     dialog->setLabelText(tr("Theme name:"));
-    dialog->setTextValue(d->m_ui->editor->model()->m_name);
+    dialog->setTextValue(d->m_ui->editor->model()->m_displayName);
     int ret = dialog->exec();
     QString newName = dialog->textValue();
     delete dialog;
@@ -382,9 +317,9 @@ void ThemeSettingsWidget::renameTheme()
         return;
 
     // overwrite file with new name
-    Theme newTheme(entry.name());
+    Theme newTheme(entry.id().toString());
     d->m_ui->editor->model()->toTheme(&newTheme);
-    newTheme.setName(newName);
+    newTheme.setDisplayName(newName);
     newTheme.writeSettings(entry.filePath());
 
     refreshThemeList();
@@ -401,8 +336,9 @@ void ThemeSettingsWidget::copyThemeByName(const QString &name)
     QString baseFileName = QFileInfo(entry.filePath()).completeBaseName();
     baseFileName += QLatin1String("_copy%1.creatortheme");
     QString fileName = createThemeFileName(baseFileName);
+    QString id = QFileInfo(fileName).completeBaseName();
 
-    if (fileName.isEmpty())
+    if (fileName.isEmpty() || id.isEmpty())
         return;
 
     // Ask about saving any existing modifactions
@@ -410,18 +346,18 @@ void ThemeSettingsWidget::copyThemeByName(const QString &name)
 
     Theme newTheme(fileName);
     d->m_ui->editor->model()->toTheme(&newTheme);
-    newTheme.setName(name);
+    newTheme.setDisplayName(name);
     newTheme.writeSettings(fileName);
 
-    d->m_currentTheme = ThemeEntry(fileName, fileName, true);
+    d->m_currentTheme = ThemeEntry(Id::fromString(id), fileName, true);
 
     refreshThemeList();
 }
 
 void ThemeSettingsWidget::apply()
 {
-    const QString themeName = d->m_currentTheme.name();
+    const QString themeId = d->m_currentTheme.id().toString();
-    Theme *newTheme = new Theme(themeName);
+    Theme *newTheme = new Theme(themeId);
     if (d->m_currentTheme.readOnly()) {
         QSettings themeSettings(d->m_currentTheme.filePath(), QSettings::IniFormat);
         newTheme->readSettings(themeSettings);
@@ -447,7 +383,7 @@ void ThemeSettingsWidget::apply()
 
     // save filename of selected theme in global config
     QSettings *settings = ICore::settings();
-    settings->setValue(QLatin1String(Constants::SETTINGS_THEME), themeName);
+    settings->setValue(QLatin1String(Constants::SETTINGS_THEME), themeId);
 }
 
 } // namespace Internal