Skip to content

Conversation

@zccrs
Copy link
Member

@zccrs zccrs commented Mar 10, 2025

pms: BUG-303949

@zccrs zccrs requested a review from robertkill March 10, 2025 11:38
robertkill
robertkill previously approved these changes Mar 10, 2025
18202781743
18202781743 previously approved these changes Mar 10, 2025
@18202781743
Copy link
Contributor

测试了下,小键盘还是不能输入,
编译了测试的pr,查看启动的是编译的libdock-plugin.so

@zccrs
Copy link
Member Author

zccrs commented Mar 10, 2025

测试了下,小键盘还是不能输入, 编译了测试的pr,查看启动的是编译的libdock-plugin.so

更新了一下,还是依赖 zqt_key_v1 协议

@zccrs zccrs force-pushed the master branch 2 times, most recently from 8ef0878 to fca251c Compare March 11, 2025 01:41
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见:

  1. 代码修改的上下文缺失:提交的代码片段没有提供足够的上下文信息,例如修改的原因、预期的行为变化等。建议在提交信息中详细说明修改的背景和目的。

  2. 宏定义的使用:在pluginmanagerextension.cpp文件中使用了#define protected public宏定义,这可能会破坏类的封装性。建议评估是否真的需要这样做,或者是否有更好的设计方式来访问私有成员。

  3. 硬编码的注释:注释// ###(zccrs): 在dde-shell中不要使用QWaylandCompositor的event handler,它会对key event进行特殊处理,会丢弃掉原生事件的信息,仅根据 native scan key code 通过xkb生成原始key event向 dde-shell 传递,这会导致需要状态切换后进行输入的字符丢失信息,比如外部环境打开NumLock后,小键盘 的数字输入会被event handler转成原始事件,丢失了NumLock的开关信息。 dde-shell不是一个独立的合成器,所以不需要额外处理key event,需要遵守原始事件中的NumLock状态 确保输出数字时在dde-shell的wayland客户端中接收到的也是数字。中包含了硬编码的注释,这可能会在代码审查时引起混淆。建议将注释内容提取到单独的文档或配置文件中。

  4. 代码重复:在pluginmanagerextension.cpp文件中,auto eventHandler = QWaylandCompositorPrivate::get(compositor)->eventHandler.get();if (eventHandler == QWindowSystemInterfacePrivate::eventHandler) {这两行代码重复了,应该合并或重构以减少代码重复。

  5. 资源管理:在pluginmanagerextension.cpp文件中,auto qtKey = new QtWayland::QtKeyExtensionGlobal(compositor);auto qtTouch = new QtWayland::TouchExtensionGlobal(compositor);这两行代码创建了新的对象,但没有看到相应的删除或释放资源的代码。建议确保在适当的时候释放这些资源,以避免内存泄漏。

  6. 代码风格:代码中存在一些风格不一致的问题,例如注释的格式和空格的使用。建议统一代码风格,以提高代码的可读性。

  7. 安全性:没有看到对代码中可能存在的安全漏洞的评估和修复。建议进行安全性审查,确保代码不会引入安全风险。

  8. 性能考虑:没有看到对代码性能的评估和优化。建议进行性能分析,确保代码的修改不会对性能产生负面影响。

综上所述,建议在提交代码前进行更全面的审查,包括代码风格、安全性、性能等方面,以确保代码的质量和稳定性。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, zccrs

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

@18202781743
Copy link
Contributor

@18202781743 18202781743 merged commit 77cac3d into linuxdeepin:master Mar 12, 2025
9 of 10 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.

4 participants