Introduce a config resolver (#2244)

* Add config resolver

Signed-off-by: Haris Gušić <harisgusic.dev@gmail.com>

* Enable resolver even when using systray

Signed-off-by: Haris Gušić <harisgusic.dev@gmail.com>

* Fix bugs

Signed-off-by: Haris Gušić <harisgusic.dev@gmail.com>

* Do not show resolver for shortcut conflicts

Signed-off-by: Haris Gušić <harisgusic.dev@gmail.com>

* Fix build error on MacOS and Windows

Signed-off-by: Haris Gušić <harisgusic.dev@gmail.com>

* Add missing translations

Signed-off-by: Haris Gušić <harisgusic.dev@gmail.com>

* Replace variable i with row

Signed-off-by: Haris Gušić <harisgusic.dev@gmail.com>

* Improve presentation

Signed-off-by: Haris Gušić <harisgusic.dev@gmail.com>

* Disambiguate shortcuts and general settings

Signed-off-by: Haris Gušić <harisgusic.dev@gmail.com>

* Wrap some strings in tr

Signed-off-by: Haris Gušić <harisgusic.dev@gmail.com>

* Update tooltips in ConfigResolver

Signed-off-by: Haris Gušić <harisgusic.dev@gmail.com>

src/config/CMakeLists.txt

@@ -3,6 +3,8 @@ target_sources(
   PRIVATE buttonlistview.cpp
           clickablelabel.cpp
           configwindow.cpp
+          configresolver.cpp
+          configerrordetails.cpp
           extendedslider.cpp
           filenameeditor.cpp
           generalconf.cpp

src/config/configerrordetails.cpp

@@ -0,0 +1,42 @@
+#include "src/config/configerrordetails.h"
+
+#include "src/utils/abstractlogger.h"
+#include "src/utils/confighandler.h"
+
+#include <QApplication>
+#include <QDialogButtonBox>
+#include <QTextEdit>
+#include <QVBoxLayout>
+
+ConfigErrorDetails::ConfigErrorDetails(QWidget* parent)
+  : QDialog(parent)
+{
+    // Generate error log message
+    QString str;
+    AbstractLogger stream(str, AbstractLogger::Error);
+    ConfigHandler().checkForErrors(&stream);
+
+    // Set up dialog
+    setWindowTitle(tr("Configuration errors"));
+    setLayout(new QVBoxLayout(this));
+
+    // Add text display
+    QTextEdit* textDisplay = new QTextEdit(this);
+    textDisplay->setPlainText(str);
+    textDisplay->setReadOnly(true);
+    layout()->addWidget(textDisplay);
+
+    // Add Ok button
+    using BBox = QDialogButtonBox;
+    BBox* buttons = new BBox(BBox::Ok);
+    layout()->addWidget(buttons);
+    connect(buttons, &BBox::clicked, this, [this]() { close(); });
+
+    show();
+
+    qApp->processEvents();
+    QPoint center = geometry().center();
+    QRect dialogRect(0, 0, 600, 400);
+    dialogRect.moveCenter(center);
+    setGeometry(dialogRect);
+}

src/config/configerrordetails.h

@@ -0,0 +1,9 @@
+#include <QDialog>
+
+#pragma once
+
+class ConfigErrorDetails : public QDialog
+{
+public:
+    ConfigErrorDetails(QWidget* parent = nullptr);
+};

src/config/configresolver.cpp

@@ -0,0 +1,142 @@
+#include "src/config/configresolver.h"
+#include "src/config/configerrordetails.h"
+#include "src/utils/confighandler.h"
+
+#include "src/utils/valuehandler.h"
+#include <QDialogButtonBox>
+#include <QLabel>
+#include <QSplitter>
+#include <QVBoxLayout>
+
+ConfigResolver::ConfigResolver(QWidget* parent)
+  : QDialog(parent)
+{
+    setWindowTitle(tr("Resolve configuration errors"));
+    setMinimumSize({ 250, 200 });
+    setSizePolicy(QSizePolicy::Maximum, QSizePolicy::Maximum);
+    populate();
+    connect(ConfigHandler::getInstance(),
+            &ConfigHandler::fileChanged,
+            this,
+            [this]() { populate(); });
+}
+
+QGridLayout* ConfigResolver::layout()
+{
+    return dynamic_cast<QGridLayout*>(QDialog::layout());
+}
+
+void ConfigResolver::populate()
+{
+    ConfigHandler config;
+    QList<QString> unrecognized;
+    QList<QString> semanticallyWrong;
+
+    config.checkUnrecognizedSettings(nullptr, &unrecognized);
+    config.checkSemantics(nullptr, &semanticallyWrong);
+
+    // Remove previous layout and children, if any
+    resetLayout();
+
+    bool anyErrors = !semanticallyWrong.isEmpty() || !unrecognized.isEmpty();
+    int row = 0;
+
+    // No errors detected
+    if (!anyErrors) {
+        accept();
+    } else {
+        layout()->addWidget(
+          new QLabel(
+            tr("<b>You must resolve all errors before continuing:</b>")),
+          0,
+          0,
+          1,
+          2);
+        ++row;
+    }
+
+    // List semantically incorrect settings with a "Reset" button
+    for (const auto& key : semanticallyWrong) {
+        auto* label = new QLabel(key);
+        auto* reset = new QPushButton(tr("Reset"));
+        label->setToolTip("This setting has a bad value.");
+        reset->setToolTip(tr("Reset to the default value."));
+        layout()->addWidget(label, row, 0);
+        layout()->addWidget(reset, row, 1);
+        reset->setSizePolicy(QSizePolicy::Fixed, QSizePolicy::Fixed);
+
+        connect(reset, &QPushButton::clicked, this, [key]() {
+            ConfigHandler().resetValue(key);
+        });
+
+        ++row;
+    }
+    // List unrecognized settings with a "Remove" button
+    for (const auto& key : unrecognized) {
+        auto* label = new QLabel(key);
+        auto* remove = new QPushButton(tr("Remove"));
+        label->setToolTip("This setting is unrecognized.");
+        remove->setToolTip(tr("Remove this setting."));
+        layout()->addWidget(label, row, 0);
+        layout()->addWidget(remove, row, 1);
+        connect(remove, &QPushButton::clicked, this, [key]() {
+            ConfigHandler().remove(key);
+        });
+        ++row;
+    }
+
+    if (!config.checkShortcutConflicts()) {
+        auto* conflicts = new QLabel(
+          tr("Some keyboard shortcuts have conflicts.\n"
+             "This will NOT prevent flameshot from starting.\n"
+             "Please solve them manually in the configuration file."));
+        conflicts->setWordWrap(true);
+        conflicts->setMaximumWidth(geometry().width());
+        conflicts->setSizePolicy(QSizePolicy::Minimum, QSizePolicy::Maximum);
+        layout()->addWidget(conflicts, row, 0, 1, 2, Qt::AlignCenter);
+        ++row;
+    }
+
+    QFrame* separator = new QFrame(this);
+    separator->setFrameShape(QFrame::HLine);
+    separator->setFrameShadow(QFrame::Sunken);
+    layout()->addWidget(separator, row, 0, 1, 2);
+    ++row;
+
+    using BBox = QDialogButtonBox;
+
+    // Add button box at the bottom
+    auto* buttons = new BBox(this);
+    layout()->addWidget(buttons, row, 0, 1, 2, Qt::AlignCenter);
+    if (anyErrors) {
+        QPushButton* resolveAll = new QPushButton(tr("Resolve all"));
+        resolveAll->setToolTip(tr("Resolve all listed errors."));
+        buttons->addButton(resolveAll, BBox::ResetRole);
+        connect(resolveAll, &QPushButton::clicked, this, [=]() {
+            for (const auto& key : semanticallyWrong)
+                ConfigHandler().resetValue(key);
+            for (const auto& key : unrecognized)
+                ConfigHandler().remove(key);
+        });
+    }
+
+    QPushButton* details = new QPushButton(tr("Details"));
+    buttons->addButton(details, BBox::HelpRole);
+    connect(details, &QPushButton::clicked, this, [this]() {
+        (new ConfigErrorDetails(this))->exec();
+    });
+
+    buttons->addButton(BBox::Cancel);
+
+    connect(buttons, &BBox::rejected, this, [this]() { reject(); });
+}
+
+void ConfigResolver::resetLayout()
+{
+    for (auto* child : children()) {
+        child->deleteLater();
+    }
+    delete layout();
+    setLayout(new QGridLayout());
+    layout()->setSizeConstraint(QLayout::SetFixedSize);
+}

src/config/configresolver.h

@@ -0,0 +1,17 @@
+#pragma once
+
+#include <QDialog>
+
+class QGridLayout;
+
+class ConfigResolver : public QDialog
+{
+public:
+    ConfigResolver(QWidget* parent = nullptr);
+
+    QGridLayout* layout();
+
+private:
+    void populate();
+    void resetLayout();
+};

src/config/configwindow.cpp

@@ -3,6 +3,7 @@
 
 #include "configwindow.h"
 #include "abstractlogger.h"
+#include "src/config/configresolver.h"
 #include "src/config/filenameeditor.h"
 #include "src/config/generalconf.h"
 #include "src/config/shortcutswidget.h"
@@ -118,7 +119,7 @@ void ConfigWindow::keyPressEvent(QKeyEvent* e)
 void ConfigWindow::initErrorIndicator(QWidget* tab, QWidget* widget)
 {
     QLabel* label = new QLabel(tab);
-    QPushButton* btnShowErrors = new QPushButton("Show errors", tab);
+    QPushButton* btnResolve = new QPushButton(tr("Resolve"), tab);
     QHBoxLayout* btnLayout = new QHBoxLayout();
 
     // Set up label
@@ -129,9 +130,9 @@ void ConfigWindow::initErrorIndicator(QWidget* tab, QWidget* widget)
     label->setVisible(ConfigHandler().hasError());
 
     // Set up "Show errors" button
-    btnShowErrors->setSizePolicy(QSizePolicy::Maximum, QSizePolicy::Fixed);
-    btnLayout->addWidget(btnShowErrors);
-    btnShowErrors->setVisible(ConfigHandler().hasError());
+    btnResolve->setSizePolicy(QSizePolicy::Maximum, QSizePolicy::Fixed);
+    btnLayout->addWidget(btnResolve);
+    btnResolve->setVisible(ConfigHandler().hasError());
 
     widget->setEnabled(!ConfigHandler().hasError());
 
@@ -142,14 +143,14 @@ void ConfigWindow::initErrorIndicator(QWidget* tab, QWidget* widget)
         layout->insertLayout(1, btnLayout);
     } else {
         widget->layout()->addWidget(label);
-        widget->layout()->addWidget(btnShowErrors);
+        widget->layout()->addWidget(btnResolve);
     }
 
     // Sigslots
     connect(ConfigHandler::getInstance(), &ConfigHandler::error, widget, [=]() {
         widget->setEnabled(false);
         label->show();
-        btnShowErrors->show();
+        btnResolve->show();
     });
     connect(ConfigHandler::getInstance(),
             &ConfigHandler::errorResolved,
@@ -157,41 +158,9 @@ void ConfigWindow::initErrorIndicator(QWidget* tab, QWidget* widget)
             [=]() {
                 widget->setEnabled(true);
                 label->hide();
-                btnShowErrors->hide();
+                btnResolve->hide();
             });
-    connect(btnShowErrors, &QPushButton::clicked, this, [this]() {
-        // Generate error log message
-        QString str;
-        AbstractLogger stream(str, AbstractLogger::Error);
-        ConfigHandler().checkForErrors(&stream);
-
-        // Set up dialog
-        QDialog dialog;
-        dialog.setWindowTitle(QStringLiteral("Configuration errors"));
-        dialog.setLayout(new QVBoxLayout(&dialog));
-
-        // Add text display
-        QTextEdit* textDisplay = new QTextEdit(&dialog);
-        textDisplay->setPlainText(str);
-        textDisplay->setReadOnly(true);
-        dialog.layout()->addWidget(textDisplay);
-
-        // Add Ok button
-        using BBox = QDialogButtonBox;
-        BBox* buttons = new BBox(BBox::Ok);
-        dialog.layout()->addWidget(buttons);
-        connect(buttons, &QDialogButtonBox::clicked, this, [&dialog]() {
-            dialog.close();
-        });
-
-        dialog.show();
-
-        qApp->processEvents();
-        QPoint center = dialog.geometry().center();
-        QRect dialogRect(0, 0, 600, 400);
-        dialogRect.moveCenter(center);
-        dialog.setGeometry(dialogRect);
-
-        dialog.exec();
+    connect(btnResolve, &QPushButton::clicked, this, [this]() {
+        ConfigResolver().exec();
     });
 }

src/core/controller.cpp

@@ -11,6 +11,7 @@
 #include "abstractlogger.h"
 #include "pinwidget.h"
 #include "screenshotsaver.h"
+#include "src/config/configresolver.h"
 #include "src/config/configwindow.h"
 #include "src/core/qguiappcurrentscreen.h"
 #include "src/tools/imgupload/imguploadermanager.h"
@@ -133,6 +134,16 @@ void Controller::setCheckForUpdatesEnabled(const bool enabled)
     }
 }
 
+void Controller::setOrigin(Origin origin)
+{
+    m_origin = origin;
+}
+
+Controller::Origin Controller::origin()
+{
+    return m_origin;
+}
+
 void Controller::getLatestAvailableVersion()
 {
     // This features is required for MacOS and Windows user and for Linux users
@@ -153,6 +164,38 @@ void Controller::getLatestAvailableVersion()
     });
 }
 
+/**
+ * @brief Prompt the user to resolve config errors if necessary.
+ * @return Whether errors were resolved.
+ */
+bool Controller::resolveAnyConfigErrors()
+{
+    bool resolved = true;
+    ConfigHandler config;
+    if (!config.checkUnrecognizedSettings() || !config.checkSemantics()) {
+        ConfigResolver* resolver = new ConfigResolver();
+        QObject::connect(
+          resolver, &ConfigResolver::rejected, [this, resolver, &resolved]() {
+              resolved = false;
+              resolver->deleteLater();
+              if (origin() == CLI) {
+                  exit(1);
+              }
+          });
+        QObject::connect(
+          resolver, &ConfigResolver::accepted, [resolver, &resolved]() {
+              resolved = true;
+              resolver->close();
+              resolver->deleteLater();
+              // Ensure that the dialog is closed before starting capture
+              qApp->processEvents();
+          });
+        resolver->exec();
+        qApp->processEvents();
+    }
+    return resolved;
+}
+
 void Controller::handleReplyCheckUpdates(QNetworkReply* reply)
 {
     if (!ConfigHandler().checkForUpdates()) {
@@ -207,6 +250,9 @@ void Controller::appUpdates()
 
 void Controller::requestCapture(const CaptureRequest& request)
 {
+    if (!resolveAnyConfigErrors())
+        return;
+
     switch (request.captureMode()) {
         case CaptureRequest::FULLSCREEN_MODE:
             doLater(request.delay(), this, [this, request]() {
@@ -235,6 +281,9 @@ void Controller::requestCapture(const CaptureRequest& request)
 // creation of a new capture in GUI mode
 void Controller::startVisualCapture(const CaptureRequest& req)
 {
+    if (!resolveAnyConfigErrors())
+        return;
+
 #if defined(Q_OS_MACOS)
     // This is required on MacOS because of Mission Control. If you'll switch to
     // another Desktop you cannot take a new screenshot from the tray, you have
@@ -295,6 +344,9 @@ void Controller::startVisualCapture(const CaptureRequest& req)
 
 void Controller::startScreenGrab(CaptureRequest req, const int screenNumber)
 {
+    if (!resolveAnyConfigErrors())
+        return;
+
     bool ok = true;
     QScreen* screen;
 
@@ -334,6 +386,9 @@ void Controller::startScreenGrab(CaptureRequest req, const int screenNumber)
 // creation of the configuration window
 void Controller::openConfigWindow()
 {
+    if (!resolveAnyConfigErrors())
+        return;
+
     if (!m_configWindow) {
         m_configWindow = new ConfigWindow();
         m_configWindow->show();
@@ -358,6 +413,9 @@ void Controller::openInfoWindow()
 
 void Controller::openLauncherWindow()
 {
+    if (!resolveAnyConfigErrors())
+        return;
+
     if (!m_launcherWindow) {
         m_launcherWindow = new CaptureLauncher();
     }
@@ -629,6 +687,9 @@ void Controller::exportCapture(QPixmap capture,
 
 void Controller::startFullscreenCapture(const CaptureRequest& req)
 {
+    if (!resolveAnyConfigErrors())
+        return;
+
     bool ok = true;
     QPixmap p(ScreenGrabber().grabEntireDesktop(ok));
     QRect region = req.initialSelection();
@@ -665,3 +726,6 @@ void Controller::doLater(int msec, QObject* receiver, lambda func)
     timer->setInterval(msec);
     timer->start();
 }
+
+// STATIC ATTRIBUTES
+Controller::Origin Controller::m_origin = Controller::DAEMON;

src/core/controller.h

@@ -31,9 +31,17 @@ class Controller : public QObject
     Q_OBJECT
 
 public:
+    enum Origin
+    {
+        CLI,
+        DAEMON
+    };
+
     static Controller* getInstance();
 
     void setCheckForUpdatesEnabled(const bool enabled);
+    static void setOrigin(Origin origin);
+    static Origin origin();
 
 signals:
     // TODO remove all parameters from captureTaken and update dependencies
@@ -51,14 +59,14 @@ public slots:
     void initTrayIcon();
     void enableTrayIcon();
     void disableTrayIcon();
-    void sendTrayNotification(
-      const QString& text,
-      const QString& title = QStringLiteral("Flameshot Info"),
-      const int timeout = 5000);
 
     void showRecentUploads();
 
     void exportCapture(QPixmap p, QRect& selection, const CaptureRequest& req);
+    void sendTrayNotification(
+      const QString& text,
+      const QString& title = QStringLiteral("Flameshot Info"),
+      const int timeout = 5000);
 
 private slots:
     void startFullscreenCapture(const CaptureRequest& req);
@@ -78,6 +86,7 @@ private:
     Controller();
     ~Controller();
     void getLatestAvailableVersion();
+    bool resolveAnyConfigErrors();
 
     // replace QTimer::singleShot introduced in Qt 5.4
     // the actual target Qt version is 5.3
@@ -88,6 +97,7 @@ private:
     QString m_appLatestUrl;
     QString m_appLatestVersion;
     bool m_showCheckAppUpdateStatus;
+    static Origin m_origin;
 
     QPointer<CaptureWidget> m_captureWindow;
     QPointer<InfoWindow> m_infoWindow;

src/main.cpp

@@ -328,6 +328,7 @@ int main(int argc, char* argv[])
 
     // PROCESS DATA
     //--------------
+    Controller::setOrigin(Controller::CLI);
     if (parser.isSet(helpOption) || parser.isSet(versionOption)) {
     } else if (parser.isSet(launcherArgument)) { // LAUNCHER
         delete qApp;
@@ -394,12 +395,14 @@ int main(int argc, char* argv[])
                 req.addSaveTask();
             }
         }
+
         requestCaptureAndWait(req);
     } else if (parser.isSet(fullArgument)) { // FULL
         // Recreate the application as a QApplication
         // TODO find a way so we don't have to do this
         delete qApp;
         new QApplication(argc, argv);
+
         // Option values
         QString path = parser.value(pathOption);
         if (!path.isEmpty()) {
@@ -437,6 +440,7 @@ int main(int argc, char* argv[])
         // TODO find a way so we don't have to do this
         delete qApp;
         new QApplication(argc, argv);
+
         QString numberStr = parser.value(screenNumberOption);
         // Option values
         int number =
@@ -495,7 +499,7 @@ int main(int argc, char* argv[])
           (filename || tray || mainColor || contrastColor || check);
         if (check) {
             AbstractLogger err = AbstractLogger::error(AbstractLogger::Stderr);
-            bool ok = ConfigHandler(true).checkForErrors(&err);
+            bool ok = ConfigHandler().checkForErrors(&err);
             if (ok) {
                 err << QStringLiteral("No errors detected.\n");
                 goto finish;
@@ -503,43 +507,45 @@ int main(int argc, char* argv[])
                 return 1;
             }
         }
-        ConfigHandler config;
-        if (autostart) {
-            config.setStartupLaunch(parser.value(autostartOption) == "true");
-        }
-        if (filename) {
-            QString newFilename(parser.value(filenameOption));
-            config.setFilenamePattern(newFilename);
-            FileNameHandler fh;
-            QTextStream(stdout)
-              << QStringLiteral("The new pattern is '%1'\n"
-                                "Parsed pattern example: %2\n")
-                   .arg(newFilename)
-                   .arg(fh.parsedPattern());
-        }
-        if (tray) {
-            config.setDisabledTrayIcon(parser.value(trayOption) == "false");
-        }
-        if (mainColor) {
-            // TODO use value handler
-            QString colorCode = parser.value(mainColorOption);
-            QColor parsedColor(colorCode);
-            config.setUiColor(parsedColor);
-        }
-        if (contrastColor) {
-            QString colorCode = parser.value(contrastColorOption);
-            QColor parsedColor(colorCode);
-            config.setContrastUiColor(parsedColor);
-        }
-
-        // Open gui when no options
         if (!someFlagSet) {
+            // Open gui when no options are given
             delete qApp;
             new QApplication(argc, argv);
             QObject::connect(
               qApp, &QApplication::lastWindowClosed, qApp, &QApplication::quit);
             Controller::getInstance()->openConfigWindow();
             qApp->exec();
+        } else {
+            ConfigHandler config;
+
+            if (autostart) {
+                config.setStartupLaunch(parser.value(autostartOption) ==
+                                        "true");
+            }
+            if (filename) {
+                QString newFilename(parser.value(filenameOption));
+                config.setFilenamePattern(newFilename);
+                FileNameHandler fh;
+                QTextStream(stdout)
+                  << QStringLiteral("The new pattern is '%1'\n"
+                                    "Parsed pattern example: %2\n")
+                       .arg(newFilename)
+                       .arg(fh.parsedPattern());
+            }
+            if (tray) {
+                config.setDisabledTrayIcon(parser.value(trayOption) == "false");
+            }
+            if (mainColor) {
+                // TODO use value handler
+                QString colorCode = parser.value(mainColorOption);
+                QColor parsedColor(colorCode);
+                config.setUiColor(parsedColor);
+            }
+            if (contrastColor) {
+                QString colorCode = parser.value(contrastColorOption);
+                QColor parsedColor(colorCode);
+                config.setContrastUiColor(parsedColor);
+            }
         }
     }
 finish:

src/utils/confighandler.cpp

@@ -173,19 +173,13 @@ static QMap<QString, QSharedPointer<KeySequence>> recognizedShortcuts = {
 
 // CLASS CONFIGHANDLER
 
-ConfigHandler::ConfigHandler(bool skipInitialErrorCheck)
+ConfigHandler::ConfigHandler()
   : m_settings(QSettings::IniFormat,
                QSettings::UserScope,
                qApp->organizationName(),
                qApp->applicationName())
 {
-    static bool wasEverChecked = false;
     static bool firstInitialization = true;
-    if (!skipInitialErrorCheck && !wasEverChecked) {
-        // check for error on initial call
-        checkAndHandleError();
-        wasEverChecked = true;
-    }
     if (firstInitialization) {
         // check for error every time the file changes
         m_configWatcher.reset(new QFileSystemWatcher());
@@ -449,9 +443,6 @@ void ConfigHandler::setValue(const QString& key, const QVariant& value)
 QVariant ConfigHandler::value(const QString& key) const
 {
     assertKeyRecognized(key);
-    // Perform check on entire config if due. Please make sure that this
-    // function is called in all scenarios - best to keep it on top.
-    hasError();
 
     auto val = m_settings.value(key);
 
@@ -468,6 +459,16 @@ QVariant ConfigHandler::value(const QString& key) const
     return handler->value(val);
 }
 
+void ConfigHandler::remove(const QString& key)
+{
+    m_settings.remove(key);
+}
+
+void ConfigHandler::resetValue(const QString& key)
+{
+    m_settings.setValue(key, valueHandler(key)->fallback());
+}
+
 QSet<QString>& ConfigHandler::recognizedGeneralOptions()
 {
 #if QT_VERSION >= QT_VERSION_CHECK(5, 14, 0)
@@ -492,8 +493,10 @@ QSet<QString>& ConfigHandler::recognizedShortcutNames()
     return names;
 }
 
-// Return keys from group `group`. Use CONFIG_GROUP_GENERAL (General) for
-// general settings.
+/**
+ * @brief Return keys from group `group`.
+ * Use CONFIG_GROUP_GENERAL (General) for general settings.
+ */
 QSet<QString> ConfigHandler::keysFromGroup(const QString& group) const
 {
     QSet<QString> keys;
@@ -515,20 +518,6 @@ bool ConfigHandler::checkForErrors(AbstractLogger* log) const
            checkSemantics(log);
 }
 
-void ConfigHandler::cleanUnusedKeys(const QString& group,
-                                    const QSet<QString>& keys) const
-{
-    for (const QString& key : keys) {
-        if (group == CONFIG_GROUP_GENERAL && !key.contains('/')) {
-            m_settings.remove(key);
-        } else {
-            m_settings.beginGroup(group);
-            m_settings.remove(key);
-            m_settings.endGroup();
-        }
-    }
-}
-
 /**
  * @brief Parse the config to find settings with unrecognized names.
  * @return Whether the config passes this check.
@@ -537,7 +526,8 @@ void ConfigHandler::cleanUnusedKeys(const QString& group,
  * `recognizedGeneralOptions` or `recognizedShortcutNames` depending on the
  * group the option belongs to.
  */
-bool ConfigHandler::checkUnrecognizedSettings(AbstractLogger* log) const
+bool ConfigHandler::checkUnrecognizedSettings(AbstractLogger* log,
+                                              QList<QString>* offenders) const
 {
     // sort the config keys by group
     QSet<QString> generalKeys = keysFromGroup(CONFIG_GROUP_GENERAL),
@@ -549,38 +539,20 @@ bool ConfigHandler::checkUnrecognizedSettings(AbstractLogger* log) const
     generalKeys.subtract(recognizedGeneralKeys);
     shortcutKeys.subtract(recognizedShortcutKeys);
 
-    // automatically clean up unused keys
-    if (!generalKeys.isEmpty()) {
-        cleanUnusedKeys(CONFIG_GROUP_GENERAL, generalKeys);
-        generalKeys = keysFromGroup(CONFIG_GROUP_GENERAL),
-        generalKeys.subtract(recognizedGeneralKeys);
-    }
-    if (!shortcutKeys.isEmpty()) {
-        cleanUnusedKeys(CONFIG_GROUP_SHORTCUTS, shortcutKeys);
-        shortcutKeys = keysFromGroup(CONFIG_GROUP_SHORTCUTS);
-        shortcutKeys.subtract(recognizedShortcutKeys);
-    }
-
-    // clean up unused groups
-    QStringList settingsGroups = m_settings.childGroups();
-    for (const auto& group : settingsGroups) {
-        if (group != QLatin1String(CONFIG_GROUP_SHORTCUTS) &&
-            group != QLatin1String(CONFIG_GROUP_GENERAL)) {
-            m_settings.beginGroup(group);
-            m_settings.remove("");
-            m_settings.endGroup();
-        }
-    }
-
     // what is left are the unrecognized keys - hopefully empty
     bool ok = generalKeys.isEmpty() && shortcutKeys.isEmpty();
-    if (log != nullptr) {
+    if (log != nullptr || offenders != nullptr) {
         for (const QString& key : generalKeys) {
-            *log << QStringLiteral("Unrecognized setting: '%1'\n").arg(key);
+            if (log)
+                *log << tr("Unrecognized setting: '%1'\n").arg(key);
+            if (offenders)
+                offenders->append(key);
         }
         for (const QString& key : shortcutKeys) {
-            *log
-              << QStringLiteral("Unrecognized shortcut name: '%1'.\n").arg(key);
+            if (log)
+                *log << tr("Unrecognized shortcut name: '%1'.\n").arg(key);
+            if (offenders)
+                offenders->append(CONFIG_GROUP_SHORTCUTS "/" + key);
         }
     }
     return ok;
@@ -619,8 +591,8 @@ bool ConfigHandler::checkShortcutConflicts(AbstractLogger* log) const
                            !reportedInLog.contains(*key2)) { // log entries
                     reportedInLog.append(*key1);
                     reportedInLog.append(*key2);
-                    *log << QStringLiteral("Shortcut conflict: '%1' and '%2' "
-                                           "have the same shortcut: %3\n")
+                    *log << tr("Shortcut conflict: '%1' and '%2' "
+                               "have the same shortcut: %3\n")
                               .arg(*key1)
                               .arg(*key2)
                               .arg(value1);
@@ -634,9 +606,12 @@ bool ConfigHandler::checkShortcutConflicts(AbstractLogger* log) const
 
 /**
  * @brief Check each config value semantically.
+ * @param log Destination for error log output.
+ * @param offenders Destination for the semantically invalid keys.
  * @return Whether the config passes this check.
  */
-bool ConfigHandler::checkSemantics(AbstractLogger* log) const
+bool ConfigHandler::checkSemantics(AbstractLogger* log,
+                                   QList<QString>* offenders) const
 {
     QStringList allKeys = m_settings.allKeys();
     bool ok = true;
@@ -650,14 +625,17 @@ bool ConfigHandler::checkSemantics(AbstractLogger* log) const
         QVariant val = m_settings.value(key);
         auto valueHandler = this->valueHandler(key);
         if (val.isValid() && !valueHandler->check(val)) {
+            // Key does not pass the check
             ok = false;
-            if (log == nullptr) {
+            if (log == nullptr && offenders == nullptr)
                 break;
-            } else {
-                *log << QStringLiteral("Semantic error in '%1'. Expected: %2\n")
+            if (log != nullptr) {
+                *log << tr("Bad value in '%1'. Expected: %2\n")
                           .arg(key)
                           .arg(valueHandler->expected());
             }
+            if (offenders != nullptr)
+                offenders->append(key);
         }
     }
     return ok;
@@ -724,7 +702,8 @@ bool ConfigHandler::hasError() const
 /// Error message that can be used by other classes as well
 QString ConfigHandler::errorMessage() const
 {
-    return tr("The configuration contains an error. Falling back to default.");
+    return tr(
+      "The configuration contains an error. Open configuration to resolve.");
 }
 
 void ConfigHandler::ensureFileWatched() const
@@ -801,7 +780,7 @@ QString ConfigHandler::baseName(QString key) const
 // STATIC MEMBER DEFINITIONS
 
 bool ConfigHandler::m_hasError = false;
-bool ConfigHandler::m_errorCheckPending = false;
+bool ConfigHandler::m_errorCheckPending = true;
 bool ConfigHandler::m_skipNextErrorCheck = false;
 
 QSharedPointer<QFileSystemWatcher> ConfigHandler::m_configWatcher;

src/utils/confighandler.h

@@ -58,7 +58,7 @@ class ConfigHandler : public QObject
     Q_OBJECT
 
 public:
-    explicit ConfigHandler(bool skipInitialErrorCheck = false);
+    explicit ConfigHandler();
 
     static ConfigHandler* getInstance();
 
@@ -133,6 +133,8 @@ public:
     QString shortcut(const QString& actionName);
     void setValue(const QString& key, const QVariant& value);
     QVariant value(const QString& key) const;
+    void remove(const QString& key);
+    void resetValue(const QString& key);
 
     // INFO
     static QSet<QString>& recognizedGeneralOptions();
@@ -141,9 +143,11 @@ public:
 
     // ERROR HANDLING
     bool checkForErrors(AbstractLogger* log = nullptr) const;
-    bool checkUnrecognizedSettings(AbstractLogger* log = nullptr) const;
+    bool checkUnrecognizedSettings(AbstractLogger* log = nullptr,
+                                   QList<QString>* offenders = nullptr) const;
     bool checkShortcutConflicts(AbstractLogger* log = nullptr) const;
-    bool checkSemantics(AbstractLogger* log = nullptr) const;
+    bool checkSemantics(AbstractLogger* log = nullptr,
+                        QList<QString>* offenders = nullptr) const;
     void checkAndHandleError() const;
     void setErrorState(bool error) const;
     bool hasError() const;