Only show close to tray reminder when not quitting the app (#7432)

* Improve trusted fingerprint message

* Fixed typo

* Improve grammar

* Simplify logging output

* Only show close reminder if not quitting

* Migrate old settings on macOS

* Raise main window when showing

* Raise critical error above main window

* Coverage for config scopes

* Fixed new client prompt

* Remove unused includes

* Further coverage for config scopes

* Update ChangeLog

ChangeLog

@@ -75,6 +75,7 @@ Enhancements:
 - #7429 Parse date numbers as long instead of int
 - #7430 Improve setting enable logic and test coverage
 - #7431 Improve handling of Qt-related warnings and errors
+- #7432 Only show close to tray reminder when not quitting the app
 
 # 1.14.6
 

src/gui/src/MainWindow.cpp

@@ -259,7 +259,11 @@ void MainWindow::connectSlots() {
       m_pActionRestore, &QAction::triggered, //
       [this]() { showAndActivate(); });
 
-  connect(m_pActionQuit, &QAction::triggered, qApp, &QCoreApplication::quit);
+  connect(m_pActionQuit, &QAction::triggered, qApp, [this] {
+    qDebug("quitting application");
+    m_Quitting = true;
+    qApp->quit();
+  });
 
   connect(
       &m_VersionChecker, &VersionChecker::updateFound, this,
@@ -316,6 +320,17 @@ void MainWindow::onShown() {
       showActivationDialog();
     }
   }
+
+  // if a critical error was shown just before the main window (i.e. on app
+  // load), it will be hidden behind the main window. therefore we need to raise
+  // it up in front of the main window.
+  // HACK: because the `onShown` event happens just as the window is shown, the
+  // message box has a chance of being raised under the main window. to solve
+  // this we delay the error dialog raise by a split second. this seems a bit
+  // hacky and fragile, so maybe there's a better approach.
+  const auto kCriticalDialogDelay = 100;
+  QTimer::singleShot(
+      kCriticalDialogDelay, [] { messages::raiseCriticalDialog(); });
 }
 
 void MainWindow::onLicenseHandlerSerialKeyChanged(const QString &serialKey) {
@@ -674,7 +689,9 @@ void MainWindow::handleLogLine(const QString &line) {
   const auto scrollAtBottom =
       qAbs(currentScroll - maxScroll) <= kScrollBottomThreshold;
 
-  m_pLogOutput->appendPlainText(line);
+  // only trim end instead of the whole line to prevent tab-indented debug
+  // filenames from losing their indentation.
+  m_pLogOutput->appendPlainText(trimEnd(line));
 
   if (scrollAtBottom) {
     verticalScroll->setValue(verticalScroll->maximum());
@@ -730,14 +747,14 @@ void MainWindow::checkFingerprint(const QString &line) {
     QMessageBox::StandardButton fingerprintReply = QMessageBox::information(
         this, QString("Security question"),
         QString(
-            "You are connecting to a server. Here is it's fingerprint:\n\n"
+            "<p>You are connecting to a server.</p>"
-            "%1\n\n"
+            "<p>Here is it's TLS fingerprint:</p>"
-            "Compare this fingerprint to the one on your server's screen."
+            "<p>%1</p>"
+            "<p>Compare this fingerprint to the one on your server's screen. "
             "If the two don't match exactly, then it's probably not the server "
-            "you're expecting (it could be a malicious user).\n\n"
+            "you're expecting (it could be a malicious user).</p>"
-            "To automatically trust this fingerprint for future "
+            "<p>Do you want to trust this fingerprint for future "
-            "connections, click Yes. To reject this fingerprint and "
+            "connections? If you don't, a connection cannot be made.</p>")
-            "disconnect from the server, click No.")
             .arg(fingerprint),
         QMessageBox::Yes | QMessageBox::No);
 
@@ -762,18 +779,23 @@ void MainWindow::showEvent(QShowEvent *event) {
 }
 
 void MainWindow::closeEvent(QCloseEvent *event) {
+  if (m_Quitting) {
+    qDebug("skipping close event handle on quit");
+    return;
+  }
+
   if (!m_AppConfig.closeToTray()) {
+    qDebug("window will not hide to tray");
     return;
   }
 
-  qDebug("window will hide to tray");
+  if (m_AppConfig.showCloseReminder()) {
-  if (!m_AppConfig.showCloseReminder()) {
+    messages::showCloseReminder(this);
-    return;
+    m_AppConfig.setShowCloseReminder(false);
   }
 
-  messages::showCloseReminder(this);
-  m_AppConfig.setShowCloseReminder(false);
   m_ConfigScopes.save();
+  qDebug("window should hide to tray");
 }
 
 void MainWindow::showFirstRunMessage() {
@@ -791,7 +813,7 @@ void MainWindow::showFirstRunMessage() {
 
 void MainWindow::showDevThanksMessage() {
   if (!m_AppConfig.showDevThanks()) {
-    qDebug("skipping dev thanks message, disabled in settings");
+    qDebug("skipping dev thanks message");
     return;
   }
 
@@ -1139,5 +1161,6 @@ void MainWindow::showAndActivate() {
   }
 
   showNormal();
+  raise();
   activateWindow();
 }

src/gui/src/MainWindow.h

@@ -92,7 +92,6 @@ private slots:
   //
   // Manual slots
   //
-
   void onCreated();
   void onShown();
   void onConfigScopesSaving();
@@ -194,6 +193,7 @@ private:
   bool m_SecureSocket = false;
   bool m_SaveWindow = false;
   LicenseHandler m_LicenseHandler;
+  bool m_Quitting = false;
 
   synergy::gui::ConfigScopes &m_ConfigScopes;
   AppConfig &m_AppConfig;

src/gui/src/main.cpp

@@ -66,8 +66,8 @@ int main(int argc, char *argv[]) {
   // HACK: set org name to app name for backwards compatibility.
   QCoreApplication::setOrganizationName(kAppName);
 
-  // HACK: set org domain to url for backwards compatibility.
+  // used as a prefix for settings paths, and must not be a url.
-  QCoreApplication::setOrganizationDomain(kUrlWebsite);
+  QCoreApplication::setOrganizationDomain(kOrgDomain);
 
   QSynergyApplication app(argc, argv);
 

src/lib/gui/Logger.cpp

@@ -55,24 +55,23 @@ QString printLine(
   auto logLine = QString("[%1] %2: %3").arg(datetime).arg(type).arg(message);
 
   QTextStream stream(&logLine);
+  stream << Qt::endl;
+
   if (!fileLine.isEmpty()) {
-    stream << Qt::endl << "\t" + fileLine;
+    stream << "\t" + fileLine << Qt::endl;
   }
 
-  QString logLineReturn = logLine;
+  auto logLineBytes = logLine.toUtf8();
-  QTextStream streamReturn(&logLineReturn);
+  auto logLine_c = logLineBytes.constData();
-  streamReturn << Qt::endl;
-
-  auto logLineReturn_c = qPrintable(logLineReturn);
 
 #if defined(Q_OS_WIN)
   // Debug output is viewable using either VS Code, Visual Studio, DebugView, or
   // DbgView++ (only one can be used at once). It's important to send output to
   // the debug output API, because it's difficult to view stdout and stderr from
   // a Windows GUI app.
-  OutputDebugStringA(logLineReturn_c);
+  OutputDebugStringA(logLine_c);
 #else
-  fprintf(out, "%s", logLineReturn_c);
+  fprintf(out, "%s", logLine_c);
   fflush(out);
 #endif
 

src/lib/gui/config/ConfigScopes.cpp

@@ -17,99 +17,41 @@
 
 #include "ConfigScopes.h"
 
+#include "proxy/QSettingsProxy.h"
+
 #include <QCoreApplication>
 #include <QDir>
 #include <QFile>
+#include <QtCore/qfileinfo.h>
+#include <QtCore/qlogging.h>
 #include <memory>
 
-const auto kSystemConfigFilename = "SystemConfig.ini";
-
-#if defined(Q_OS_UNIX)
-const auto kUnixSystemConfigPath = "/usr/local/etc/symless/";
-#endif
-
 namespace synergy::gui {
 
 using namespace proxy;
 
-QString getSystemSettingPath() {
+//
-  const QString settingFilename(kSystemConfigFilename);
+// ConfigScopes::Deps
-#if defined(Q_OS_WIN)
+//
-  return QCoreApplication::applicationDirPath() + QDir::separator();
+
-#elif defined(Q_OS_MAC)
+std::shared_ptr<QSettingsProxy> ConfigScopes::Deps::makeUserSettings() {
-  // it would be nice to use /Library dir, but qt has no elevate system.
+  return std::make_shared<QSettingsProxy>();
-  return kUnixSystemConfigPath + settingFilename;
-#elif defined(Q_OS_LINUX)
-  // qt already adds application and filename to the end of the path on linux.
-  return kUnixSystemConfigPath;
-#else
-  qFatal("unsupported platform");
-  return "";
-#endif
 }
 
-#if defined(Q_OS_WIN)
+std::shared_ptr<QSettingsProxy> ConfigScopes::Deps::makeSystemSettings() {
-void loadWindowsLegacy(QSettings &settings) {
+  return std::make_shared<QSettingsProxy>();
-  if (QFile(settings.fileName()).exists()) {
-    qDebug("system settings already exist, skipping legacy load");
-    return;
-  }
-
-  QSettings::setPath(
-      QSettings::IniFormat, QSettings::SystemScope, kSystemConfigFilename);
-  QSettings oldSystemSettings(
-      QSettings::IniFormat, QSettings::SystemScope,
-      QCoreApplication::organizationName(),
-      QCoreApplication::applicationName());
-
-  if (QFile(oldSystemSettings.fileName()).exists()) {
-    for (const auto &key : oldSystemSettings.allKeys()) {
-      settings.setValue(key, oldSystemSettings.value(key));
-    }
-  }
-
-  QSettings::setPath(
-      QSettings::IniFormat, QSettings::SystemScope, getSystemSettingPath());
 }
-#endif
 
-ConfigScopes::ConfigScopes() {
+//
-  auto orgName = QCoreApplication::organizationName();
+// ConfigScopes
-  if (orgName.isEmpty()) {
+//
-    qFatal("unable to load config, organization name is empty");
-    return;
-  } else {
-    qDebug() << "org name for config:" << orgName;
-  }
 
-  auto appName = QCoreApplication::applicationName();
+ConfigScopes::ConfigScopes(std::shared_ptr<Deps> deps)
-  if (appName.isEmpty()) {
+    : m_pUserSettingsProxy(deps->makeUserSettings()),
-    qFatal("unable to load config, application name is empty");
+      m_pSystemSettingsProxy(deps->makeSystemSettings()) {
-    return;
-  } else {
-    qDebug() << "app name for config:" << appName;
-  }
 
-  // default to user scope.
+  m_pUserSettingsProxy->loadUser();
-  // if we set the scope specifically then we also have to set the application
+  m_pSystemSettingsProxy->loadSystem();
-  // name and the organisation name which breaks backwards compatibility.
-  m_pUserSettings = std::make_unique<QSettings>();
-  m_userSettingsProxy.set(*m_pUserSettings);
-
-  QSettings::setPath(
-      QSettings::Format::IniFormat, QSettings::Scope::SystemScope,
-      getSystemSettingPath());
-
-  // Config will default to User settings if they exist,
-  //  otherwise it will load System setting and save them to User settings
-  m_pSystemSettings = std::make_unique<QSettings>(
-      QSettings::Format::IniFormat, QSettings::Scope::SystemScope, orgName,
-      appName);
-  m_systemSettingsProxy.set(*m_pSystemSettings);
-
-#if defined(Q_OS_WIN)
-  loadWindowsLegacy(*m_pSystemSettings);
-#endif
 }
 
 void ConfigScopes::signalReady() { emit ready(); }
@@ -119,8 +61,8 @@ void ConfigScopes::save() {
   emit saving();
 
   qDebug("writing config to filesystem");
-  m_pUserSettings->sync();
+  m_pUserSettingsProxy->sync();
-  m_pSystemSettings->sync();
+  m_pSystemSettingsProxy->sync();
 }
 
 bool ConfigScopes::isActiveScopeWritable() const {
@@ -136,9 +78,9 @@ ConfigScopes::Scope ConfigScopes::activeScope() const { return m_currentScope; }
 bool ConfigScopes::scopeContains(const QString &name, Scope scope) const {
   switch (scope) {
   case Scope::User:
-    return m_pUserSettings->contains(name);
+    return m_pUserSettingsProxy->contains(name);
   case Scope::System:
-    return m_pSystemSettings->contains(name);
+    return m_pSystemSettingsProxy->contains(name);
   default:
     return activeSettings().contains(name);
   }
@@ -146,17 +88,17 @@ bool ConfigScopes::scopeContains(const QString &name, Scope scope) const {
 
 QSettingsProxy &ConfigScopes::activeSettings() {
   if (m_currentScope == Scope::User) {
-    return m_userSettingsProxy;
+    return *m_pUserSettingsProxy;
   } else {
-    return m_systemSettingsProxy;
+    return *m_pSystemSettingsProxy;
   }
 }
 
 const QSettingsProxy &ConfigScopes::activeSettings() const {
   if (m_currentScope == Scope::User) {
-    return m_userSettingsProxy;
+    return *m_pUserSettingsProxy;
   } else {
-    return m_systemSettingsProxy;
+    return *m_pSystemSettingsProxy;
   }
 }
 
@@ -168,9 +110,9 @@ QVariant ConfigScopes::getFromScope(
     const QString &name, const QVariant &defaultValue, Scope scope) const {
   switch (scope) {
   case Scope::User:
-    return m_pUserSettings->value(name, defaultValue);
+    return m_pUserSettingsProxy->value(name, defaultValue);
   case Scope::System:
-    return m_pSystemSettings->value(name, defaultValue);
+    return m_pSystemSettingsProxy->value(name, defaultValue);
   default:
     return activeSettings().value(name, defaultValue);
   }
@@ -180,10 +122,10 @@ void ConfigScopes::setInScope(
     const QString &name, const QVariant &value, Scope scope) {
   switch (scope) {
   case Scope::User:
-    m_pUserSettings->setValue(name, value);
+    m_pUserSettingsProxy->setValue(name, value);
     break;
   case Scope::System:
-    m_pSystemSettings->setValue(name, value);
+    m_pSystemSettingsProxy->setValue(name, value);
     break;
   default:
     activeSettings().setValue(name, value);

src/lib/gui/config/ConfigScopes.h

@@ -33,7 +33,13 @@ class ConfigScopes : public QObject, public IConfigScopes {
   Q_OBJECT
 
 public:
-  explicit ConfigScopes();
+  struct Deps {
+    virtual ~Deps() = default;
+    virtual std::shared_ptr<QSettingsProxy> makeUserSettings();
+    virtual std::shared_ptr<QSettingsProxy> makeSystemSettings();
+  };
+
+  explicit ConfigScopes(std::shared_ptr<Deps> deps = std::make_shared<Deps>());
   ~ConfigScopes() override = default;
 
   void signalReady() override;
@@ -59,10 +65,8 @@ signals:
 
 private:
   Scope m_currentScope = Scope::User;
-  std::unique_ptr<QSettings> m_pUserSettings;
+  std::shared_ptr<QSettingsProxy> m_pUserSettingsProxy;
-  std::unique_ptr<QSettings> m_pSystemSettings;
+  std::shared_ptr<QSettingsProxy> m_pSystemSettingsProxy;
-  QSettingsProxy m_userSettingsProxy;
-  QSettingsProxy m_systemSettingsProxy;
 };
 
 } // namespace synergy::gui

src/lib/gui/constants.h

@@ -21,28 +21,10 @@
 
 namespace synergy::gui {
 
-const int kDebugLogLevel = 1;
+// important: this is used for settings paths on some platforms,
-
+// and must not be a url. qt automatically converts this to reverse domain
-#ifdef SYNERGY_PRODUCT_NAME
+// notation (rdn), e.g. com.symless
-const QString kProductName = SYNERGY_PRODUCT_NAME;
+const auto kOrgDomain = "symless.com";
-#else
-const QString kProductName;
-#endif
-
-#ifdef SYNERGY_LICENSED_PRODUCT
-const bool kLicensedProduct = true;
-#else
-const bool kLicensedProduct = false;
-#endif
-
-#ifdef SYNERGY_ENABLE_ACTIVATION
-#ifndef SYNERGY_LICENSED_PRODUCT
-#error "activation requires licensed product"
-#endif
-const bool kEnableActivation = true;
-#else
-const bool kEnableActivation = false;
-#endif // SYNERGY_ENABLE_ACTIVATION
 
 const auto kLinkBuy = R"(<a href="%1" style="color: %2">Buy now</a>)";
 const auto kLinkRenew = R"(<a href="%1" style="color: %2">Renew now</a>)";
@@ -65,4 +47,25 @@ const auto kUrlDownload =
     QString("%1/download?%2").arg(kUrlProduct, kUrlSourceQuery);
 const auto kUrlBugReport = QString("%1/issues").arg(kUrlGitHub);
 
+#ifdef SYNERGY_PRODUCT_NAME
+const QString kProductName = SYNERGY_PRODUCT_NAME;
+#else
+const QString kProductName;
+#endif
+
+#ifdef SYNERGY_LICENSED_PRODUCT
+const bool kLicensedProduct = true;
+#else
+const bool kLicensedProduct = false;
+#endif
+
+#ifdef SYNERGY_ENABLE_ACTIVATION
+#ifndef SYNERGY_LICENSED_PRODUCT
+#error "activation requires licensed product"
+#endif
+const bool kEnableActivation = true;
+#else
+const bool kEnableActivation = false;
+#endif // SYNERGY_ENABLE_ACTIVATION
+
 } // namespace synergy::gui

src/lib/gui/core/ServerConnection.cpp

@@ -49,38 +49,62 @@ ServerConnection::ServerConnection(
 void ServerConnection::handleLogLine(const QString &logLine) {
   ServerMessage message(logLine);
 
-  if (!m_appConfig.useExternalConfig() && message.isNewClientMessage() &&
+  if (!message.isNewClientMessage()) {
-      !m_ignoredClients.contains(message.getClientName())) {
+    return;
-    handleNewClient(message.getClientName());
   }
+
+  if (m_appConfig.useExternalConfig()) {
+    qDebug("external config enabled, skipping new client prompt");
+    return;
+  }
+
+  const auto client = message.getClientName();
+
+  if (m_receivedClients.contains(client)) {
+    qDebug(
+        "already got request, skipping new client prompt for: %s",
+        qPrintable(client));
+    return;
+  }
+
+  handleNewClient(message.getClientName());
 }
 
 void ServerConnection::handleNewClient(const QString &clientName) {
   using enum messages::NewClientPromptResult;
 
+  m_receivedClients.append(clientName);
+
+  if (m_messageShowing) {
+    qDebug("new client message already shown, skipping");
+    return;
+  }
+
   if (m_serverConfig.isFull()) {
     qDebug(
-        "server config full, skipping add client prompt for: %s",
+        "server config full, skipping new client prompt for: %s",
         qPrintable(clientName));
     return;
   }
 
   if (m_serverConfig.screenExists(clientName)) {
     qDebug(
-        "client already added, skipping add client prompt for: %s",
+        "client already added, skipping new client prompt for: %s",
         qPrintable(clientName));
     return;
   }
 
   emit messageShowing();
 
+  m_messageShowing = true;
   const auto result = m_pDeps->showNewClientPrompt(m_pParent, clientName);
+  m_messageShowing = false;
+
   if (result == Add) {
     qDebug("accepted dialog, adding client: %s", qPrintable(clientName));
     emit configureClient(clientName);
   } else if (result == Ignore) {
     qDebug("declined dialog, ignoring client: %s", qPrintable(clientName));
-    m_ignoredClients.append(clientName);
   } else {
     qFatal("unexpected add client result");
   }

src/lib/gui/core/ServerConnection.h

@@ -53,7 +53,8 @@ private:
   IAppConfig &m_appConfig;
   IServerConfig &m_serverConfig;
   std::shared_ptr<Deps> m_pDeps;
-  QStringList m_ignoredClients;
+  QStringList m_receivedClients;
+  bool m_messageShowing = false;
 };
 
 } // namespace synergy::gui

src/lib/gui/messages.cpp

@@ -40,6 +40,12 @@ struct Errors {
 std::unique_ptr<QMessageBox> Errors::s_criticalMessage;
 QStringList Errors::s_ignoredErrors;
 
+void raiseCriticalDialog() {
+  if (Errors::s_criticalMessage) {
+    Errors::s_criticalMessage->raise();
+  }
+}
+
 void showErrorDialog(
     const QString &message, const QMessageLogContext &context, QtMsgType type) {
   auto filename = QFileInfo(context.file).fileName();
@@ -72,7 +78,9 @@ void showErrorDialog(
   if (type == QtFatalMsg) {
     // create a blocking message box for fatal errors, as we want to wait
     // until the dialog is dismissed before aborting the app.
-    QMessageBox::critical(nullptr, title, text);
+    QMessageBox critical(
+        QMessageBox::Critical, title, text, QMessageBox::Abort);
+    critical.exec();
   } else if (!Errors::s_ignoredErrors.contains(message)) {
     // prevent message boxes piling up by deleting the last one if it exists.
     // if none exists yet, then nothing will happen.
@@ -85,6 +93,7 @@ void showErrorDialog(
     Errors::s_criticalMessage = std::make_unique<QMessageBox>(
         QMessageBox::Critical, title, text,
         QMessageBox::Ok | QMessageBox::Ignore);
+
     Errors::s_criticalMessage->open();
 
     QAction::connect(

src/lib/gui/messages.h

@@ -31,6 +31,8 @@ enum class NewClientPromptResult { Add, Ignore };
 void messageHandler(
     QtMsgType type, const QMessageLogContext &context, const QString &msg);
 
+void raiseCriticalDialog();
+
 void showFirstRunMessage(
     QWidget *parent, bool closeToTray, bool enableService, bool isServer);
 

src/lib/gui/proxy/QSettingsProxy.cpp

@@ -17,8 +17,151 @@
 
 #include "QSettingsProxy.h"
 
+#include "common/constants.h"
+#include "gui/Logger.h"
+
+#include <QCoreApplication>
+#include <QDir>
+#include <QFile>
+#include <QSettings>
+#include <memory>
+
 namespace synergy::gui::proxy {
 
+const auto kLegacyOrgDomain = "http-symless-com";
+
+const auto kSystemConfigFilename = "SystemConfig.ini";
+
+#if defined(Q_OS_UNIX)
+const auto kUnixSystemConfigPath = "/usr/local/etc/symless/";
+#endif
+
+//
+// Free functions
+//
+
+QString getSystemSettingPath() {
+  const QString settingFilename(kSystemConfigFilename);
+#if defined(Q_OS_WIN)
+  return QCoreApplication::applicationDirPath() + QDir::separator();
+#elif defined(Q_OS_MAC)
+  // it would be nice to use /Library dir, but qt has no elevate system.
+  return kUnixSystemConfigPath + settingFilename;
+#elif defined(Q_OS_LINUX)
+  // qt already adds application and filename to the end of the path on linux.
+  return kUnixSystemConfigPath;
+#else
+#error "unsupported platform"
+#endif
+}
+
+void migrateLegacySystemSettings(QSettings &settings) {
+  if (QFile(settings.fileName()).exists()) {
+    qDebug("system settings already exist, skipping migration");
+    return;
+  }
+
+  QSettings::setPath(
+      QSettings::IniFormat, QSettings::SystemScope, kSystemConfigFilename);
+  QSettings oldSystemSettings(
+      QSettings::IniFormat, QSettings::SystemScope,
+      QCoreApplication::organizationName(),
+      QCoreApplication::applicationName());
+
+  if (QFile(oldSystemSettings.fileName()).exists()) {
+    for (const auto &key : oldSystemSettings.allKeys()) {
+      settings.setValue(key, oldSystemSettings.value(key));
+    }
+  }
+
+  QSettings::setPath(
+      QSettings::IniFormat, QSettings::SystemScope, getSystemSettingPath());
+}
+
+void migrateLegacyUserSettings(QSettings &newSettings) {
+  QString newPath = newSettings.fileName();
+  QFile newFile(newPath);
+  if (newFile.exists()) {
+    qInfo("user settings already exist, skipping migration");
+    return;
+  }
+
+  QSettings oldSettings(kLegacyOrgDomain, kAppName);
+  QString oldPath = oldSettings.fileName();
+
+  if (oldPath.isEmpty()) {
+    qInfo("no legacy settings to migrate, filename empty");
+    return;
+  }
+
+  if (!QFile(oldPath).exists()) {
+    qInfo("no legacy settings to migrate, file does not exist");
+    return;
+  }
+
+  QFileInfo oldFileInfo(oldPath);
+  QFileInfo newFileInfo(newPath);
+
+  qDebug(
+      "migrating legacy settings: '%s' -> '%s'", //
+      qPrintable(oldFileInfo.fileName()), qPrintable(newFileInfo.fileName()));
+
+  QStringList keys = oldSettings.allKeys();
+  for (const QString &key : keys) {
+    QVariant oldValue = oldSettings.value(key);
+    newSettings.setValue(key, oldValue);
+    logVerbose(
+        QString("migrating setting '%1' = '%2'").arg(key, oldValue.toString()));
+  }
+
+  newSettings.sync();
+}
+
+//
+// QSettingsProxy
+//
+
+void QSettingsProxy::loadUser() {
+  m_pSettings = std::make_unique<QSettings>();
+
+#if defined(Q_OS_MAC)
+  // on mac, we used to save settings to "com.http-symless-com.Synergy.plist"
+  // because `setOrganizationName` was historically called using a url instead
+  // of an actual domain (e.g. symless.com).
+  migrateLegacyUserSettings(*m_pSettings);
+#endif // Q_OS_MAC
+}
+
+void QSettingsProxy::loadSystem() {
+  auto orgName = QCoreApplication::organizationName();
+  if (orgName.isEmpty()) {
+    qFatal("unable to load config, organization name is empty");
+    return;
+  } else {
+    qDebug() << "org name for config:" << orgName;
+  }
+
+  auto appName = QCoreApplication::applicationName();
+  if (appName.isEmpty()) {
+    qFatal("unable to load config, application name is empty");
+    return;
+  } else {
+    qDebug() << "app name for config:" << appName;
+  }
+
+  QSettings::setPath(
+      QSettings::Format::IniFormat, QSettings::Scope::SystemScope,
+      getSystemSettingPath());
+
+  m_pSettings = std::make_unique<QSettings>(
+      QSettings::Format::IniFormat, QSettings::Scope::SystemScope, orgName,
+      appName);
+
+#if defined(Q_OS_WIN)
+  migrateLegacySystemSettings(*m_pSettings);
+#endif // Q_OS_WIN
+}
+
 int QSettingsProxy::beginReadArray(const QString &prefix) {
   return m_pSettings->beginReadArray(prefix);
 }

src/lib/gui/proxy/QSettingsProxy.h

@@ -21,9 +21,16 @@
 
 namespace synergy::gui::proxy {
 
+QString getSystemSettingPath();
+
 class QSettingsProxy {
 public:
   virtual ~QSettingsProxy() = default;
+
+  virtual void loadUser();
+  virtual void loadSystem();
+
+  virtual void sync() { m_pSettings->sync(); }
   virtual int beginReadArray(const QString &prefix);
   virtual void beginWriteArray(const QString &prefix);
   virtual void setArrayIndex(int i);
@@ -39,11 +46,10 @@ public:
   virtual bool contains(const QString &key) const;
   virtual QString fileName() const { return m_pSettings->fileName(); }
 
-  void set(QSettings &settings) { m_pSettings = &settings; }
   QSettings &get() const { return *m_pSettings; }
 
 private:
-  QSettings *m_pSettings;
+  std::unique_ptr<QSettings> m_pSettings;
 };
 
 } // namespace synergy::gui::proxy

src/lib/gui/string_utils.h

@@ -25,3 +25,11 @@
 inline bool strToTrue(const QString &str) {
   return str.toLower() == "true" || str == "1";
 }
+
+inline QString trimEnd(const QString &str) {
+  QString result = str;
+  while (!result.isEmpty() && result.at(result.size() - 1).isSpace()) {
+    result.chop(1);
+  }
+  return result;
+}

src/test/unittests/gui/config/ConfigScopesTests.cpp

@@ -0,0 +1,206 @@
+/*
+ * synergy -- mouse and keyboard sharing utility
+ * Copyright (C) 2024 Symless Ltd.
+ *
+ * This package is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * found in the file LICENSE that should have accompanied this file.
+ *
+ * This package is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "gui/config/ConfigScopes.h"
+
+#include "gmock/gmock.h"
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+#include <memory>
+
+using namespace testing;
+using namespace synergy::gui;
+using namespace synergy::gui::proxy;
+
+namespace {
+
+class QSettingsProxyMock : public QSettingsProxy {
+public:
+  MOCK_METHOD(void, loadSystem, (), (override));
+  MOCK_METHOD(void, loadUser, (), (override));
+  MOCK_METHOD(QString, fileName, (), (const, override));
+  MOCK_METHOD(void, sync, (), (override));
+  MOCK_METHOD(bool, isWritable, (), (const, override));
+  MOCK_METHOD(bool, contains, (const QString &), (const, override));
+  MOCK_METHOD(QVariant, value, (const QString &), (const, override));
+  MOCK_METHOD(
+      QVariant, value, (const QString &, const QVariant &), (const, override));
+  MOCK_METHOD(void, setValue, (const QString &, const QVariant &), (override));
+};
+
+struct DepsMock : public ConfigScopes::Deps {
+  DepsMock() {
+    ON_CALL(*this, makeUserSettings()).WillByDefault(Return(m_pUserSettings));
+    ON_CALL(*this, makeSystemSettings())
+        .WillByDefault(Return(m_pSystemSettings));
+  }
+
+  MOCK_METHOD(
+      std::shared_ptr<QSettingsProxy>, makeUserSettings, (), (override));
+  MOCK_METHOD(
+      std::shared_ptr<QSettingsProxy>, makeSystemSettings, (), (override));
+
+  std::shared_ptr<QSettingsProxyMock> m_pUserSettings =
+      std::make_shared<NiceMock<QSettingsProxyMock>>();
+  std::shared_ptr<QSettingsProxyMock> m_pSystemSettings =
+      std::make_shared<NiceMock<QSettingsProxyMock>>();
+};
+
+} // namespace
+
+TEST(ConfigScopesTests, ctor_callsMakeUserSettings) {
+  std::shared_ptr<NiceMock<DepsMock>> deps =
+      std::make_shared<NiceMock<DepsMock>>();
+
+  EXPECT_CALL(*deps, makeUserSettings()).Times(1);
+
+  ConfigScopes configScopes(deps);
+}
+
+TEST(ConfigScopesTests, ctor_callsMakeSystemSettings) {
+  std::shared_ptr<NiceMock<DepsMock>> deps =
+      std::make_shared<NiceMock<DepsMock>>();
+
+  EXPECT_CALL(*deps, makeSystemSettings()).Times(1);
+
+  ConfigScopes configScopes(deps);
+}
+
+TEST(ConfigScopesTests, save_syncsBothScopes) {
+  std::shared_ptr<NiceMock<DepsMock>> deps =
+      std::make_shared<NiceMock<DepsMock>>();
+
+  ConfigScopes configScopes(deps);
+
+  EXPECT_CALL(*deps->m_pUserSettings, sync()).Times(1);
+  EXPECT_CALL(*deps->m_pSystemSettings, sync()).Times(1);
+
+  configScopes.save();
+}
+
+TEST(ConfigScopesTests, activeSettings_returnsUserSettingsByDefault) {
+  std::shared_ptr<NiceMock<DepsMock>> deps =
+      std::make_shared<NiceMock<DepsMock>>();
+
+  ConfigScopes configScopes(deps);
+
+  EXPECT_EQ(&configScopes.activeSettings(), deps->m_pUserSettings.get());
+}
+
+TEST(ConfigScopesTests, activeSettings_returnsSystemSettingsWhenSystemScope) {
+  std::shared_ptr<NiceMock<DepsMock>> deps =
+      std::make_shared<NiceMock<DepsMock>>();
+
+  ConfigScopes configScopes(deps);
+  configScopes.setActiveScope(ConfigScopes::Scope::System);
+
+  EXPECT_EQ(&configScopes.activeSettings(), deps->m_pSystemSettings.get());
+}
+
+TEST(ConfigScopesTests, setActiveScope_setsCurrentScope) {
+  std::shared_ptr<NiceMock<DepsMock>> deps =
+      std::make_shared<NiceMock<DepsMock>>();
+
+  ConfigScopes configScopes(deps);
+
+  configScopes.setActiveScope(ConfigScopes::Scope::System);
+
+  EXPECT_EQ(configScopes.activeScope(), ConfigScopes::Scope::System);
+}
+
+TEST(
+    ConfigScopesTests,
+    isActiveScopeWritable_returnsTrueWhenUserSettingsWritable) {
+  std::shared_ptr<NiceMock<DepsMock>> deps =
+      std::make_shared<NiceMock<DepsMock>>();
+
+  ConfigScopes configScopes(deps);
+
+  EXPECT_CALL(*deps->m_pUserSettings, isWritable()).WillOnce(Return(true));
+
+  EXPECT_TRUE(configScopes.isActiveScopeWritable());
+}
+
+TEST(
+    ConfigScopesTests,
+    scopeContains_byDefault_returnsTrueWhenUserSettingsContainsKey) {
+  std::shared_ptr<NiceMock<DepsMock>> deps =
+      std::make_shared<NiceMock<DepsMock>>();
+
+  ON_CALL(*deps->m_pUserSettings, contains(_)).WillByDefault(Return(true));
+
+  ConfigScopes configScopes(deps);
+
+  EXPECT_TRUE(configScopes.scopeContains("stub"));
+}
+
+TEST(
+    ConfigScopesTests,
+    scopeContains_userScope_returnsTrueWhenUserSettingsContainsKey) {
+  std::shared_ptr<NiceMock<DepsMock>> deps =
+      std::make_shared<NiceMock<DepsMock>>();
+
+  ON_CALL(*deps->m_pUserSettings, contains(_)).WillByDefault(Return(true));
+
+  ConfigScopes configScopes(deps);
+
+  EXPECT_TRUE(configScopes.scopeContains("stub", ConfigScopes::Scope::User));
+}
+
+TEST(
+    ConfigScopesTests,
+    scopeContains_systemScope_returnsTrueWhenSystemSettingsContainsKey) {
+  std::shared_ptr<NiceMock<DepsMock>> deps =
+      std::make_shared<NiceMock<DepsMock>>();
+
+  ON_CALL(*deps->m_pSystemSettings, contains(_)).WillByDefault(Return(true));
+
+  ConfigScopes configScopes(deps);
+
+  EXPECT_TRUE(configScopes.scopeContains("stub", ConfigScopes::Scope::System));
+}
+
+TEST(ConfigScopesTests, activeFilePath_returnsUserSettingsFileNameByDefault) {
+  std::shared_ptr<NiceMock<DepsMock>> deps =
+      std::make_shared<NiceMock<DepsMock>>();
+  ON_CALL(*deps->m_pUserSettings, fileName()).WillByDefault(Return("test"));
+
+  ConfigScopes configScopes(deps);
+
+  EXPECT_EQ(configScopes.activeFilePath(), "test");
+}
+
+TEST(ConfigScopesTests, getFromScope_byDefault_returnsValueFromActiveSettings) {
+  std::shared_ptr<NiceMock<DepsMock>> deps =
+      std::make_shared<NiceMock<DepsMock>>();
+  ON_CALL(*deps->m_pUserSettings, value(_, _)).WillByDefault(Return("test"));
+
+  ConfigScopes configScopes(deps);
+
+  EXPECT_EQ(configScopes.getFromScope("stub"), "test");
+}
+
+TEST(ConfigScopesTests, setInScope_byDefault_setsValueInActiveSettings) {
+  std::shared_ptr<NiceMock<DepsMock>> deps =
+      std::make_shared<NiceMock<DepsMock>>();
+
+  ConfigScopes configScopes(deps);
+
+  EXPECT_CALL(*deps->m_pUserSettings, setValue(_, _)).Times(1);
+
+  configScopes.setInScope("stub", "test");
+}