Skip to content

fix(keyboard): resolve layout name disappearing after deletion#149

Merged
wyu71 merged 1 commit intolinuxdeepin:masterfrom
wyu71:master
Apr 28, 2026
Merged

fix(keyboard): resolve layout name disappearing after deletion#149
wyu71 merged 1 commit intolinuxdeepin:masterfrom
wyu71:master

Conversation

@wyu71
Copy link
Copy Markdown
Contributor

@wyu71 wyu71 commented Apr 28, 2026

Batch collect layout descriptions before updating model to avoid transient empty state during async DBus calls.

批量收集布局描述后再更新模型,避免异步DBus调用期间的空状态。

Also show done button when in editing mode to prevent user getting stuck.

同时修复编辑模式下完成按钮消失的问题。

Log: 修复删除键盘布局后名称消失及完成按钮不显示的问题
PMS: BUG-356121
Influence: 键盘布局编辑功能现在正常工作,删除布局后名称正确显示,编辑模式下始终可以退出编辑。

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @wyu71, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@wyu71 wyu71 force-pushed the master branch 2 times, most recently from 607cbb1 to bca76ea Compare April 28, 2026 08:17
Batch collect layout descriptions before updating model to avoid
transient empty state during async DBus calls.

批量收集布局描述后再更新模型,避免异步DBus调用期间的空状态。

Also show done button when in editing mode to prevent user getting stuck.

同时修复编辑模式下完成按钮消失的问题。

Log: 修复删除键盘布局后名称消失及完成按钮不显示的问题
PMS: BUG-356121
Influence: 键盘布局编辑功能现在正常工作,删除布局后名称正确显示,编辑模式下始终可以退出编辑。
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这份代码 diff 主要对键盘布局的加载逻辑进行了重构,引入了批处理机制以避免异步回调导致的界面闪烁问题。以下是对代码的详细审查意见,包括语法逻辑、代码质量、代码性能和代码安全四个方面:

1. 语法逻辑

优点:

  • 异步批处理逻辑正确:引入了 m_batchIdm_pendingLayoutCount。当 onUserLayout 被调用时,生成一个新的 batchId。在 onUserLayoutFinished 回调中,首先检查 batchId 是否匹配。这有效地解决了异步操作时序问题(即前一次请求的回调在后一次请求开始后才返回的情况),防止了旧数据覆盖新数据。
  • 空列表处理:在 onUserLayout 中增加了 if (list.isEmpty()) 的判断,直接清理并返回,避免了不必要的 DBus 调用和后续逻辑处理。
  • 错误处理:在 onUserLayoutFinished 中增加了 reply.isError() 的检查,使用 qWarning 打印错误日志,防止了 DBus 调用失败时程序崩溃或状态异常。

改进建议:

  • UI 显示逻辑:在 KeyboardLayoutModule.qml 中,修改了按钮的可见性条件:
    visible: dccData.keyboardController.layoutCount > 1 || keyboardLayoutTitle.isEditing
    这在逻辑上是合理的,允许用户在编辑模式下(即使布局数量 <= 1)也能看到相关按钮。但需确保 layoutCount 的更新时机与 UI 刷新同步,避免在 cleanUserLayout()addUserLayout() 之间出现短暂的 UI 状态不一致。

2. 代码质量

优点:

  • 可读性提升:通过引入中间变量 m_pendingLayouts 缓存结果,最后统一更新 Model,使得数据流向更加清晰。
  • 资源管理:使用了 watch->deleteLater() 来管理 QDBusPendingCallWatcher 的生命周期,符合 Qt 的内存管理规范。

改进建议:

  • 魔法数字/类型m_batchId 使用了 quint64,这是一个很大的范围。虽然不太可能在短时间内溢出,但建议在代码注释中说明其用途及溢出后的行为(尽管溢出后重新从 0 开始在逻辑上通常也是安全的,因为旧请求的 watcher 会被过滤)。
  • 成员变量初始化:在头文件中 m_pendingLayoutCountm_batchId 进行了就地初始化(C++11 特性),这是好的做法。但建议在构造函数初始化列表中也显式列出它们,以便于维护者一目了然地看到所有成员的初始状态。

3. 代码性能

优点:

  • 减少 UI 刷新:原代码在每次 onUserLayoutFinished 回调时都调用 addUserLayout,这会导致 Model 频繁触发 dataChanged 信号,进而导致 UI 频繁重绘。新代码将所有布局收集到 m_pendingLayouts 中,待所有请求完成后,先 cleanUserLayout 再批量 addUserLayout。这显著减少了 UI 的刷新次数,提升了性能和用户体验(消除了闪烁)。

改进建议:

  • 批量操作优化:目前的批量操作是循环调用 addUserLayout。如果 Model 的实现允许,最好能提供一个 setUserLayouts(const QMap<QString, QString> &layouts) 的接口,这样 Model 内部只需发出一次 layoutReset 或类似的信号,而不是 N 次 add 信号。这将进一步提升性能。
  • DBus 调用for 循环中发起了多个 DBus 异步调用。如果布局列表非常大(虽然通常键盘布局不会太多),可能会对 DBus 总线造成瞬时压力。目前的实现是合理的,因为它们是并行的。

4. 代码安全

优点:

  • 防御性编程:在 onUserLayoutFinished 中增加了 batchId 检查,这是一种很好的防御性编程实践,防止了"野指针"或"过期回调"对当前状态的破坏。
  • 错误日志:记录了 DBus 调用的错误信息,有助于排查线上问题。

改进建议:

  • 数据一致性m_pendingLayoutCount 是一个 int 类型。如果在极端高并发或异常情况下(虽然 Qt DBus 通常是单线程事件循环),如果逻辑复杂化,单纯依赖 --m_pendingLayoutCount 可能存在风险。但在当前上下文中,由于 Qt 的事件循环机制,这是安全的。
  • 异常安全QDBusPendingReply<QString> reply = *watch; 这行代码如果 watch 已经失效或无效,可能会抛出异常或导致未定义行为。虽然 finished 信号通常保证 watcher 有效,但增加一个 watcher 的有效性检查(虽然 Qt 文档说 finished 信号发出时 watcher 依然有效)会更严谨。
  • SPDX 更新:将版权年份更新至 2026 是合规操作,确保了法律层面的安全性。

总结

这是一次高质量的代码重构。主要改进点在于通过引入批处理 ID 和计数器,解决了异步 DBus 调用导致的 UI 状态不一致和闪烁问题。

建议的后续优化:

  1. 考虑在 Model 层增加批量设置接口,减少信号发射频率。
  2. 在构造函数中显式初始化新增的成员变量。
  3. 确保 layoutCountcleanUserLayout 后能立即反映为 0,以保证 QML 中的 visible 判断逻辑在数据清空瞬间也是准确的。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lzwind, wyu71

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@wyu71 wyu71 merged commit 86e9023 into linuxdeepin:master Apr 28, 2026
12 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants