-
Notifications
You must be signed in to change notification settings - Fork 16
refactor: Refactor config reader & config API #77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,283 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Copyright (C) 2026 UnionTech Software Technology Co., Ltd. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // SPDX-License-Identifier: GPL-2.0-or-later | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #pragma once | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #include <QFile> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #include <QMap> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #include <QObject> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #include <QPair> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #include <QVariant> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #include "Constants.h" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| namespace DDM { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class Config : public QObject { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Q_OBJECT | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Constructor. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param configPath Path to the configuration file. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param defaults Default configuration values and descriptions. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param parent Parent QObject. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Config(const QString &configPath, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Check warning on line 25 in src/common/Config.h
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // section entry default description | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| QMap<QString, QMap<QString, QPair<QVariant, QString>>> defaults = {}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| QObject *parent = nullptr) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : QObject(parent) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| , m_configPath(configPath) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| , m_defaults(defaults) { } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Get a configuration entry value. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * If the entry is not found in the loaded configuration, the default value is returned. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * If no default value is set, an exception is thrown. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param section The configuration section name. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param entry The configuration entry name. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @return The value of the configuration entry. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| template<typename T> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| inline T get(const QString §ion, const QString &entry) const { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (m_data.contains(section) && m_data[section].contains(entry)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return m_data[section][entry].value<T>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else if (m_defaults.contains(section) && m_defaults[section].contains(entry)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return m_defaults[section][entry].first.value<T>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw std::runtime_error(QStringLiteral("Config entry '%1/%2' not found") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .arg(section, entry) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .toStdString()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Get a configuration entry value inside the default section. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * If the entry is not found in the loaded configuration, the default value is returned. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * If no default value is set, an exception is thrown. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param entry The configuration entry name. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @return The value of the configuration entry. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| template<typename T> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| inline T get(const QString &entry) const { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return get<T>({}, entry); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+62
to
+65
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Set a configuration entry value. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param section The configuration section name. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param entry The configuration entry name. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param value The value to set. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| template<typename T> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| inline void set(const QString §ion, const QString &entry, const T &value) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| m_data[section][entry] = QVariant::fromValue(value); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Set a configuration entry value inside the default section. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param entry The configuration entry name. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param value The value to set. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| template<typename T> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| inline void set(const QString &entry, const T &value) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| m_data[{}][entry] = QVariant::fromValue(value); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Reset a configuration entry to its default value. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param section The configuration section name. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param entry The configuration entry name. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @return True if the entry was reset, false if it was not found or already default. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| inline bool setDefault(const QString §ion, const QString &entry) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (m_data.contains(section) && m_data[section].contains(entry)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| m_data[section].remove(entry); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Reset a configuration entry to its default value inside the default section. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param entry The configuration entry name. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @return True if the entry was reset, false if it was not found or already default. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| inline bool setDefault(const QString &entry) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return setDefault({}, entry); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Load the configuration from the file. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @return True if the configuration was loaded successfully, false otherwise. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bool load() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| QFile file(m_configPath); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!file.open(QIODevice::ReadOnly)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| QString currentSection{}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| while (!file.atEnd()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| QString line = QString::fromLocal8Bit(file.readLine()).trimmed(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (line.isEmpty() || line.startsWith('#')) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| continue; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (line.startsWith('[') && line.endsWith(']')) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| currentSection = line.mid(1, line.length() - 2).trimmed(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| int separatorPosition = line.indexOf('='); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (separatorPosition >= 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!m_data.contains(currentSection)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| m_data[currentSection] = {}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| QString name = line.left(separatorPosition).trimmed(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| QString value = line.mid(separatorPosition + 1).trimmed(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (value.isEmpty()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| m_data[currentSection][name] = QVariant(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else if (value.indexOf(',') >= 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| QStringList list = value.split(',', Qt::SkipEmptyParts); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (QString &item : list) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| item = item.trimmed(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| m_data[currentSection][name] = QVariant(list); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| m_data[currentSection][name] = QVariant(value); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+141
to
+147
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else if (value.indexOf(',') >= 0) { | |
| QStringList list = value.split(',', Qt::SkipEmptyParts); | |
| for (QString &item : list) | |
| item = item.trimmed(); | |
| m_data[currentSection][name] = QVariant(list); | |
| } else | |
| m_data[currentSection][name] = QVariant(value); | |
| } else { | |
| // Decide whether to treat the value as a list based on the default type. | |
| bool treatAsList = false; | |
| auto sectionDefaultsIt = m_defaults.constFind(currentSection); | |
| if (sectionDefaultsIt != m_defaults.cend()) { | |
| const auto &entryDefaults = sectionDefaultsIt.value(); | |
| auto entryIt = entryDefaults.constFind(name); | |
| if (entryIt != entryDefaults.cend() && | |
| entryIt->first.userType() == QMetaType::QStringList) { | |
| treatAsList = true; | |
| } | |
| } | |
| if (treatAsList) { | |
| QStringList list = value.split(',', Qt::SkipEmptyParts); | |
| for (QString &item : list) | |
| item = item.trimmed(); | |
| m_data[currentSection][name] = QVariant(list); | |
| } else { | |
| m_data[currentSection][name] = QVariant(value); | |
| } | |
| } |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The old ConfigReader implementation had backward compatibility for renamed sections: "XDisplay" was automatically mapped to "X11", and "WaylandDisplay" to "Wayland" during loading (see deleted ConfigReader.cpp lines 210-213). The new Config class doesn't include this backward compatibility, which means existing configuration files using the old section names will not work correctly.
Consider adding backward compatibility mappings in the load() method to handle legacy section names, or document this as a breaking change requiring manual configuration migration.
旧的 ConfigReader 实现对重命名的部分具有向后兼容性:"XDisplay" 自动映射到 "X11","WaylandDisplay" 映射到 "Wayland"(参见已删除的 ConfigReader.cpp 第210-213行)。新的 Config 类不包含此向后兼容性,这意味着使用旧部分名称的现有配置文件将无法正常工作。
建议在 load() 方法中添加向后兼容性映射以处理旧的部分名称,或将此记录为需要手动配置迁移的破坏性更改。
| } | |
| } | |
| // Backward compatibility for legacy section names. | |
| // Map "XDisplay" -> "X11". | |
| if (m_data.contains(QStringLiteral("XDisplay"))) { | |
| auto legacy = m_data.take(QStringLiteral("XDisplay")); | |
| auto &target = m_data[QStringLiteral("X11")]; | |
| for (auto it = legacy.cbegin(); it != legacy.cend(); ++it) { | |
| if (!target.contains(it.key())) { | |
| target.insert(it.key(), it.value()); | |
| } | |
| } | |
| } | |
| // Map "WaylandDisplay" -> "Wayland". | |
| if (m_data.contains(QStringLiteral("WaylandDisplay"))) { | |
| auto legacy = m_data.take(QStringLiteral("WaylandDisplay")); | |
| auto &target = m_data[QStringLiteral("Wayland")]; | |
| for (auto it = legacy.cbegin(); it != legacy.cend(); ++it) { | |
| if (!target.contains(it.key())) { | |
| target.insert(it.key(), it.value()); | |
| } | |
| } | |
| } |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The load() method stores all configuration values as QString (line 147), including boolean values. However, when calling get(), the code attempts to convert QString to bool using QVariant::value(). QVariant's QString to bool conversion doesn't work as expected - it only returns true if the QString can be converted to a number and that number is non-zero, not based on "true"/"false" string values. This will cause boolean configuration values like "RememberLastUser" to always evaluate to false when loaded from a file.
Consider implementing proper type detection and conversion in the load() method. For example, check if a default value exists for the entry and use its type to determine how to parse the string value. Alternatively, implement special parsing for known boolean string values like "true", "false", "yes", "no", "1", "0" (case-insensitive).
boolean 配置值加载存在bug。load() 方法将所有配置值存储为 QString(第147行),包括布尔值。但调用 get() 时,代码尝试使用 QVariant::value() 将 QString 转换为 bool。QVariant 的 QString 到 bool 的转换不是基于 "true"/"false" 字符串,而是检查字符串是否可以转换为非零数字。这将导致从文件加载的布尔配置值(如 "RememberLastUser")总是评估为 false。
建议在 load() 方法中实现正确的类型检测和转换。例如,检查该条目是否存在默认值,并使用其类型确定如何解析字符串值。或者,为已知的布尔字符串值(如 "true"、"false"、"yes"、"no"、"1"、"0",不区分大小写)实现特殊解析。
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new Config class only loads from a single configuration file (m_configPath), while the old ConfigReader implementation supported loading from multiple configuration directories in order of priority: system config directory (/usr/lib/ddm/ddm.conf.d/), user config directory (/etc/ddm.conf.d/), and the main config file (/etc/ddm.conf). This is a breaking change that removes the ability to use drop-in configuration files, which is a common pattern in system configuration.
Consider restoring support for configuration directories to maintain backward compatibility and allow modular configuration. The old implementation loaded files from configDir and sysConfigDir in alphabetical order before loading the main config file.
新的 Config 类仅从单个配置文件(m_configPath)加载,而旧的 ConfigReader 实现支持按优先级顺序从多个配置目录加载:系统配置目录(/usr/lib/ddm/ddm.conf.d/)、用户配置目录(/etc/ddm.conf.d/)和主配置文件(/etc/ddm.conf)。这是一个破坏性更改,删除了使用插入式配置文件的能力,这是系统配置中的常见模式。
建议恢复对配置目录的支持以保持向后兼容性并允许模块化配置。旧实现在加载主配置文件之前按字母顺序从 configDir 和 sysConfigDir 加载文件。
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The inline stateConfig instance is never loaded from the configuration file. While stateConfig.save() is called when user state changes (lines 268, 392 in Display.cpp), there's no corresponding stateConfig.load() call anywhere in the codebase. This means the last user and session state are never restored across daemon restarts, breaking the "remember last user/session" functionality.
Add a stateConfig.load() call during application initialization, likely in DaemonApp constructor or before the first use in Display.cpp.
stateConfig 实例从未从配置文件加载。虽然在用户状态更改时调用了 stateConfig.save()(Display.cpp 的第268、392行),但代码库中没有相应的 stateConfig.load() 调用。这意味着最后一个用户和会话状态在守护进程重启后不会恢复,破坏了"记住上次用户/会话"功能。
建议在应用程序初始化期间添加 stateConfig.load() 调用,可能在 DaemonApp 构造函数中或在 Display.cpp 中首次使用之前。
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The stateConfig path has been changed from a dynamic computation to a static constant. The old implementation (in deleted Configuration.h) attempted to use the home directory of the "ddm" user if it exists, falling back to STATE_DIR otherwise. The new implementation uses STATE_DIR directly, which may break existing deployments where the state is stored in the ddm user's home directory.
Consider restoring the dynamic path computation for backward compatibility, or document this as a breaking change and provide a migration path for existing installations.
stateConfig 路径已从动态计算更改为静态常量。旧实现(在已删除的 Configuration.h 中)尝试使用 "ddm" 用户的主目录(如果存在),否则回退到 STATE_DIR。新实现直接使用 STATE_DIR,这可能会破坏将状态存储在 ddm 用户主目录中的现有部署。
建议恢复动态路径计算以保持向后兼容性,或将此记录为破坏性更改并为现有安装提供迁移路径。
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The inline global Config instances (mainConfig and stateConfig) are not thread-safe. These instances may be accessed from multiple threads (e.g., different Display instances, signal handlers, etc.), but the Config class has no synchronization mechanisms. Operations like load(), save(), get(), and set() can race with each other, potentially causing data corruption or crashes.
Since Config inherits from QObject, consider using QMutex to protect all member data access, or clearly document that these instances should only be accessed from the main thread.
内联全局 Config 实例(mainConfig 和 stateConfig)不是线程安全的。这些实例可能从多个线程访问(例如,不同的 Display 实例、信号处理程序等),但 Config 类没有同步机制。load()、save()、get() 和 set() 等操作可能相互竞争,可能导致数据损坏或崩溃。
由于 Config 继承自 QObject,建议使用 QMutex 保护所有成员数据访问,或明确记录这些实例只应从主线程访问。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The get() method throws std::runtime_error when a configuration entry is not found, but this exception type and the throwing behavior are not documented in the class documentation or the method's docstring. Callers may not expect exceptions and could fail to handle them properly, potentially causing crashes.
Add exception specifications to the documentation, or consider returning an optional/default value instead of throwing to make the API more robust and Qt-like. If exceptions are the intended design, document them clearly in both the class-level and method-level documentation.
get() 方法在找不到配置条目时抛出 std::runtime_error,但此异常类型和抛出行为未在类文档或方法的文档字符串中记录。调用者可能不期望异常,可能无法正确处理它们,可能导致崩溃。
建议在文档中添加异常规范,或考虑返回可选/默认值而不是抛出异常,以使 API 更健壮和更像 Qt。如果异常是预期的设计,请在类级别和方法级别的文档中明确记录它们。