Fixes #965 - Fail to duplicate color theme on windows. (#970)

Fixes bug with color change
Performed small refactoring of AppearanceOptionsWidget, ColorSchemeFileSaver

src/common/ColorSchemeFileSaver.cpp

@@ -26,38 +26,25 @@ static const QStringList cutterSpecificOptions = {
 
 ColorSchemeFileSaver::ColorSchemeFileSaver(QObject *parent) : QObject (parent)
 {
-    customR2ThemesLocationPath = QStandardPaths::writableLocation(QStandardPaths::GenericDataLocation) +
+    char* szThemes = r_str_home(R2_HOME_THEMES);
-                                 QDir::separator() +
+    customR2ThemesLocationPath = szThemes;
-                                 "radare2" + QDir::separator() +
+    R_FREE(szThemes);
-                                 "cons";
     if (!QDir(customR2ThemesLocationPath).exists()) {
         QDir().mkpath(customR2ThemesLocationPath);
     }
 
-    QDir currDir;
+    QDir currDir { QStringLiteral("%1%2%3")
-    QStringList dirs = QStandardPaths::standardLocations(QStandardPaths::GenericDataLocation);
+        .arg(r_sys_prefix(nullptr))
-    dirs.removeOne(QStandardPaths::writableLocation(QStandardPaths::GenericDataLocation));
+        .arg(R_SYS_DIR)
-    standardR2ThemesLocationPath = "";
+        .arg(R2_THEMES)
-
+    };
-    for (const QString &it : dirs) {
-        currDir = QDir(it).filePath("radare2");
-        if (currDir.exists()) {
-            break;
-        }
-        currDir.setPath("");
-    }
-
-    currDir.setPath(QString(r_sys_prefix(nullptr)) + QString(R_SYS_DIR) + R2_THEMES);
     if (currDir.exists()) {
         standardR2ThemesLocationPath = currDir.absolutePath();
     } else {
-        standardR2ThemesLocationPath = "";
+        QMessageBox::critical(nullptr,
-        QMessageBox mb;
+            tr("Standard themes not found!"),
-        mb.setIcon(QMessageBox::Critical);
+            tr("The radare2 standard themes could not be found! This probably means radare2 is not properly installed. If you think it is open an issue please.")
-        mb.setStandardButtons(QMessageBox::Ok);
+        );
-        mb.setWindowTitle(tr("Standard themes not found!"));
-        mb.setText(tr("The radare2 standard themes could not be found! This probably means radare2 is not properly installed. If you think it is open an issue please."));
-        mb.exec();
     }
 }
 
@@ -67,15 +54,17 @@ QFile::FileError ColorSchemeFileSaver::copy(const QString &srcThemeName,
     QFile fIn(standardR2ThemesLocationPath + QDir::separator() + srcThemeName);
     QFile fOut(customR2ThemesLocationPath + QDir::separator() + copyThemeName);
 
-    if (srcThemeName != "default" && !fIn.open(QFile::ReadOnly)) {
+    if (srcThemeName != QStringLiteral("default") && !fIn.open(QFile::ReadOnly)) {
         fIn.setFileName(customR2ThemesLocationPath + QDir::separator() + srcThemeName);
         if (!fIn.open(QFile::ReadOnly)) {
             return fIn.error();
         }
     }
 
-    if (!fOut.open(QFile::WriteOnly | QFile::Truncate)) {
+    const QString &srcTheme = fIn.readAll();
     fIn.close();
+
+    if (!fOut.open(QFile::WriteOnly | QFile::Truncate)) {
         return fOut.error();
     }
 
@@ -84,22 +73,23 @@ QFile::FileError ColorSchemeFileSaver::copy(const QString &srcThemeName,
     QStringList src;
 
     if (srcThemeName == "default") {
-        QString theme = Config()->getCurrentTheme();
+        const QString &theme = Config()->getColorTheme();
         Core()->cmd("ecd");
-        QJsonObject _obj = Core()->cmdj("ecj").object();
+        QJsonObject obj = Core()->cmdj("ecj").object();
-        Core()->cmd(QString("eco %1").arg(theme));
+        Core()->cmd(QStringLiteral("eco %1").arg(theme));
         QColor back = Config()->getColor(standardBackgroundOptionName);
-        _obj[standardBackgroundOptionName] = QJsonArray({back.red(), back.green(), back.blue()});
+        obj[standardBackgroundOptionName] = QJsonArray({back.red(), back.green(), back.blue()});
-        for (const QString &it : _obj.keys()) {
+        for (const QString &it : obj.keys()) {
-            QJsonArray rgb = _obj[it].toArray();
+            QJsonArray rgb = obj[it].toArray();
             if (rgb.size() != 3) {
                 continue;
             }
-            src.push_back("ec " + it + " " +
+            src.push_back(QStringLiteral("ec %1 rgb:%2")
-                          QColor(rgb[0].toInt(), rgb[1].toInt(), rgb[2].toInt()).name().replace("#", "rgb:"));
+                .arg(it)
+                .arg(QColor(rgb[0].toInt(), rgb[1].toInt(), rgb[2].toInt()).name().remove('#')));
         }
     } else {
-        src = QString(fIn.readAll()).split('\n');
+        src = srcTheme.split('\n');
     }
 
     QStringList tmp;
@@ -123,12 +113,12 @@ QFile::FileError ColorSchemeFileSaver::copy(const QString &srcThemeName,
         } else {
             fOut.write("ec ");
         }
-        fOut.write(QString(it + " rgb:%1\n").
+        fOut.write(QStringLiteral("%1 rgb:%2\n")
-                   arg(Config()->getColor(it).name().remove("#")).toUtf8());
+            .arg(it)
+            .arg(Config()->getColor(it).name().remove('#')).toUtf8());
     }
 
     fOut.close();
-    fIn.close();
 
     return QFile::FileError::NoError;
 }
@@ -146,26 +136,25 @@ QFile::FileError ColorSchemeFileSaver::save(const QString &scheme, const QString
 
 bool ColorSchemeFileSaver::isCustomScheme(const QString &schemeName) const
 {
-    for (const QFileInfo &it : QDir(customR2ThemesLocationPath).entryInfoList())
+    return QFile::exists(QDir(customR2ThemesLocationPath).filePath(schemeName));
-        if (it.fileName() == schemeName)
-            return true;
-    return false;
 }
 
 bool ColorSchemeFileSaver::isNameEngaged(const QString &name) const
 {
-    return QFile::exists(standardR2ThemesLocationPath + QDir::separator() + name) ||
+    return (!standardR2ThemesLocationPath.isEmpty() && QFile::exists(standardR2ThemesLocationPath + QDir::separator() + name)) ||
            QFile::exists(customR2ThemesLocationPath + QDir::separator() + name);
 }
 
 QMap<QString, QColor> ColorSchemeFileSaver::getCutterSpecific() const
 {
-    QFile f(customR2ThemesLocationPath + QDir::separator() + Config()->getCurrentTheme());
+    QFile f(customR2ThemesLocationPath + QDir::separator() + Config()->getColorTheme());
     if (!f.open(QFile::ReadOnly))
         return  QMap<QString, QColor>();
 
+    const QStringList &data = QString(f.readAll()).split('\n');
+    f.close();
+
     QMap<QString, QColor> ret;
-    QStringList data = QString(f.readAll()).split('\n');
     for (const QString &it : data) {
         if (it.length() > 2 && it.left(2) == "#~") {
             QStringList currLine = it.split(' ');
@@ -175,15 +164,12 @@ QMap<QString, QColor> ColorSchemeFileSaver::getCutterSpecific() const
         }
     }
 
-    f.close();
     return ret;
 }
 
 QStringList ColorSchemeFileSaver::getCustomSchemes() const
 {
-    QStringList sl;
+    return QDir(customR2ThemesLocationPath).entryList(QDir::Files | QDir::NoDotAndDotDot, QDir::Name);
-    sl = QDir(customR2ThemesLocationPath).entryList(QDir::Files | QDir::NoDotAndDotDot, QDir::Name);
-    return sl;
 }
 
 void ColorSchemeFileSaver::deleteScheme(const QString &schemeName) const

src/common/Configuration.cpp

@@ -67,7 +67,7 @@ Configuration *Configuration::instance()
 void Configuration::loadInitial()
 {
     setTheme(getTheme());
-    setColorTheme(getCurrentTheme());
+    setColorTheme(getColorTheme());
     applySavedAsmOptions();
 }
 
@@ -234,7 +234,7 @@ void Configuration::setFont(const QFont &font)
 QString Configuration::getLastThemeOf(const CutterQtTheme &currQtTheme) const
 {
     return s.value("lastThemeOf." + currQtTheme.name,
-                   Config()->getCurrentTheme()).toString();
+                   Config()->getColorTheme()).toString();
 }
 
 void Configuration::setTheme(int theme)
@@ -287,21 +287,21 @@ const QColor Configuration::getColor(const QString &name) const
     }
 }
 
-void Configuration::setColorTheme(QString theme)
+void Configuration::setColorTheme(const QString &theme)
 {
     if (theme == "default") {
         Core()->cmd("ecd");
         s.setValue("theme", "default");
     } else {
-        Core()->cmd(QString("eco %1").arg(theme));
+        Core()->cmd(QStringLiteral("eco %1").arg(theme));
         s.setValue("theme", theme);
     }
     // Duplicate interesting colors into our Cutter Settings
     // Dirty fix for arrow colors, TODO refactor getColor, setColor, etc.
     QJsonDocument colors = Core()->cmdj("ecj");
     QJsonObject colorsObject = colors.object();
-    QJsonObject::iterator it;
+
-    for (it = colorsObject.begin(); it != colorsObject.end(); it++) {
+    for (auto it = colorsObject.constBegin(); it != colorsObject.constEnd(); it++) {
         QJsonArray rgb = it.value().toArray();
         if (rgb.size() != 3) {
             continue;

src/common/Configuration.h

@@ -88,8 +88,8 @@ public:
         s.setValue("graph.maxcols", ch);
     }
 
-    QString getCurrentTheme() const     { return s.value("theme", "cutter").toString(); }
+    QString getColorTheme() const     { return s.value("theme", "cutter").toString(); }
-    void setColorTheme(QString theme);
+    void setColorTheme(const QString &theme);
 
     /*!
      * \brief Get the value of a config var either from r2 or settings, depending on the key.

src/dialogs/preferences/AppearanceOptionsWidget.cpp

@@ -66,7 +66,7 @@ AppearanceOptionsWidget::AppearanceOptionsWidget(PreferencesDialog *dialog, QWid
     ui->setupUi(this);
 
     updateFontFromConfig();
-    updateThemeFromConfig();
+    updateThemeFromConfig(false);
 
     QStringList langs = findLanguages();
     ui->languageComboBox->addItems(langs);
@@ -94,11 +94,14 @@ void AppearanceOptionsWidget::updateFontFromConfig()
     ui->fontSelectionLabel->setText(currentFont.toString());
 }
 
-void AppearanceOptionsWidget::updateThemeFromConfig()
+void AppearanceOptionsWidget::updateThemeFromConfig(bool qtThemeChanged)
 {
     // Disconnect currentIndexChanged because clearing the comboxBox and refiling it causes its index to change.
     QSignalBlocker signalBlockerColorBox(ui->colorComboBox);
     QSignalBlocker signalBlockerThemeBox(ui->themeComboBox);
+    Q_UNUSED(signalBlockerColorBox);
+    Q_UNUSED(signalBlockerThemeBox);
+
     ui->themeComboBox->clear();
     for (auto &it : kCutterQtThemesList) {
         ui->themeComboBox->addItem(it.name);
@@ -112,25 +115,26 @@ void AppearanceOptionsWidget::updateThemeFromConfig()
 
     QList<QString> themes = Core()->getColorThemes();
     ui->colorComboBox->clear();
-    for (QString theme : themes) {
+    for (const QString &theme : themes) {
         if (ColorSchemeFileWorker().isCustomScheme(theme) ||
             (kCutterQtThemesList[curQtThemeIndex].flag & kRelevantSchemes[theme])) {
             ui->colorComboBox->addItem(theme);
         }
     }
 
-    QString curTheme = Config()->getLastThemeOf(kCutterQtThemesList[curQtThemeIndex]);
+    QString curTheme = qtThemeChanged
-    int index = ui->colorComboBox->findText(curTheme);
+        ? Config()->getLastThemeOf(kCutterQtThemesList[curQtThemeIndex])
-    if (index == -1) {
+        : Config()->getColorTheme();
-        index = 0;
+    const int index = ui->colorComboBox->findText(curTheme);
-    }
 
-    ui->colorComboBox->setCurrentIndex(index);
+    ui->colorComboBox->setCurrentIndex(index == -1 ? 0 : index);
+    if (qtThemeChanged || index == -1) {
         curTheme = ui->colorComboBox->currentText();
         Config()->setColorTheme(curTheme);
+    }
     ui->colorSchemePrefWidget->updateSchemeFromConfig();
     int maxThemeLen = 0;
-    for (QString str : themes) {
+    for (const QString &str : themes) {
         int strLen = str.length();
         if (strLen > maxThemeLen) {
             maxThemeLen = strLen;
@@ -179,27 +183,29 @@ void AppearanceOptionsWidget::on_copyButton_clicked()
         newSchemeName = QInputDialog::getText(this, tr("Enter scheme name"),
                                               tr("Name:"), QLineEdit::Normal,
                                               QDir::home().dirName());
-    } while (ColorSchemeFileWorker().isNameEngaged(newSchemeName) && !newSchemeName.isEmpty());
+    } while ((!newSchemeName.isEmpty() && ColorSchemeFileWorker().isNameEngaged(newSchemeName))
+        || newSchemeName.contains(QRegExp("[^\\w.()\\[\\]_-]"))
+        || newSchemeName.startsWith('.'));
 
     if (newSchemeName.isEmpty())
         return;
-    ColorSchemeFileWorker().copy(Config()->getCurrentTheme(), newSchemeName);
+    ColorSchemeFileWorker().copy(Config()->getColorTheme(), newSchemeName);
     Config()->setColorTheme(newSchemeName);
     ui->colorSchemePrefWidget->updateSchemeFromConfig();
-    updateThemeFromConfig();
+    updateThemeFromConfig(false);
 }
 
 void AppearanceOptionsWidget::on_deleteButton_clicked()
 {
-    if (ColorSchemeFileWorker().isCustomScheme(Config()->getCurrentTheme())) {
+    if (ColorSchemeFileWorker().isCustomScheme(Config()->getColorTheme())) {
         QMessageBox mb;
         mb.setWindowTitle(tr("Delete"));
-        mb.setText(tr("Are you sure you want to delete theme ") + Config()->getCurrentTheme());
+        mb.setText(tr("Are you sure you want to delete theme ") + Config()->getColorTheme());
         mb.setIcon(QMessageBox::Question);
         mb.setStandardButtons(QMessageBox::Yes | QMessageBox::No);
         if (mb.exec() == QMessageBox::Yes) {
-            ColorSchemeFileWorker().deleteScheme(Config()->getCurrentTheme());
+            ColorSchemeFileWorker().deleteScheme(Config()->getColorTheme());
-            updateThemeFromConfig();
+            updateThemeFromConfig(false);
         }
     }
 }

src/dialogs/preferences/AppearanceOptionsWidget.h

@@ -27,7 +27,7 @@ private:
 
 private slots:
     void updateFontFromConfig();
-    void updateThemeFromConfig();
+    void updateThemeFromConfig(bool qtThemeChanged = true);
 
     void on_fontSelectionButton_clicked();
     void on_themeComboBox_currentIndexChanged(int index);

src/widgets/ColorSchemePrefWidget.cpp

@@ -424,8 +424,8 @@ void ColorSchemePrefWidget::apply()
         }
         scheme += curr.optionName + " rgb:" + curr.color.name().remove("#").toLower() + "\n";
     }
-    ColorSchemeFileWorker().save(scheme, Config()->getCurrentTheme());
+    ColorSchemeFileWorker().save(scheme, Config()->getColorTheme());
-    Config()->setColorTheme(Config()->getCurrentTheme());
+    Config()->setColorTheme(Config()->getColorTheme());
 }
 
 void ColorSchemePrefWidget::newColor()
@@ -436,13 +436,15 @@ void ColorSchemePrefWidget::newColor()
 
     ColorOption currCO = ui->preferencesListView->model()->data(ui->preferencesListView->currentIndex(),
                                                                 Qt::UserRole).value<ColorOption>();
-    QColorDialog d;
+    QColorDialog d(currCO.color, this);
-    d.setCurrentColor(currCO.color);
+    if (QDialog::Accepted != d.exec())
-    d.exec();
+        return;
 
     static_cast<ColorSettingsModel *>(ui->preferencesListView->model())->setColor(currCO.optionName,
                                                                                   d.selectedColor());
     ui->preferencesListView->setStandardColors();
+    if (ui->preferencesListView->model()->rowCount())
+        indexChanged(ui->preferencesListView->selectionModel()->selectedIndexes().first());
 }
 
 void ColorSchemePrefWidget::indexChanged(const QModelIndex &ni)
@@ -452,7 +454,7 @@ void ColorSchemePrefWidget::indexChanged(const QModelIndex &ni)
 
 void ColorSchemePrefWidget::updateSchemeFromConfig()
 {
-    isEditable = ColorSchemeFileWorker().isCustomScheme(Config()->getCurrentTheme());
+    isEditable = ColorSchemeFileWorker().isCustomScheme(Config()->getColorTheme());
     static_cast<ColorSettingsModel *>(ui->preferencesListView->model())->updateScheme();
 }
 
@@ -486,7 +488,7 @@ void ColorSettingsModel::setColor(const QString &option, const QColor &color)
 
 QColor ColorSettingsModel::getBackroundColor() const
 {
-    if (!ColorSchemeFileWorker().isCustomScheme(Config()->getCurrentTheme())) {
+    if (!ColorSchemeFileWorker().isCustomScheme(Config()->getColorTheme())) {
         return Config()->getColor(standardBackgroundOptionName);
     }
 
@@ -500,7 +502,7 @@ QColor ColorSettingsModel::getBackroundColor() const
 
 QColor ColorSettingsModel::getTextColor() const
 {
-    if (!ColorSchemeFileWorker().isCustomScheme(Config()->getCurrentTheme())) {
+    if (!ColorSchemeFileWorker().isCustomScheme(Config()->getColorTheme())) {
         return Config()->getColor(standardTextOptionName);
     }