fix: handle cursor size configuration changes #246
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1. Added handling for cursor size changes from XSettings DConfig in AppearanceManager 2. Fixed cursor size initialization from 24 instead of -1 when selecting cursor theme 3. Corrected GSettings key from "cursor-size" to "cursorSize" for GNOME desktop interface compatibility Log: Fixed cursor size configuration handling and GNOME desktop integration Influence: 1. Test cursor size changes through DConfig settings 2. Verify cursor theme switching with different available sizes 3. Test GNOME desktop compatibility with cursor size settings 4. Verify cursor size persistence across system restarts 5. Test cursor size changes in different desktop environments fix: 处理光标大小配置变更 1. 在 AppearanceManager 中添加了对 XSettings DConfig 中光标大小变更的 处理 2. 修复了选择光标主题时，光标大小从24而不是-1初始化的逻辑 3. 更正了 GNOME 桌面接口的 GSettings 键值，从 "cursor-size" 改为 "cursorSize" 以提升兼容性 Log: 修复了光标大小配置处理和 GNOME 桌面集成问题 Influence: 1. 测试通过 DConfig 设置更改光标大小的功能 2. 验证在不同可用尺寸下切换光标主题的功能 3. 测试光标大小设置在 GNOME 桌面环境下的兼容性 4. 验证光标大小在系统重启后的持久性 5. 测试在不同桌面环境下更改光标大小的功能
|
TAG Bot TAG: 1.1.74 |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideImplements proper propagation and persistence of cursor size configuration changes from DConfig/XSettings through AppearanceManager, fixes cursor theme size selection defaults, and aligns GNOME interface GSettings keys for cursor size to ensure cross-desktop compatibility. Sequence diagram for cursor size change via XSettings DConfigsequenceDiagram
participant XSettingsDconfig
participant AppearanceManager
participant Property
participant Listener
XSettingsDconfig->>AppearanceManager: notifyChange(DCKEYCURSORSIZEBASE)
AppearanceManager->>XSettingsDconfig: value(DCKEYCURSORSIZEBASE)
XSettingsDconfig-->>AppearanceManager: int cursorSize
AppearanceManager->>Property: cursorSize = value
AppearanceManager->>Listener: Changed(CursorSize, cursorSize)
Sequence diagram for cursor theme change and cursor size selectionsequenceDiagram
actor User
participant AppearanceManager
participant Subthemes
participant CursorTheme
participant Utils
User->>AppearanceManager: doSetCursorTheme(themeId)
AppearanceManager->>Subthemes: listCursorThemes()
Subthemes-->>AppearanceManager: cursorThemes
loop findMatchingTheme
AppearanceManager->>CursorTheme: getId()
CursorTheme-->>AppearanceManager: id
end
AppearanceManager->>CursorTheme: getPath()
CursorTheme-->>AppearanceManager: themePath
AppearanceManager->>Utils: getAvailableCursorSizes(themePath)
Utils-->>AppearanceManager: availableSizes
Note over AppearanceManager: initialize cursorSize = 24
loop chooseClosestSize
AppearanceManager->>AppearanceManager: cursorSize = closest to Property.cursorSize
end
AppearanceManager->>AppearanceManager: apply themeId and cursorSize
Sequence diagram for GNOME GSettings cursor size synchronizationsequenceDiagram
participant Client
participant ThemesApi
participant QGSettings
Client->>ThemesApi: setCursorSize(size)
ThemesApi->>QGSettings: isSchemaInstalled(org.gnome.desktop.interface)
QGSettings-->>ThemesApi: bool installed
alt schema installed
ThemesApi->>QGSettings: QGSettings(schema, path, parent)
QGSettings-->>ThemesApi: gtkSettings
ThemesApi->>gtkSettings: keys()
gtkSettings-->>ThemesApi: keyList
alt keyList contains cursorSize
ThemesApi->>gtkSettings: set(cursorSize, size)
end
end
Class diagram for updated cursor size handlingclassDiagram
class AppearanceManager {
+int handleXsettingDConfigChange(QString key)
+bool doSetCursorTheme(QString value)
-int getSuitableCursorSize(QString value)
-Property* m_property
-XSettingsDconfig* m_XSettingsDconfig
-Subthemes* m_subthemes
}
class Property {
+int cursorSize
+int windowRadius
}
class XSettingsDconfig {
+QVariant value(QString key)
}
class Subthemes {
+QList~CursorTheme*~ listCursorThemes()
}
class CursorTheme {
+QString getId()
+QString getPath()
}
class Utils {
+QList~int~ getAvailableCursorSizes(QString themePath)
}
class ThemesApi {
+bool setCursorSize(int size)
}
class QGSettings {
+QGSettings(QString schema, QByteArray path, QObject parent)
+QStringList keys()
+void set(QString key, int value)
+static bool isSchemaInstalled(QString schema)
}
AppearanceManager --> Property : manages
AppearanceManager --> XSettingsDconfig : reads_from
AppearanceManager --> Subthemes : queries
Subthemes --> CursorTheme : contains
AppearanceManager ..> Utils : uses
ThemesApi ..> QGSettings : updates
ThemesApi ..> Property : affects_cursorSize
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In
doSetCursorTheme, the lambda capture[this, value]still includesvalueeven though it’s no longer used; consider removing it from the capture list to avoid confusion. - The
getSuitableCursorSizelambda now defaultscursorSizeto a hard-coded 24 when no matching theme/sizes are found; consider making this default explicit (e.g., via a named constant or configuration) or handling the "no suitable size" case separately so the behavior is clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `doSetCursorTheme`, the lambda capture `[this, value]` still includes `value` even though it’s no longer used; consider removing it from the capture list to avoid confusion.
- The `getSuitableCursorSize` lambda now defaults `cursorSize` to a hard-coded 24 when no matching theme/sizes are found; consider making this default explicit (e.g., via a named constant or configuration) or handling the "no suitable size" case separately so the behavior is clearer.
## Individual Comments
### Comment 1
<location> `src/service/modules/api/themes.cpp:710-711` </location>
<code_context>
QGSettings gtkSettings("org.gnome.desktop.interface", QByteArray(), this);
- if (gtkSettings.keys().contains("cursor-size")) {
- gtkSettings.set("cursor-size", size);
+ if (gtkSettings.keys().contains("cursorSize")) {
+ gtkSettings.set("cursorSize", size);
}
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Changing the GNOME cursor key to `cursorSize` is likely incorrect and will prevent the setting from taking effect.
Upstream `org.gnome.desktop.interface` uses the `cursor-size` key (with a dash). Changing this to `cursorSize` means the `contains` check and `set` call will no longer match the actual schema, so the cursor size won’t be applied to GTK. Unless you’re targeting a forked schema that really uses `cursorSize`, this should stay as `"cursor-size"` for compatibility.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
deepin pr auto review我来对这个git diff进行代码审查：
建议改进：
} else if (key == DCKEYCURSORSIZEBASE) {
int newSize = m_XSettingsDconfig->value(DCKEYCURSORSIZEBASE).toInt();
if (newSize > 0) { // 添加有效性检查
m_property->cursorSize = newSize;
Q_EMIT Changed("CursorSize", QString::number(m_property->cursorSize));
}
}
auto getSuitableCursorSize = [this, value]() {
int cursorSize = 24;
for (auto iter : m_subthemes->listCursorThemes()) {
if (iter->getId() == value) {
auto availableSizes = utils::getAvailableCursorSizes(iter->getPath());
if (availableSizes.isEmpty()) { // 添加空列表检查
return cursorSize;
}
int len = INT_MAX;
for (auto size : availableSizes) {
if (std::abs(size - m_property->cursorSize) < len) {
len = std::abs(size - m_property->cursorSize);
cursorSize = size;
}
}
break;
}
}
return cursorSize;
};
bool ThemesApi::setCursorSize(int size)
{
if (size <= 0) { // 添加参数有效性检查
return false;
}
// ... 其余代码
}这些改进建议主要是为了增强代码的健壮性，防止可能的异常情况。整体来说，这次修改的质量是不错的。 |
yixinshark
approved these changes
Dec 8, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mhduiy, yixinshark The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Log: Fixed cursor size configuration handling and GNOME desktop integration
Influence:
fix: 处理光标大小配置变更
Log: 修复了光标大小配置处理和 GNOME 桌面集成问题
Influence:
Summary by Sourcery
Handle cursor size configuration changes across DConfig, theme selection, and GNOME integration.
Bug Fixes: